Skip to content

Commit 3ef313c

Browse files
rahul2393rahul2393
committed
fix(spanner): derive built-in metrics project from database client
1 parent c8234cf commit 3ef313c

8 files changed

Lines changed: 170 additions & 41 deletions

File tree

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsProvider.java‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.cloud.opentelemetry.detection.AttributeKeys;
3333
import com.google.cloud.opentelemetry.detection.DetectedPlatform;
3434
import com.google.cloud.opentelemetry.detection.GCPPlatformDetector;
35+
import com.google.common.annotations.VisibleForTesting;
3536
import com.google.common.base.Strings;
3637
import com.google.common.hash.HashFunction;
3738
import com.google.common.hash.Hashing;
@@ -75,10 +76,12 @@ final class BuiltInMetricsProvider {
7576
private static final String default_location = "global";
7677

7778
private OpenTelemetry openTelemetry;
79+
private String projectId;
80+
private boolean mismatchedProjectIdLogged;
7881

7982
private BuiltInMetricsProvider() {}
8083

81-
OpenTelemetry getOrCreateOpenTelemetry(
84+
synchronized OpenTelemetry getOrCreateOpenTelemetry(
8285
String projectId,
8386
@Nullable Credentials credentials,
8487
@Nullable String monitoringHost,
@@ -88,7 +91,7 @@ OpenTelemetry getOrCreateOpenTelemetry(
8891
SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder();
8992
BuiltInMetricsView.registerBuiltinMetrics(
9093
SpannerCloudMonitoringExporter.create(
91-
projectId, credentials, monitoringHost, universeDomain),
94+
this::getProjectId, credentials, monitoringHost, universeDomain),
9295
sdkMeterProviderBuilder);
9396
sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId)));
9497
SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build();
@@ -106,6 +109,39 @@ OpenTelemetry getOrCreateOpenTelemetry(
106109
}
107110
}
108111

112+
synchronized void setProjectIdIfAbsent(String projectId) {
113+
if (this.projectId == null) {
114+
this.projectId = projectId;
115+
} else if (!this.projectId.equals(projectId) && !mismatchedProjectIdLogged) {
116+
mismatchedProjectIdLogged = true;
117+
logger.log(
118+
Level.WARNING,
119+
"Built-in metrics are already initialized for project {0}. Metrics for project {1} will"
120+
+ " be exported using the existing project.",
121+
new Object[] {this.projectId, projectId});
122+
}
123+
}
124+
125+
@Nullable
126+
synchronized OpenTelemetry getOpenTelemetry() {
127+
return this.openTelemetry;
128+
}
129+
130+
@VisibleForTesting
131+
synchronized String getProjectId() {
132+
return this.projectId;
133+
}
134+
135+
@VisibleForTesting
136+
synchronized void reset() {
137+
if (this.openTelemetry instanceof OpenTelemetrySdk) {
138+
((OpenTelemetrySdk) this.openTelemetry).getSdkMeterProvider().close();
139+
}
140+
this.openTelemetry = null;
141+
this.projectId = null;
142+
this.mismatchedProjectIdLogged = false;
143+
}
144+
109145
// TODO: Remove when
110146
// https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/issues/421
111147
// has been fixed.

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java‎

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,15 @@
4242
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
4343
import io.opentelemetry.sdk.metrics.data.MetricData;
4444
import io.opentelemetry.sdk.metrics.export.MetricExporter;
45-
import io.opentelemetry.sdk.resources.Resource;
4645
import java.io.IOException;
4746
import java.time.Duration;
4847
import java.util.ArrayList;
4948
import java.util.Collection;
5049
import java.util.List;
5150
import java.util.concurrent.atomic.AtomicBoolean;
51+
import java.util.function.Supplier;
5252
import java.util.logging.Level;
5353
import java.util.logging.Logger;
54-
import java.util.stream.Collectors;
5554
import javax.annotation.Nonnull;
5655
import javax.annotation.Nullable;
5756

@@ -72,14 +71,23 @@ class SpannerCloudMonitoringExporter implements MetricExporter {
7271
private final AtomicBoolean spannerExportFailureLogged = new AtomicBoolean(false);
7372
private final AtomicBoolean lastExportSkippedData = new AtomicBoolean(false);
7473
private final MetricServiceClient client;
75-
private final String spannerProjectId;
74+
private final Supplier<String> spannerProjectIdSupplier;
7675

7776
static SpannerCloudMonitoringExporter create(
7877
String projectId,
7978
@Nullable Credentials credentials,
8079
@Nullable String monitoringHost,
8180
String universeDomain)
8281
throws IOException {
82+
return create(() -> projectId, credentials, monitoringHost, universeDomain);
83+
}
84+
85+
static SpannerCloudMonitoringExporter create(
86+
Supplier<String> projectIdSupplier,
87+
@Nullable Credentials credentials,
88+
@Nullable String monitoringHost,
89+
String universeDomain)
90+
throws IOException {
8391
MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder();
8492
CredentialsProvider credentialsProvider;
8593
if (credentials == null || credentials instanceof NoCredentials) {
@@ -114,13 +122,18 @@ static SpannerCloudMonitoringExporter create(
114122
settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout);
115123

116124
return new SpannerCloudMonitoringExporter(
117-
projectId, MetricServiceClient.create(settingsBuilder.build()));
125+
projectIdSupplier, MetricServiceClient.create(settingsBuilder.build()));
118126
}
119127

120128
@VisibleForTesting
121129
SpannerCloudMonitoringExporter(String projectId, MetricServiceClient client) {
130+
this(() -> projectId, client);
131+
}
132+
133+
@VisibleForTesting
134+
SpannerCloudMonitoringExporter(Supplier<String> projectIdSupplier, MetricServiceClient client) {
122135
this.client = client;
123-
this.spannerProjectId = projectId;
136+
this.spannerProjectIdSupplier = projectIdSupplier;
124137
}
125138

126139
@Override
@@ -140,37 +153,22 @@ MetricServiceClient getMetricServiceClient() {
140153

141154
/** Export client built in metrics */
142155
private CompletableResultCode exportSpannerClientMetrics(Collection<MetricData> collection) {
143-
// Filter spanner metrics. Only include metrics that contain a valid project.
144-
List<MetricData> spannerMetricData = collection.stream().collect(Collectors.toList());
145-
146-
// Log warnings for metrics that will be skipped.
147-
boolean mustFilter = false;
148-
if (spannerMetricData.stream()
149-
.map(metricData -> metricData.getResource())
150-
.anyMatch(this::shouldSkipPointDataDueToProjectId)) {
151-
logger.log(
152-
Level.WARNING, "Some metric data contain a different projectId. These will be skipped.");
153-
mustFilter = true;
154-
}
155-
156-
if (mustFilter) {
157-
spannerMetricData =
158-
spannerMetricData.stream()
159-
.filter(this::shouldSkipMetricData)
160-
.collect(Collectors.toList());
156+
String spannerProjectId = spannerProjectIdSupplier.get();
157+
if (Strings.isNullOrEmpty(spannerProjectId)) {
158+
return CompletableResultCode.ofSuccess();
161159
}
162-
lastExportSkippedData.set(mustFilter);
160+
lastExportSkippedData.set(false);
163161

164162
// Skips exporting if there's none
165-
if (spannerMetricData.isEmpty()) {
163+
if (collection.isEmpty()) {
166164
return CompletableResultCode.ofSuccess();
167165
}
168166

169167
List<TimeSeries> spannerTimeSeries;
170168
try {
171169
spannerTimeSeries =
172170
SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries(
173-
spannerMetricData, this.spannerProjectId);
171+
new ArrayList<>(collection), spannerProjectId);
174172
} catch (Throwable e) {
175173
logger.log(
176174
Level.WARNING,
@@ -218,14 +216,6 @@ public void onSuccess(List<Empty> empty) {
218216
return spannerExportCode;
219217
}
220218

221-
private boolean shouldSkipMetricData(MetricData metricData) {
222-
return shouldSkipPointDataDueToProjectId(metricData.getResource());
223-
}
224-
225-
private boolean shouldSkipPointDataDueToProjectId(Resource resource) {
226-
return !spannerProjectId.equals(SpannerCloudMonitoringExporterUtils.getProjectId(resource));
227-
}
228-
229219
boolean lastExportSkippedData() {
230220
return this.lastExportSkippedData.get();
231221
}

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import io.opentelemetry.sdk.metrics.data.MetricDataType;
6262
import io.opentelemetry.sdk.metrics.data.PointData;
6363
import io.opentelemetry.sdk.metrics.data.SumData;
64-
import io.opentelemetry.sdk.resources.Resource;
6564
import java.util.ArrayList;
6665
import java.util.List;
6766
import java.util.logging.Level;
@@ -75,10 +74,6 @@ class SpannerCloudMonitoringExporterUtils {
7574

7675
private SpannerCloudMonitoringExporterUtils() {}
7776

78-
static String getProjectId(Resource resource) {
79-
return resource.getAttributes().get(PROJECT_ID_KEY);
80-
}
81-
8277
static List<TimeSeries> convertToSpannerTimeSeries(
8378
List<MetricData> collection, String projectId) {
8479
List<TimeSeries> allTimeSeries = new ArrayList<>();
@@ -102,6 +97,7 @@ static List<TimeSeries> convertToSpannerTimeSeries(
10297
monitoredResourceBuilder.putLabels(
10398
key.getKey(), String.valueOf(resourceAttributes.get(key)));
10499
}
100+
monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId);
105101

106102
metricData.getData().getPoints().stream()
107103
.map(

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ public DatabaseClient getDatabaseClient(DatabaseId db) {
309309
if (clientId == null) {
310310
clientId = nextDatabaseClientId(db);
311311
}
312+
getOptions().initializeBuiltInMetrics(db);
312313
MultiplexedSessionDatabaseClient multiplexedSessionDatabaseClient =
313314
new MultiplexedSessionDatabaseClient(SpannerImpl.this.getSessionClient(db));
314315
DatabaseClientImpl dbClient =
@@ -337,6 +338,7 @@ public BatchClient getBatchClient(DatabaseId db) {
337338
if (this.dbBatchClients.containsKey(db)) {
338339
return this.dbBatchClients.get(db);
339340
}
341+
getOptions().initializeBuiltInMetrics(db);
340342
BatchClientImpl batchClient = new BatchClientImpl(getSessionClient(db));
341343
this.dbBatchClients.put(db, batchClient);
342344
return batchClient;

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,6 +2520,17 @@ public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelPr
25202520
}
25212521
}
25222522

2523+
void initializeBuiltInMetrics(DatabaseId databaseId) {
2524+
if (isEnableBuiltInMetrics() && !usesNoCredentials()) {
2525+
this.builtInMetricsProvider.setProjectIdIfAbsent(databaseId.getInstanceId().getProject());
2526+
this.builtInMetricsProvider.getOrCreateOpenTelemetry(
2527+
databaseId.getInstanceId().getProject(),
2528+
getCredentials(),
2529+
this.monitoringHost,
2530+
getUniverseDomain());
2531+
}
2532+
}
2533+
25232534
public ApiTracerFactory getApiTracerFactory(boolean isAdminClient, boolean isEmulatorEnabled) {
25242535
return createApiTracerFactory(isAdminClient, isEmulatorEnabled);
25252536
}

‎java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProviderTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,31 @@
1717
package com.google.cloud.spanner;
1818

1919
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNull;
2021
import static org.junit.Assert.assertTrue;
2122

23+
import com.google.auth.oauth2.AccessToken;
24+
import com.google.auth.oauth2.OAuth2Credentials;
25+
import java.util.Date;
26+
import org.junit.After;
27+
import org.junit.Before;
2228
import org.junit.Test;
2329
import org.junit.runner.RunWith;
2430
import org.junit.runners.JUnit4;
2531

2632
@RunWith(JUnit4.class)
2733
public class BuiltInOpenTelemetryMetricsProviderTest {
2834

35+
@Before
36+
public void setUp() {
37+
BuiltInMetricsProvider.INSTANCE.reset();
38+
}
39+
40+
@After
41+
public void tearDown() {
42+
BuiltInMetricsProvider.INSTANCE.reset();
43+
}
44+
2945
@Test
3046
public void testGenerateClientHashWithSimpleUid() {
3147
String clientUid = "testClient";
@@ -56,11 +72,46 @@ public void testGenerateClientHashWithSpecialCharacters() {
5672
verifyHash(BuiltInMetricsProvider.generateClientHash(clientUid));
5773
}
5874

75+
@Test
76+
public void testApiTracerFactoryDoesNotSetBuiltInMetricsProject() {
77+
SpannerOptions options = newTestOptions();
78+
79+
options.getApiTracerFactory(/* isAdminClient= */ false, /* isEmulatorEnabled= */ false);
80+
81+
assertNull(BuiltInMetricsProvider.INSTANCE.getProjectId());
82+
}
83+
84+
@Test
85+
public void testBuiltInOpenTelemetryDoesNotSetMetricsProject() {
86+
SpannerOptions options = newTestOptions();
87+
88+
options.getBuiltInOpenTelemetry();
89+
90+
assertNull(BuiltInMetricsProvider.INSTANCE.getProjectId());
91+
}
92+
93+
@Test
94+
public void testInitializeBuiltInMetricsUsesDatabaseProject() {
95+
SpannerOptions options = newTestOptions();
96+
97+
options.initializeBuiltInMetrics(DatabaseId.of("database-project", "i", "d"));
98+
99+
assertEquals("database-project", BuiltInMetricsProvider.INSTANCE.getProjectId());
100+
}
101+
59102
private void verifyHash(String hash) {
60103
// Check if the hash length is 6
61104
assertEquals(hash.length(), 6);
62105
// Check if the hash is in the range [000000, 0003ff]
63106
long hashValue = Long.parseLong(hash, 16); // Convert hash from hex to decimal
64107
assertTrue(hashValue >= 0 && hashValue <= 0x3FF);
65108
}
109+
110+
private SpannerOptions newTestOptions() {
111+
return SpannerOptions.newBuilder()
112+
.setProjectId("host-project")
113+
.setCredentials(
114+
OAuth2Credentials.create(new AccessToken("test-token", new Date(Long.MAX_VALUE))))
115+
.build();
116+
}
66117
}

‎java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,47 @@ public void testExportingSumData() {
202202
assertThat(timeSeries.getPoints(0).getInterval().getEndTime().getNanos()).isEqualTo(endEpoch);
203203
}
204204

205+
@Test
206+
public void testExportingOverridesResourceProject() {
207+
ArgumentCaptor<CreateTimeSeriesRequest> argumentCaptor =
208+
ArgumentCaptor.forClass(CreateTimeSeriesRequest.class);
209+
210+
UnaryCallable<CreateTimeSeriesRequest, Empty> mockCallable = Mockito.mock(UnaryCallable.class);
211+
Mockito.when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable);
212+
ApiFuture<Empty> future = ApiFutures.immediateFuture(Empty.getDefaultInstance());
213+
Mockito.when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future);
214+
215+
Resource resourceWithDifferentProject =
216+
Resource.create(
217+
resourceAttributes.toBuilder().put(PROJECT_ID_KEY, "resource-project").build());
218+
LongPointData longPointData = ImmutableLongPointData.create(10, 15, attributes, 11L);
219+
MetricData longData =
220+
ImmutableMetricData.createLongSum(
221+
resourceWithDifferentProject,
222+
scope,
223+
"spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME,
224+
"description",
225+
"1",
226+
ImmutableSumData.create(
227+
true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData)));
228+
229+
exporter.export(Collections.singletonList(longData));
230+
231+
CreateTimeSeriesRequest request = argumentCaptor.getValue();
232+
assertThat(request.getName()).isEqualTo("projects/" + projectId);
233+
assertThat(request.getTimeSeries(0).getResource().getLabelsMap())
234+
.containsEntry(PROJECT_ID_KEY.getKey(), projectId);
235+
}
236+
237+
@Test
238+
public void testExportingSkipsUntilProjectIsSet() {
239+
exporter = new SpannerCloudMonitoringExporter(() -> null, fakeMetricServiceClient);
240+
241+
exporter.export(Collections.emptyList());
242+
243+
Mockito.verify(mockMetricServiceStub, Mockito.never()).createServiceTimeSeriesCallable();
244+
}
245+
205246
@Test
206247
public void testExportingHistogramData() {
207248
ArgumentCaptor<CreateTimeSeriesRequest> argumentCaptor =

‎java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.hamcrest.MatcherAssert.assertThat;
2222
import static org.junit.Assert.assertNotNull;
2323
import static org.junit.Assert.assertThrows;
24+
import static org.mockito.Mockito.verify;
2425
import static org.mockito.Mockito.when;
2526

2627
import com.google.api.core.NanoClock;
@@ -113,6 +114,7 @@ public void getDbclientAgainGivesSame() {
113114
DatabaseClient databaseClient1 = impl.getDatabaseClient(db);
114115

115116
assertThat(databaseClient1).isSameInstanceAs(databaseClient);
117+
verify(spannerOptions).initializeBuiltInMetrics(db);
116118
}
117119

118120
@Test

0 commit comments

Comments
 (0)