diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 51dc890902..3577c9f7b4 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -138,6 +138,14 @@ private CompletableResultCode exportSpannerClientMetrics(Collection return CompletableResultCode.ofFailure(); } + // Verifies if metrics data has missing instance id. + if (spannerMetricData.stream() + .flatMap(metricData -> metricData.getData().getPoints().stream()) + .anyMatch(pd -> SpannerCloudMonitoringExporterUtils.getInstanceId(pd) == null)) { + logger.log(Level.WARNING, "Metric data has missing instanceId. Skipping export."); + return CompletableResultCode.ofFailure(); + } + List spannerTimeSeries; try { spannerTimeSeries = @@ -166,7 +174,7 @@ public void onFailure(Throwable throwable) { // TODO: Add the link of public documentation when available in the log message. msg += String.format( - " Need monitoring metric writer permission on project=%s.", + " Need monitoring metric writer permission on project=%s. Follow https://cloud.google.com/spanner/docs/view-manage-client-side-metrics#access-client-side-metrics to set up permissions", projectName.getProject()); } logger.log(Level.WARNING, msg, throwable); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java index a6d1e29d58..21fcba8194 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterUtils.java @@ -23,6 +23,7 @@ import static com.google.api.MetricDescriptor.ValueType.DOUBLE; import static com.google.api.MetricDescriptor.ValueType.INT64; import static com.google.cloud.spanner.BuiltInMetricsConstant.GAX_METER_NAME; +import static com.google.cloud.spanner.BuiltInMetricsConstant.INSTANCE_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.PROJECT_ID_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_PROMOTED_RESOURCE_LABELS; import static com.google.cloud.spanner.BuiltInMetricsConstant.SPANNER_RESOURCE_TYPE; @@ -66,6 +67,10 @@ static String getProjectId(PointData pointData) { return pointData.getAttributes().get(PROJECT_ID_KEY); } + static String getInstanceId(PointData pointData) { + return pointData.getAttributes().get(INSTANCE_ID_KEY); + } + static List convertToSpannerTimeSeries(List collection) { List allTimeSeries = new ArrayList<>(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index db245d3af8..acb7ae9fa1 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner; +import static com.google.cloud.spanner.BuiltInMetricsConstant.ATTEMPT_COUNT_NAME; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_HASH_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_NAME_KEY; import static com.google.cloud.spanner.BuiltInMetricsConstant.CLIENT_UID_KEY; @@ -44,6 +45,7 @@ import com.google.monitoring.v3.TimeSeries; import com.google.protobuf.Empty; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.data.AggregationTemporality; @@ -340,6 +342,55 @@ public void getAggregationTemporality() throws IOException { .isEqualTo(AggregationTemporality.CUMULATIVE); } + @Test + public void testSkipExportingDataIfMissingInstanceId() throws IOException { + Attributes attributesWithoutInstanceId = + Attributes.builder().putAll(attributes).remove(INSTANCE_ID_KEY).build(); + + SpannerCloudMonitoringExporter actualExporter = + SpannerCloudMonitoringExporter.create(projectId, null); + assertThat(actualExporter.getAggregationTemporality(InstrumentType.COUNTER)) + .isEqualTo(AggregationTemporality.CUMULATIVE); + ArgumentCaptor argumentCaptor = + ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); + + UnaryCallable mockCallable = Mockito.mock(UnaryCallable.class); + Mockito.when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); + ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); + Mockito.when(mockCallable.futureCall(argumentCaptor.capture())).thenReturn(future); + + long fakeValue = 11L; + + long startEpoch = 10; + long endEpoch = 15; + LongPointData longPointData = + ImmutableLongPointData.create(startEpoch, endEpoch, attributesWithoutInstanceId, fakeValue); + + MetricData operationLongData = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + OPERATION_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + MetricData attemptLongData = + ImmutableMetricData.createLongSum( + resource, + scope, + "spanner.googleapis.com/internal/client/" + ATTEMPT_COUNT_NAME, + "description", + "1", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(longPointData))); + + CompletableResultCode resultCode = + exporter.export(Arrays.asList(operationLongData, attemptLongData)); + assertThat(resultCode).isEqualTo(CompletableResultCode.ofFailure()); + } + private static class FakeMetricServiceClient extends MetricServiceClient { protected FakeMetricServiceClient(MetricServiceStub stub) {