Skip to content

Commit 592d47d

Browse files
rahul2393rahul2393
committed
incorporate suggestions
1 parent 52234f4 commit 592d47d

8 files changed

Lines changed: 297 additions & 72 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ public class BuiltInMetricsConstant {
144144
ImmutableList.of(EEF_FALLBACK_COUNT_NAME, EEF_CALL_STATUS_NAME);
145145

146146
public static final String SPANNER_RESOURCE_TYPE = "spanner_instance_client";
147+
public static final String UNDEFINED_PROJECT_ID = "undefined-project";
147148

148149
public static final AttributeKey<String> PROJECT_ID_KEY = AttributeKey.stringKey("project_id");
149150
public static final AttributeKey<String> INSTANCE_ID_KEY = AttributeKey.stringKey("instance_id");

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ final class BuiltInMetricsProvider {
7878
private OpenTelemetry openTelemetry;
7979
private String projectId;
8080
private boolean mismatchedProjectIdLogged;
81+
private Thread shutdownHook;
8182

8283
private BuiltInMetricsProvider() {}
8384

@@ -96,7 +97,8 @@ synchronized OpenTelemetry getOrCreateOpenTelemetry(
9697
sdkMeterProviderBuilder.setResource(Resource.create(createResourceAttributes(projectId)));
9798
SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build();
9899
this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build();
99-
Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close));
100+
this.shutdownHook = new Thread(sdkMeterProvider::close);
101+
Runtime.getRuntime().addShutdownHook(this.shutdownHook);
100102
}
101103
return this.openTelemetry;
102104
} catch (IOException ex) {
@@ -116,8 +118,9 @@ synchronized void setProjectIdIfAbsent(String projectId) {
116118
mismatchedProjectIdLogged = true;
117119
logger.log(
118120
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+
"Built-in metrics fallback project is already initialized to project {0}. Non-Spanner"
122+
+ " metrics without project information will be exported using that project instead"
123+
+ " of project {1}.",
121124
new Object[] {this.projectId, projectId});
122125
}
123126
}
@@ -127,7 +130,6 @@ synchronized OpenTelemetry getOpenTelemetry() {
127130
return this.openTelemetry;
128131
}
129132

130-
@VisibleForTesting
131133
synchronized String getProjectId() {
132134
return this.projectId;
133135
}
@@ -137,9 +139,17 @@ synchronized void reset() {
137139
if (this.openTelemetry instanceof OpenTelemetrySdk) {
138140
((OpenTelemetrySdk) this.openTelemetry).getSdkMeterProvider().close();
139141
}
142+
if (this.shutdownHook != null) {
143+
try {
144+
Runtime.getRuntime().removeShutdownHook(this.shutdownHook);
145+
} catch (IllegalStateException ignored) {
146+
// The JVM is already shutting down.
147+
}
148+
}
140149
this.openTelemetry = null;
141150
this.projectId = null;
142151
this.mismatchedProjectIdLogged = false;
152+
this.shutdownHook = null;
143153
}
144154

145155
// TODO: Remove when

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

Lines changed: 74 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@
4747
import java.util.ArrayList;
4848
import java.util.Collection;
4949
import java.util.List;
50-
import java.util.concurrent.atomic.AtomicBoolean;
50+
import java.util.Map;
51+
import java.util.Set;
52+
import java.util.concurrent.ConcurrentHashMap;
5153
import java.util.function.Supplier;
5254
import java.util.logging.Level;
5355
import java.util.logging.Logger;
56+
import java.util.stream.Collectors;
5457
import javax.annotation.Nonnull;
5558
import javax.annotation.Nullable;
5659

@@ -68,21 +71,12 @@ class SpannerCloudMonitoringExporter implements MetricExporter {
6871
// This the quota limit from Cloud Monitoring. More details in
6972
// https://cloud.google.com/monitoring/quotas#custom_metrics_quotas.
7073
private static final int EXPORT_BATCH_SIZE_LIMIT = 200;
71-
private final AtomicBoolean spannerExportFailureLogged = new AtomicBoolean(false);
74+
private final Set<String> spannerExportFailureLoggedProjects = ConcurrentHashMap.newKeySet();
7275
private final MetricServiceClient client;
73-
private final Supplier<String> spannerProjectIdSupplier;
76+
private final Supplier<String> fallbackProjectIdSupplier;
7477

7578
static SpannerCloudMonitoringExporter create(
76-
String projectId,
77-
@Nullable Credentials credentials,
78-
@Nullable String monitoringHost,
79-
String universeDomain)
80-
throws IOException {
81-
return create(() -> projectId, credentials, monitoringHost, universeDomain);
82-
}
83-
84-
static SpannerCloudMonitoringExporter create(
85-
Supplier<String> projectIdSupplier,
79+
Supplier<String> fallbackProjectIdSupplier,
8680
@Nullable Credentials credentials,
8781
@Nullable String monitoringHost,
8882
String universeDomain)
@@ -121,18 +115,19 @@ static SpannerCloudMonitoringExporter create(
121115
settingsBuilder.createServiceTimeSeriesSettings().setSimpleTimeoutNoRetriesDuration(timeout);
122116

123117
return new SpannerCloudMonitoringExporter(
124-
projectIdSupplier, MetricServiceClient.create(settingsBuilder.build()));
118+
fallbackProjectIdSupplier, MetricServiceClient.create(settingsBuilder.build()));
125119
}
126120

127121
@VisibleForTesting
128-
SpannerCloudMonitoringExporter(String projectId, MetricServiceClient client) {
129-
this(() -> projectId, client);
122+
SpannerCloudMonitoringExporter(MetricServiceClient client) {
123+
this(() -> null, client);
130124
}
131125

132126
@VisibleForTesting
133-
SpannerCloudMonitoringExporter(Supplier<String> projectIdSupplier, MetricServiceClient client) {
127+
SpannerCloudMonitoringExporter(
128+
Supplier<String> fallbackProjectIdSupplier, MetricServiceClient client) {
134129
this.client = client;
135-
this.spannerProjectIdSupplier = projectIdSupplier;
130+
this.fallbackProjectIdSupplier = fallbackProjectIdSupplier;
136131
}
137132

138133
@Override
@@ -152,10 +147,6 @@ MetricServiceClient getMetricServiceClient() {
152147

153148
/** Export client built in metrics */
154149
private CompletableResultCode exportSpannerClientMetrics(Collection<MetricData> collection) {
155-
String spannerProjectId = spannerProjectIdSupplier.get();
156-
if (Strings.isNullOrEmpty(spannerProjectId)) {
157-
return CompletableResultCode.ofSuccess();
158-
}
159150
// Skips exporting if there's none
160151
if (collection.isEmpty()) {
161152
return CompletableResultCode.ofSuccess();
@@ -165,7 +156,7 @@ private CompletableResultCode exportSpannerClientMetrics(Collection<MetricData>
165156
try {
166157
spannerTimeSeries =
167158
SpannerCloudMonitoringExporterUtils.convertToSpannerTimeSeries(
168-
collection, spannerProjectId);
159+
collection, fallbackProjectIdSupplier.get());
169160
} catch (Throwable e) {
170161
logger.log(
171162
Level.WARNING,
@@ -174,37 +165,60 @@ private CompletableResultCode exportSpannerClientMetrics(Collection<MetricData>
174165
return CompletableResultCode.ofFailure();
175166
}
176167

177-
ProjectName projectName = ProjectName.of(spannerProjectId);
168+
if (spannerTimeSeries.isEmpty()) {
169+
return CompletableResultCode.ofSuccess();
170+
}
178171

179-
ApiFuture<List<Empty>> futureList = exportTimeSeriesInBatch(projectName, spannerTimeSeries);
172+
Map<String, List<TimeSeries>> timeSeriesByProject =
173+
spannerTimeSeries.stream()
174+
.collect(
175+
Collectors.groupingBy(
176+
timeSeries ->
177+
timeSeries
178+
.getResource()
179+
.getLabelsMap()
180+
.get(BuiltInMetricsConstant.PROJECT_ID_KEY.getKey())));
181+
182+
List<ApiFuture<List<Empty>>> futures = new ArrayList<>();
183+
for (Map.Entry<String, List<TimeSeries>> entry : timeSeriesByProject.entrySet()) {
184+
ProjectName projectName = ProjectName.of(entry.getKey());
185+
ApiFuture<List<Empty>> future = exportTimeSeriesInBatch(projectName, entry.getValue());
186+
ApiFutures.addCallback(
187+
future,
188+
new ApiFutureCallback<List<Empty>>() {
189+
@Override
190+
public void onFailure(Throwable throwable) {
191+
logExportFailure(throwable, projectName);
192+
}
193+
194+
@Override
195+
public void onSuccess(List<Empty> ignored) {
196+
spannerExportFailureLoggedProjects.remove(projectName.getProject());
197+
}
198+
},
199+
MoreExecutors.directExecutor());
200+
futures.add(future);
201+
}
202+
203+
ApiFuture<List<List<Empty>>> groupedFuture = ApiFutures.allAsList(futures);
204+
ApiFuture<List<Empty>> futureList =
205+
ApiFutures.transform(
206+
groupedFuture,
207+
groupedResults ->
208+
groupedResults.stream().flatMap(List::stream).collect(Collectors.toList()),
209+
MoreExecutors.directExecutor());
180210

181211
CompletableResultCode spannerExportCode = new CompletableResultCode();
182212
ApiFutures.addCallback(
183213
futureList,
184214
new ApiFutureCallback<List<Empty>>() {
185215
@Override
186216
public void onFailure(Throwable throwable) {
187-
if (spannerExportFailureLogged.compareAndSet(false, true)) {
188-
String msg = "createServiceTimeSeries request failed for spanner metrics.";
189-
if (throwable instanceof PermissionDeniedException) {
190-
// TODO: Add the link of public documentation when available in the log message.
191-
msg +=
192-
String.format(
193-
" Need monitoring metric writer permission on project=%s. Follow"
194-
+ " https://cloud.google.com/spanner/docs/view-manage-client-side-metrics#access-client-side-metrics"
195-
+ " to set up permissions",
196-
projectName.getProject());
197-
}
198-
logger.log(Level.WARNING, msg, throwable);
199-
}
200217
spannerExportCode.fail();
201218
}
202219

203220
@Override
204221
public void onSuccess(List<Empty> empty) {
205-
// When an export succeeded reset the export failure flag to false so if there's a
206-
// transient failure it'll be logged.
207-
spannerExportFailureLogged.set(false);
208222
spannerExportCode.succeed();
209223
}
210224
},
@@ -213,6 +227,25 @@ public void onSuccess(List<Empty> empty) {
213227
return spannerExportCode;
214228
}
215229

230+
private void logExportFailure(Throwable throwable, ProjectName projectName) {
231+
if (spannerExportFailureLoggedProjects.add(projectName.getProject())) {
232+
String msg = "createServiceTimeSeries request failed for spanner metrics.";
233+
if (throwable instanceof PermissionDeniedException) {
234+
// TODO: Add the link of public documentation when available in the log message.
235+
msg +=
236+
String.format(
237+
" Need monitoring metric writer permission on project=%s. Follow"
238+
+ " https://cloud.google.com/spanner/docs/view-manage-client-side-metrics"
239+
+ "#access-client-side-metrics"
240+
+ " to set up permissions",
241+
projectName.getProject());
242+
} else {
243+
msg += String.format(" project=%s.", projectName.getProject());
244+
}
245+
logger.log(Level.WARNING, msg, throwable);
246+
}
247+
}
248+
216249
private ApiFuture<List<Empty>> exportTimeSeriesInBatch(
217250
ProjectName projectName, List<TimeSeries> timeSeries) {
218251
List<ApiFuture<Empty>> batchResults = new ArrayList<>();

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

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_METER_NAME;
3131
import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_PROMOTED_RESOURCE_LABELS;
3232
import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_RESOURCE_TYPE;
33+
import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID;
3334

3435
import com.google.api.Distribution;
3536
import com.google.api.Distribution.BucketOptions;
@@ -64,9 +65,11 @@
6465
import java.util.ArrayList;
6566
import java.util.Collection;
6667
import java.util.List;
68+
import java.util.Objects;
6769
import java.util.logging.Level;
6870
import java.util.logging.Logger;
6971
import java.util.stream.Collectors;
72+
import javax.annotation.Nullable;
7073

7174
class SpannerCloudMonitoringExporterUtils {
7275

@@ -76,7 +79,7 @@ class SpannerCloudMonitoringExporterUtils {
7679
private SpannerCloudMonitoringExporterUtils() {}
7780

7881
static List<TimeSeries> convertToSpannerTimeSeries(
79-
Collection<MetricData> collection, String projectId) {
82+
Collection<MetricData> collection, String fallbackProjectId) {
8083
List<TimeSeries> allTimeSeries = new ArrayList<>();
8184

8285
for (MetricData metricData : collection) {
@@ -95,26 +98,30 @@ static List<TimeSeries> convertToSpannerTimeSeries(
9598

9699
Attributes resourceAttributes = metricData.getResource().getAttributes();
97100
for (AttributeKey<?> key : resourceAttributes.asMap().keySet()) {
98-
monitoredResourceBuilder.putLabels(
99-
key.getKey(), String.valueOf(resourceAttributes.get(key)));
101+
if (!PROJECT_ID_KEY.equals(key)) {
102+
monitoredResourceBuilder.putLabels(
103+
key.getKey(), String.valueOf(resourceAttributes.get(key)));
104+
}
100105
}
101-
monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId);
106+
MonitoredResource baseMonitoredResource = monitoredResourceBuilder.build();
102107

103108
metricData.getData().getPoints().stream()
104109
.map(
105110
pointData ->
106111
convertPointToSpannerTimeSeries(
107-
metricData, pointData, monitoredResourceBuilder, projectId))
112+
metricData, pointData, baseMonitoredResource, fallbackProjectId))
113+
.filter(Objects::nonNull)
108114
.forEach(allTimeSeries::add);
109115
}
110116
return allTimeSeries;
111117
}
112118

119+
@Nullable
113120
private static TimeSeries convertPointToSpannerTimeSeries(
114121
MetricData metricData,
115122
PointData pointData,
116-
MonitoredResource.Builder monitoredResourceBuilder,
117-
String projectId) {
123+
MonitoredResource baseMonitoredResource,
124+
String fallbackProjectId) {
118125
MetricKind metricKind = convertMetricKind(metricData);
119126
TimeSeries.Builder builder =
120127
TimeSeries.newBuilder()
@@ -123,9 +130,20 @@ private static TimeSeries convertPointToSpannerTimeSeries(
123130
Metric.Builder metricBuilder = Metric.newBuilder().setType(metricData.getName());
124131

125132
Attributes attributes = pointData.getAttributes();
133+
String projectId = attributes.get(PROJECT_ID_KEY);
134+
if (!isUsableProjectId(projectId)) {
135+
projectId = shouldUseFallbackProject(metricData) ? fallbackProjectId : null;
136+
}
137+
if (!isUsableProjectId(projectId)) {
138+
return null;
139+
}
140+
MonitoredResource.Builder monitoredResourceBuilder = baseMonitoredResource.toBuilder();
141+
monitoredResourceBuilder.putLabels(PROJECT_ID_KEY.getKey(), projectId);
126142

127143
for (AttributeKey<?> key : attributes.asMap().keySet()) {
128-
if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) {
144+
if (PROJECT_ID_KEY.equals(key)) {
145+
continue;
146+
} else if (SPANNER_PROMOTED_RESOURCE_LABELS.contains(key)) {
129147
monitoredResourceBuilder.putLabels(key.getKey(), String.valueOf(attributes.get(key)));
130148
} else {
131149
// Replace metric label names by converting "." to "_" since Cloud Monitoring does not
@@ -156,6 +174,15 @@ private static TimeSeries convertPointToSpannerTimeSeries(
156174
return builder.build();
157175
}
158176

177+
private static boolean isUsableProjectId(String projectId) {
178+
return projectId != null && !projectId.isEmpty() && !UNDEFINED_PROJECT_ID.equals(projectId);
179+
}
180+
181+
private static boolean shouldUseFallbackProject(MetricData metricData) {
182+
String meterName = metricData.getInstrumentationScopeInfo().getName();
183+
return GRPC_METER_NAME.equals(meterName) || GRPC_GCP_METER_NAME.equals(meterName);
184+
}
185+
159186
private static MetricKind convertMetricKind(MetricData metricData) {
160187
switch (metricData.getType()) {
161188
case HISTOGRAM:

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2523,11 +2523,6 @@ public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelPr
25232523
void initializeBuiltInMetrics(DatabaseId databaseId) {
25242524
if (isEnableBuiltInMetrics() && !usesNoCredentials()) {
25252525
this.builtInMetricsProvider.setProjectIdIfAbsent(databaseId.getInstanceId().getProject());
2526-
this.builtInMetricsProvider.getOrCreateOpenTelemetry(
2527-
databaseId.getInstanceId().getProject(),
2528-
getCredentials(),
2529-
this.monitoringHost,
2530-
getUniverseDomain());
25312526
}
25322527
}
25332528

‎java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.cloud.spanner.spi.v1;
1717

1818
import static com.google.api.gax.grpc.GrpcCallContext.TRACER_KEY;
19+
import static com.google.cloud.spanner.BuiltInMetricsConstant.UNDEFINED_PROJECT_ID;
1920
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.DATABASE_ID;
2021
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.INSTANCE_ID;
2122
import static com.google.cloud.spanner.spi.v1.SpannerRpcViews.METHOD;
@@ -268,7 +269,7 @@ private DatabaseName extractDatabaseName(Metadata headers) throws ExecutionExcep
268269
return databaseNameCache.get(
269270
googleResourcePrefix,
270271
() -> {
271-
String projectId = "undefined-project";
272+
String projectId = UNDEFINED_PROJECT_ID;
272273
String instanceId = "undefined-database";
273274
String databaseId = "undefined-database";
274275
Matcher matcher = GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN.matcher(googleResourcePrefix);
@@ -329,6 +330,10 @@ private Map<String, String> getBuiltInMetricAttributes(String key, DatabaseName
329330
key,
330331
() -> {
331332
Map<String, String> attributes = new HashMap<>();
333+
if (!UNDEFINED_PROJECT_ID.equals(databaseName.getProject())) {
334+
attributes.put(
335+
BuiltInMetricsConstant.PROJECT_ID_KEY.getKey(), databaseName.getProject());
336+
}
332337
attributes.put(BuiltInMetricsConstant.DATABASE_KEY.getKey(), databaseName.getDatabase());
333338
attributes.put(
334339
BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance());

0 commit comments

Comments
 (0)