-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(spanner): derive built-in metrics project from database client #13262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |
| import com.google.cloud.opentelemetry.detection.AttributeKeys; | ||
| import com.google.cloud.opentelemetry.detection.DetectedPlatform; | ||
| import com.google.cloud.opentelemetry.detection.GCPPlatformDetector; | ||
| import com.google.common.annotations.VisibleForTesting; | ||
| import com.google.common.base.Strings; | ||
| import com.google.common.hash.HashFunction; | ||
| import com.google.common.hash.Hashing; | ||
|
|
@@ -75,10 +76,12 @@ final class BuiltInMetricsProvider { | |
| private static final String default_location = "global"; | ||
|
|
||
| private OpenTelemetry openTelemetry; | ||
| private String projectId; | ||
| private boolean mismatchedProjectIdLogged; | ||
|
|
||
| private BuiltInMetricsProvider() {} | ||
|
|
||
| OpenTelemetry getOrCreateOpenTelemetry( | ||
| synchronized OpenTelemetry getOrCreateOpenTelemetry( | ||
| String projectId, | ||
| @Nullable Credentials credentials, | ||
| @Nullable String monitoringHost, | ||
|
|
@@ -88,7 +91,7 @@ OpenTelemetry getOrCreateOpenTelemetry( | |
| SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); | ||
| BuiltInMetricsView.registerBuiltinMetrics( | ||
| SpannerCloudMonitoringExporter.create( | ||
| projectId, credentials, monitoringHost, universeDomain), | ||
| this::getProjectId, credentials, monitoringHost, universeDomain), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are also passing the projectId in next line to create OpenTelemetry Resource
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also do monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId);So the resource created during SDK initialization may contain the early/default project, but before sending |
||
| sdkMeterProviderBuilder); | ||
| sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId))); | ||
| SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); | ||
|
|
@@ -106,6 +109,39 @@ OpenTelemetry getOrCreateOpenTelemetry( | |
| } | ||
| } | ||
|
|
||
| synchronized void setProjectIdIfAbsent(String projectId) { | ||
| if (this.projectId == null) { | ||
| this.projectId = projectId; | ||
| } else if (!this.projectId.equals(projectId) && !mismatchedProjectIdLogged) { | ||
| mismatchedProjectIdLogged = true; | ||
| logger.log( | ||
| Level.WARNING, | ||
| "Built-in metrics are already initialized for project {0}. Metrics for project {1} will" | ||
| + " be exported using the existing project.", | ||
| new Object[] {this.projectId, projectId}); | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| synchronized OpenTelemetry getOpenTelemetry() { | ||
| return this.openTelemetry; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| synchronized String getProjectId() { | ||
| return this.projectId; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| synchronized void reset() { | ||
| if (this.openTelemetry instanceof OpenTelemetrySdk) { | ||
| ((OpenTelemetrySdk) this.openTelemetry).getSdkMeterProvider().close(); | ||
| } | ||
| this.openTelemetry = null; | ||
| this.projectId = null; | ||
| this.mismatchedProjectIdLogged = false; | ||
| } | ||
|
|
||
| // TODO: Remove when | ||
| // https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/issues/421 | ||
| // has been fixed. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rahul2393 I did not understand this solution. getOrCreateOpenTelemetry is called from GapicSpannerRPC while creating SpannerClient. At the time
projectIdshared here could be the projectId of GKE instance for example.So in this case we will be initialising SpannerCloudMonitoringExporter with null projectId ? As by this time
setProjectIdIfAbsentwon't be called, it is called later during database init.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So flow is:
SpannerClientinit → OpenTelemetry/exporter may be created, project supplier returnsnullgetDatabaseClient(DatabaseId)→ database project is set once