From 21407aa131fe9c7defffb79d6551d043e818b643 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 26 Feb 2024 12:04:25 -0500 Subject: [PATCH 01/24] feat: Implement OpentelemetryMetricsRecorder --- CONTRIBUTING.md | 8 +- gax-java/dependencies.properties | 1 + gax-java/gax/BUILD.bazel | 1 + gax-java/gax/pom.xml | 12 + .../google/api/gax/rpc/ClientSettings.java | 16 + .../google/api/gax/tracing/MetricsTracer.java | 3 + .../tracing/OpentelemetryMetricsRecorder.java | 109 ++++ .../api/gax/tracing/MetricsTracerTest.java | 24 +- .../OpentelemetryMetricsRecorderTest.java | 266 ++++++++ gax-java/pom.xml | 7 + showcase/gapic-showcase/pom.xml | 15 + .../showcase/v1beta1/it/ITOtelMetrics.java | 606 ++++++++++++++++++ .../it/util/TestClientInitializer.java | 36 ++ showcase/pom.xml | 8 +- 14 files changed, 1097 insertions(+), 15 deletions(-) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java create mode 100644 gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java create mode 100644 showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f2b357f2ab..22b241cb73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,11 +1,7 @@ # How to Contribute -We'd love to accept your patches and contributions to this project. There are just a few small guidelines you need to follow before opening an issue or a PR: -1. Ensure the issue was not already reported. -2. Open a new issue if you are unable to find an existing issue addressing your problem. Make sure to include a title and clear description, as much relevant information as possible, and a code sample or an executable test case demonstrating the expected behavior that is not occurring. -3. Discuss the priority and potential solutions with the maintainers in the issue. The maintainers would review the issue and add a label "Accepting Contributions" once the issue is ready for accepting contributions. -4. Open a PR only if the issue is labeled with "Accepting Contributions", ensure the PR description clearly describes the problem and solution. Note that an open PR without an issue labeled with "Accepting Contributions" will not be accepted. - +We'd love to accept your patches and contributions to this project. There are +just a few small guidelines you need to follow. ## Contributor License Agreement diff --git a/gax-java/dependencies.properties b/gax-java/dependencies.properties index f869063376..64691d8ad7 100644 --- a/gax-java/dependencies.properties +++ b/gax-java/dependencies.properties @@ -39,6 +39,7 @@ maven.com_google_api_grpc_proto_google_common_protos=com.google.api.grpc:proto-g maven.com_google_api_grpc_grpc_google_common_protos=com.google.api.grpc:grpc-google-common-protos:2.33.0 maven.com_google_auth_google_auth_library_oauth2_http=com.google.auth:google-auth-library-oauth2-http:1.23.0 maven.com_google_auth_google_auth_library_credentials=com.google.auth:google-auth-library-credentials:1.23.0 +maven.io_opentelemetry_opentelemetry_api=io.opentelemetry:opentelemetry-api:1.34.1 maven.io_opencensus_opencensus_api=io.opencensus:opencensus-api:0.31.1 maven.io_opencensus_opencensus_contrib_grpc_metrics=io.opencensus:opencensus-contrib-grpc-metrics:0.31.1 maven.io_opencensus_opencensus_contrib_http_util=io.opencensus:opencensus-contrib-http-util:0.31.1 diff --git a/gax-java/gax/BUILD.bazel b/gax-java/gax/BUILD.bazel index 85c41f8b58..309564c87f 100644 --- a/gax-java/gax/BUILD.bazel +++ b/gax-java/gax/BUILD.bazel @@ -18,6 +18,7 @@ _COMPILE_DEPS = [ "@com_google_code_findbugs_jsr305//jar", "@com_google_errorprone_error_prone_annotations//jar", "@com_google_guava_guava//jar", + "@io_opentelemetry_opentelemetry_api//jar", "@io_opencensus_opencensus_api//jar", "@io_opencensus_opencensus_contrib_http_util//jar", "@io_grpc_grpc_java//context:context", diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 57fb6f5799..950d4388f9 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -57,6 +57,18 @@ graal-sdk provided + + io.opentelemetry + opentelemetry-api + + + io.opentelemetry + opentelemetry-sdk + + + io.opentelemetry + opentelemetry-exporter-otlp + diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index 25929756f5..f986e2be81 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -33,6 +33,7 @@ import com.google.api.core.ApiFunction; import com.google.api.gax.core.CredentialsProvider; import com.google.api.gax.core.ExecutorProvider; +import com.google.api.gax.tracing.ApiTracerFactory; import com.google.common.base.MoreObjects; import java.io.IOException; import java.util.concurrent.Executor; @@ -284,6 +285,16 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { return self(); } + /** + * Sets the ApiTracerFactory for the client instance. To enable default metrics, users need to + * create an instance of metricsRecorder and pass it to the metricsTracerFactory, and set it + * here. + */ + public B setTracerFactory(@Nullable ApiTracerFactory tracerFactory) { + stubSettings.setTracerFactory(tracerFactory); + return self(); + } + /** * Gets the ExecutorProvider that was previously set on this Builder. This ExecutorProvider is * to use for running asynchronous API call logic (such as retries and long-running operations), @@ -351,6 +362,11 @@ public Duration getWatchdogCheckInterval() { return stubSettings.getStreamWatchdogCheckInterval(); } + /** Gets the TracerFactory that was previously set in this Builder */ + public ApiTracerFactory getTracerFactory() { + return stubSettings.getTracerFactory(); + } + /** Gets the GDCH API audience that was previously set in this Builder */ @Nullable public String getGdchApiAudience() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 45a8558599..af7b97664f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -47,12 +47,14 @@ * This class computes generic metrics that can be observed in the lifecycle of an RPC operation. * The responsibility of recording metrics should delegate to {@link MetricsRecorder}, hence this * class should not have any knowledge about the observability framework used for metrics recording. + * method_name and language will be autopopulated attributes. Default value of language is 'Java'. */ @BetaApi @InternalApi public class MetricsTracer implements ApiTracer { private static final String STATUS_ATTRIBUTE = "status"; + private static final String LANGUAGE = "Java"; private Stopwatch attemptTimer; @@ -64,6 +66,7 @@ public class MetricsTracer implements ApiTracer { public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) { this.attributes.put("method_name", methodName.toString()); + this.attributes.put("language", LANGUAGE); this.metricsRecorder = metricsRecorder; } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java new file mode 100644 index 0000000000..3297c7a69c --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -0,0 +1,109 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.tracing; + +import com.google.common.annotations.VisibleForTesting; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.Meter; +import java.util.Map; + +public class OpentelemetryMetricsRecorder implements MetricsRecorder { + + private Meter meter; + + private DoubleHistogram attemptLatencyRecorder; + + private DoubleHistogram operationLatencyRecorder; + + private LongCounter operationCountRecorder; + + private LongCounter attemptCountRecorder; + + public OpentelemetryMetricsRecorder(Meter meter) { + this.meter = meter; + this.attemptLatencyRecorder = + meter + .histogramBuilder("attempt_latency") + .setDescription("Duration of an individual attempt") + .setUnit("ms") + .build(); + this.operationLatencyRecorder = + meter + .histogramBuilder("operation_latency") + .setDescription( + "Total time until final operation success or failure, including retries and backoff.") + .setUnit("ms") + .build(); + this.operationCountRecorder = + meter + .counterBuilder("operation_count") + .setDescription("Count of Operations") + .setUnit("1") + .build(); + this.attemptCountRecorder = + meter + .counterBuilder("attempt_count") + .setDescription("Count of Attempts") + .setUnit("1") + .build(); + } + + public void recordAttemptLatency(double attemptLatency, Map attributes) { + attemptLatencyRecorder.record(attemptLatency, toOtelAttributes(attributes)); + } + + public void recordAttemptCount(long count, Map attributes) { + attemptCountRecorder.add(count, toOtelAttributes(attributes)); + } + + public void recordOperationLatency(double operationLatency, Map attributes) { + operationLatencyRecorder.record(operationLatency, toOtelAttributes(attributes)); + } + + public void recordOperationCount(long count, Map attributes) { + operationCountRecorder.add(count, toOtelAttributes(attributes)); + } + + @VisibleForTesting + Attributes toOtelAttributes(Map attributes) { + + if (attributes == null) { + throw new IllegalArgumentException("Input attributes map cannot be null"); + } + + AttributesBuilder attributesBuilder = Attributes.builder(); + attributes.forEach(attributesBuilder::put); + return attributesBuilder.build(); + } +} diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java index 7b6b76f181..e30d7f1662 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java @@ -77,7 +77,8 @@ public void testOperationSucceeded_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "OK", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -96,7 +97,8 @@ public void testOperationFailed_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "INVALID_ARGUMENT", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -112,7 +114,8 @@ public void testOperationCancelled_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "CANCELLED", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -132,7 +135,8 @@ public void testAttemptSucceeded_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "OK", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -155,7 +159,8 @@ public void testAttemptFailed_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "INVALID_ARGUMENT", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -174,7 +179,8 @@ public void testAttemptCancelled_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "CANCELLED", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -196,7 +202,8 @@ public void testAttemptFailedRetriesExhausted_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "DEADLINE_EXCEEDED", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -218,7 +225,8 @@ public void testAttemptPermanentFailure_recordsAttributes() { Map attributes = ImmutableMap.of( "status", "NOT_FOUND", - "method_name", "fake_service.fake_method"); + "method_name", "fake_service.fake_method", + "language", "Java"); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java new file mode 100644 index 0000000000..8bf9162315 --- /dev/null +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java @@ -0,0 +1,266 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.tracing; + +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import com.google.common.collect.ImmutableMap; +import com.google.common.truth.Truth; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.DoubleHistogramBuilder; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongCounterBuilder; +import io.opentelemetry.api.metrics.Meter; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockito.quality.Strictness; + +@RunWith(JUnit4.class) +public class OpentelemetryMetricsRecorderTest { + + // stricter way of testing for early detection of unused stubs and argument mismatches + @Rule + public final MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); + + private OpentelemetryMetricsRecorder otelMetricsRecorder; + + @Mock private Meter meter; + @Mock private LongCounter attemptCountRecorder; + @Mock private LongCounterBuilder attemptCountRecorderBuilder; + @Mock private DoubleHistogramBuilder attemptLatencyRecorderBuilder; + @Mock private DoubleHistogram attemptLatencyRecorder; + @Mock private DoubleHistogram operationLatencyRecorder; + @Mock private DoubleHistogramBuilder operationLatencyRecorderBuilder; + @Mock private LongCounter operationCountRecorder; + @Mock private LongCounterBuilder operationCountRecorderBuilder; + + @Before + public void setUp() { + + meter = Mockito.mock(Meter.class); + + // setup mocks for all the recorders using chained mocking + setupAttemptCountRecorder(); + setupAttemptLatencyRecorder(); + setupOperationLatencyRecorder(); + setupOperationCountRecorder(); + + otelMetricsRecorder = new OpentelemetryMetricsRecorder(meter); + } + + @Test + public void testAttemptCountRecorder_recordsAttributes() { + + ImmutableMap attributes = + ImmutableMap.of( + "status", "OK", + "method_name", "fake_service.fake_method", + "language", "Java"); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordAttemptCount(1, attributes); + + verify(attemptCountRecorder).add(1, otelAttributes); + verifyNoMoreInteractions(attemptCountRecorder); + } + + @Test + public void testAttemptLatencyRecorder_recordsAttributes() { + + ImmutableMap attributes = + ImmutableMap.of( + "status", "NOT_FOUND", + "method_name", "fake_service.fake_method", + "language", "Java"); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordAttemptLatency(1.1, attributes); + + verify(attemptLatencyRecorder).record(1.1, otelAttributes); + verifyNoMoreInteractions(attemptLatencyRecorder); + } + + @Test + public void testOperationCountRecorder_recordsAttributes() { + + ImmutableMap attributes = + ImmutableMap.of( + "status", "OK", + "method_name", "fake_service.fake_method", + "language", "Java"); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordOperationCount(1, attributes); + + verify(operationCountRecorder).add(1, otelAttributes); + verifyNoMoreInteractions(operationCountRecorder); + } + + @Test + public void testOperationLatencyRecorder_recordsAttributes() { + + ImmutableMap attributes = + ImmutableMap.of( + "status", "INVALID_ARGUMENT", + "method_name", "fake_service.fake_method", + "language", "Java"); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordOperationLatency(1.7, attributes); + + verify(operationLatencyRecorder).record(1.7, otelAttributes); + verifyNoMoreInteractions(operationLatencyRecorder); + } + + @Test + public void testToOtelAttributes_correctConversion() { + + ImmutableMap attributes = + ImmutableMap.of( + "status", "OK", + "method_name", "fake_service.fake_method", + "language", "Java"); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))) + .isEqualTo("fake_service.fake_method"); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); + } + + @Test + public void testToOtelAttributes_nullInput() { + + Throwable thrown = + assertThrows( + IllegalArgumentException.class, + () -> { + otelMetricsRecorder.toOtelAttributes(null); + }); + Truth.assertThat(thrown).hasMessageThat().contains("Input attributes map cannot be null"); + } + + /// this is a potential candidate for test in the future when we enforce non-null values in the + // attributes map + // will remove this before merging the PR + // @Test + // public void testToOtelAttributes_nullKeyValuePair() { + // + // + // Map attributes = new HashMap<>(); + // attributes.put("status", "OK"); + // attributes.put("method_name", "fake_service.fake_method"); + // attributes.put("language", "Java"); + // // try to insert a key-value pair with a null value + // attributes.put("fakeDatabaseId", null); + // + // Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + // + // //only 3 attributes should be added to the Attributes object + // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); + // + // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))).isEqualTo("fake_service.fake_method"); + // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); + // + // // attributes should only have 3 entries since the 4th attribute value is null + // Truth.assertThat(otelAttributes.size()).isEqualTo(3); + // } + + private void setupAttemptCountRecorder() { + + attemptCountRecorderBuilder = Mockito.mock(LongCounterBuilder.class); + attemptCountRecorder = Mockito.mock(LongCounter.class); + + // Configure chained mocking for AttemptCountRecorder + Mockito.when(meter.counterBuilder("attempt_count")).thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.setDescription("Count of Attempts")) + .thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.setUnit("1")).thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.build()).thenReturn(attemptCountRecorder); + } + + private void setupOperationCountRecorder() { + + operationCountRecorderBuilder = Mockito.mock(LongCounterBuilder.class); + operationCountRecorder = Mockito.mock(LongCounter.class); + + // Configure chained mocking for operationCountRecorder + Mockito.when(meter.counterBuilder("operation_count")).thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.setDescription("Count of Operations")) + .thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.setUnit("1")) + .thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.build()).thenReturn(operationCountRecorder); + } + + private void setupAttemptLatencyRecorder() { + + attemptLatencyRecorderBuilder = Mockito.mock(DoubleHistogramBuilder.class); + attemptLatencyRecorder = Mockito.mock(DoubleHistogram.class); + + // Configure chained mocking for attemptLatencyRecorder + Mockito.when(meter.histogramBuilder("attempt_latency")) + .thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.setDescription("Duration of an individual attempt")) + .thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.setUnit("ms")) + .thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.build()).thenReturn(attemptLatencyRecorder); + } + + private void setupOperationLatencyRecorder() { + + operationLatencyRecorderBuilder = Mockito.mock(DoubleHistogramBuilder.class); + operationLatencyRecorder = Mockito.mock(DoubleHistogram.class); + + // Configure chained mocking for operationLatencyRecorder + Mockito.when(meter.histogramBuilder("operation_latency")) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when( + operationLatencyRecorderBuilder.setDescription( + "Total time until final operation success or failure, including retries and backoff.")) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when(operationLatencyRecorderBuilder.setUnit("ms")) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when(operationLatencyRecorderBuilder.build()).thenReturn(operationLatencyRecorder); + } +} diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 5e98463187..4ebfbd34a0 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -158,6 +158,13 @@ pom import + + io.opentelemetry + opentelemetry-bom + 1.27.0 + pom + import + diff --git a/showcase/gapic-showcase/pom.xml b/showcase/gapic-showcase/pom.xml index 582db4d93c..a38b53bc69 100644 --- a/showcase/gapic-showcase/pom.xml +++ b/showcase/gapic-showcase/pom.xml @@ -182,5 +182,20 @@ grpc-google-iam-v1 test + + io.opentelemetry + opentelemetry-api + test + + + io.opentelemetry + opentelemetry-sdk + test + + + io.opentelemetry + opentelemetry-sdk-testing + test + diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java new file mode 100644 index 0000000000..ec03c18d2e --- /dev/null +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -0,0 +1,606 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.showcase.v1beta1.it; + +import static org.junit.Assert.assertThrows; + +import com.google.api.client.http.javanet.NetHttpTransport; +import com.google.api.core.ApiFuture; +import com.google.api.core.InternalApi; +import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.core.NoCredentialsProvider; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.InvalidArgumentException; +import com.google.api.gax.rpc.StatusCode.Code; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.rpc.UnavailableException; +import com.google.api.gax.tracing.MetricsTracerFactory; +import com.google.api.gax.tracing.OpentelemetryMetricsRecorder; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.truth.Truth; +import com.google.protobuf.Duration; +import com.google.rpc.Status; +import com.google.showcase.v1beta1.BlockRequest; +import com.google.showcase.v1beta1.BlockResponse; +import com.google.showcase.v1beta1.EchoClient; +import com.google.showcase.v1beta1.EchoRequest; +import com.google.showcase.v1beta1.EchoSettings; +import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import com.google.showcase.v1beta1.stub.EchoStubSettings; +import io.grpc.ManagedChannelBuilder; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.HistogramData; +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.data.MetricDataType; +import io.opentelemetry.sdk.metrics.data.PointData; +import io.opentelemetry.sdk.metrics.data.SumData; +import io.opentelemetry.sdk.resources.Resource; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import org.junit.After; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; + +/** + * Showcase Test to confirm that metrics are being collected and that the correct metrics are being + * recorded. Utilizes an in-memory metric reader to collect the data. + */ +public class ITOtelMetrics { + private static final String ATTEMPT_COUNT = "attempt_count"; + private static final String OPERATION_COUNT = "operation_count"; + private InMemoryMetricReader inMemoryMetricReader; + private EchoClient grpcClient; + private EchoClient httpClient; + + private OpentelemetryMetricsRecorder createOtelMetricsRecorder( + InMemoryMetricReader inMemoryMetricReader) { + + Resource resource = Resource.getDefault(); + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder() + .setResource(resource) + .registerMetricReader(inMemoryMetricReader) + .build(); + + OpenTelemetry openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); + + // Meter Creation + Meter meter = + openTelemetry + .meterBuilder("gax") + .setInstrumentationVersion(GaxProperties.getGaxVersion()) + .build(); + // OpenTelemetry Metrics Recorder + return new OpentelemetryMetricsRecorder(meter); + } + + @Before + public void setup() throws Exception { + inMemoryMetricReader = InMemoryMetricReader.create(); + OpentelemetryMetricsRecorder otelMetricsRecorder = + createOtelMetricsRecorder(inMemoryMetricReader); + grpcClient = + TestClientInitializer.createGrpcEchoClientOpentelemetry( + new MetricsTracerFactory(otelMetricsRecorder)); + httpClient = + TestClientInitializer.createHttpJsonEchoClientOpentelemetry( + new MetricsTracerFactory(otelMetricsRecorder)); + } + + @After + public void cleanup() throws InterruptedException { + inMemoryMetricReader.shutdown(); + + grpcClient.close(); + httpClient.close(); + + grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + + private void verifyLongSumCount(MetricData metricData, long count) { + SumData longSumData = metricData.getLongSumData(); + List points = new ArrayList<>(longSumData.getPoints()); + Truth.assertThat(points.size()).isEqualTo(1); + LongPointData pointData = points.get(0); + Truth.assertThat(pointData.getValue()).isEqualTo(count); + } + + private void verifyMetricData( + List metricDataList, int operationCount, int attemptCount) { + for (MetricData metricData : metricDataList) { + switch (metricData.getName()) { + case OPERATION_COUNT: + verifyLongSumCount(metricData, operationCount); + break; + case ATTEMPT_COUNT: + verifyLongSumCount(metricData, attemptCount); + break; + default: + // It is difficult to verify latency (operation or attempt) without flaky behavior + break; + } + } + } + + private void verifyMetricAttributes(MetricData metricData, Map attributeMapping) { + List pointDataList; + if (metricData.getType().equals(MetricDataType.HISTOGRAM)) { + HistogramData histogramData = metricData.getHistogramData(); + pointDataList = new ArrayList<>(histogramData.getPoints()); + } else if (metricData.getType().equals(MetricDataType.LONG_SUM)) { + SumData longSumData = metricData.getLongSumData(); + pointDataList = new ArrayList<>(longSumData.getPoints()); + } else { + pointDataList = new ArrayList<>(); + } + Truth.assertThat(pointDataList.size()).isEqualTo(1); + PointData pointData = pointDataList.get(0); + Attributes attributes = pointData.getAttributes(); + for (Map.Entry, Object> entrySet : attributes.asMap().entrySet()) { + if (attributeMapping.containsKey(entrySet.getKey().getKey())) { + String key = entrySet.getKey().getKey(); + Truth.assertThat(entrySet.getValue()).isEqualTo(attributeMapping.get(key)); + } + } + } + + @Test + public void testGrpc_operationSucceeded_recordsMetrics() { + int operationCount = 1; + int attemptCount = 1; + grpcClient.echo(EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build()); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "Echo.Echo", "status", "OK"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("Echo.Echo"); + // Truth.assertThat(status).isEqualTo("OK"); + // } + // } + } + + @Test + public void testHttpJson_operationSucceeded_recordsMetrics() { + int operationCount = 1; + int attemptCount = 1; + httpClient.echo(EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build()); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Echo", "status", "OK"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Echo"); + // Truth.assertThat(status).isEqualTo("OK"); + // } + // } + // } + } + + @Test + public void testGrpc_operationCancelled_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); + + UnaryCallable blockCallable = grpcClient.blockCallable(); + ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + Thread.sleep(1000); + blockResponseApiFuture.cancel(true); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "Echo.Block", "status", "CANCELLED"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("Echo.Block"); + // Truth.assertThat(status).isEqualTo("CANCELLED"); + // } + // } + // } + } + + @Test + public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); + + UnaryCallable blockCallable = httpClient.blockCallable(); + ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + Thread.sleep(1000); + blockResponseApiFuture.cancel(true); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "CANCELLED"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); + // Truth.assertThat(status).isEqualTo("CANCELLED"); + // } + // } + // } + } + + @Test + public void testGrpc_operationFailed_recordsMetrics() { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(2)) + .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal())) + .build(); + + UnaryCallable blockCallable = grpcClient.blockCallable(); + ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + assertThrows(ExecutionException.class, blockResponseApiFuture::get); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "Echo.Block", "status", "INVALID_ARGUMENT"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("Echo.Block"); + // Truth.assertThat(status).isEqualTo("INVALID_ARGUMENT"); + // } + // } + // } + } + + @Test + public void testHttpJson_operationFailed_recordsMetrics() { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(2)) + .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal())) + .build(); + + UnaryCallable blockCallable = httpClient.blockCallable(); + ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + assertThrows(ExecutionException.class, blockResponseApiFuture::get); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "UNKNOWN"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); + // Truth.assertThat(status).isEqualTo("UNKNOWN"); + // } + // } + // } + } + + @Test + public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 3; + // A custom EchoClient is used in this test because retries have jitter, and we cannot + // predict the number of attempts that are scheduled for an RPC invocation otherwise. + // The custom retrySettings limit to a set number of attempts before the call gives up. + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setTotalTimeout(org.threeten.bp.Duration.ofMillis(5000L)) + .setMaxAttempts(3) + .build(); + + EchoStubSettings.Builder grpcEchoSettingsBuilder = EchoStubSettings.newBuilder(); + grpcEchoSettingsBuilder + .echoSettings() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of(Code.UNAVAILABLE)); + EchoSettings grpcEchoSettings = EchoSettings.create(grpcEchoSettingsBuilder.build()); + grpcEchoSettings = + grpcEchoSettings + .toBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .build()) + .setEndpoint("localhost:7469") + .build(); + EchoClient grpcClient = EchoClient.create(grpcEchoSettings); + EchoRequest echoRequest = + EchoRequest.newBuilder() + .setError(Status.newBuilder().setCode(Code.UNAVAILABLE.ordinal()).build()) + .build(); + + assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "Echo.Echo", "status", "UNAVAILABLE"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // + // System.out.println(pointData); + // + // // add a comment why I am doing this + // double max = pointData.getMax(); + // double min = pointData.getMin(); + // if (max != min) { + // Truth.assertThat(pointData.getCount()).isEqualTo(3); + // } + // Truth.assertThat(method).isEqualTo("Echo.Echo"); + // Truth.assertThat(status).isEqualTo("UNKNOWN"); + // } + // } + // } + grpcClient.close(); + grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + + @Ignore("Temp ignore while investigating this failure") + @Test + public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 3; + // A custom EchoClient is used in this test because retries have jitter, and we cannot + // predict the number of attempts that are scheduled for an RPC invocation otherwise. + // The custom retrySettings limit to a set number of attempts before the call gives up. + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setTotalTimeout(org.threeten.bp.Duration.ofMillis(5000L)) + .setMaxAttempts(3) + .build(); + + EchoStubSettings.Builder httpJsonEchoSettingsBuilder = EchoStubSettings.newHttpJsonBuilder(); + httpJsonEchoSettingsBuilder + .echoSettings() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of(Code.UNAVAILABLE)); + EchoSettings httpJsonEchoSettings = EchoSettings.create(httpJsonEchoSettingsBuilder.build()); + httpJsonEchoSettings = + httpJsonEchoSettings + .toBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint("http://localhost:7469") + .build()) + .build(); + + EchoClient httpClient = EchoClient.create(httpJsonEchoSettings); + + EchoRequest echoRequest = + EchoRequest.newBuilder() + .setError(Status.newBuilder().setCode(Code.UNAVAILABLE.ordinal()).build()) + .build(); + + assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of( + "method_name", "google.showcase.v1beta1.Echo/Echo", "status", "UNAVAILABLE"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Echo"); + // Truth.assertThat(language).isEqualTo("Java"); + // Truth.assertThat(status).isEqualTo("UNKNOWN"); + // Truth.assertThat(pointData.getCount()).isEqualTo(3); + // } + // } + // } + httpClient.close(); + httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + + @Test + public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(2).build()) + .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal()).build()) + .build(); + + assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "Echo.Block", "status", "INVALID_ARGUMENT"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("Echo.Block"); + // Truth.assertThat(language).isEqualTo("Java"); + // Truth.assertThat(status).isEqualTo("INVALID_ARGUMENT"); + // Truth.assertThat(pointData.getCount()).isEqualTo(1); + // } + // } + // } + } + + @Test + public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Exception { + int operationCount = 1; + int attemptCount = 1; + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(2).build()) + .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal()).build()) + .build(); + + assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); + + inMemoryMetricReader.forceFlush(); + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, operationCount, attemptCount); + + ImmutableMap attributeMapping = + ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "UNKNOWN"); + verifyMetricAttributes(metricDataList.get(0), attributeMapping); + // inMemoryMetricReader.forceFlush(); + // List metricDataList = new + // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + // for (MetricData metricData : metricDataList) { + // HistogramData histogramData = metricData.getHistogramData(); + // if (!histogramData.getPoints().isEmpty()) { + // for (HistogramPointData pointData : histogramData.getPoints()) { + // String method = + // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); + // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); + // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); + // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); + // Truth.assertThat(language).isEqualTo("Java"); + // Truth.assertThat(status).isEqualTo("UNKNOWN"); + // Truth.assertThat(pointData.getCount()).isEqualTo(3); + // } + // } + // } + } +} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java index e620e254c6..ff3fb0c82d 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java @@ -24,6 +24,7 @@ import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.StatusCode; import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.tracing.ApiTracerFactory; import com.google.common.collect.ImmutableList; import com.google.showcase.v1beta1.ComplianceClient; import com.google.showcase.v1beta1.ComplianceSettings; @@ -284,4 +285,39 @@ public static ComplianceClient createHttpJsonComplianceClient( .build(); return ComplianceClient.create(httpJsonComplianceSettings); } + + public static EchoClient createHttpJsonEchoClientOpentelemetry( + ApiTracerFactory metricsTracerFactory) throws Exception { + + EchoSettings httpJsonEchoSettings = + EchoSettings.newHttpJsonBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory(metricsTracerFactory) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint("http://localhost:7469") + .build()) + .build(); + + return EchoClient.create(httpJsonEchoSettings); + } + + public static EchoClient createGrpcEchoClientOpentelemetry(ApiTracerFactory metricsTracerFactory) + throws Exception { + + EchoSettings grpcEchoSettings = + EchoSettings.newBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory(metricsTracerFactory) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .build()) + .setEndpoint("localhost:7469") + .build(); + + return EchoClient.create(grpcEchoSettings); + } } diff --git a/showcase/pom.xml b/showcase/pom.xml index 13fbf1e1ea..5ee87583a4 100644 --- a/showcase/pom.xml +++ b/showcase/pom.xml @@ -38,6 +38,13 @@ pom import + + io.opentelemetry + opentelemetry-bom + 1.35.0 + pom + import + com.google.api.grpc proto-gapic-showcase-v1beta1 @@ -53,7 +60,6 @@ gapic-showcase 0.0.1-SNAPSHOT - junit junit From f7578112d6b97235a9bacb019a7899de43281e29 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 26 Feb 2024 12:06:09 -0500 Subject: [PATCH 02/24] chore: Update CONTRIBUTING.md --- CONTRIBUTING.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22b241cb73..a203e7b1b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,10 @@ # How to Contribute -We'd love to accept your patches and contributions to this project. There are -just a few small guidelines you need to follow. +We'd love to accept your patches and contributions to this project. There are just a few small guidelines you need to follow before opening an issue or a PR: +1. Ensure the issue was not already reported. +2. Open a new issue if you are unable to find an existing issue addressing your problem. Make sure to include a title and clear description, as much relevant information as possible, and a code sample or an executable test case demonstrating the expected behavior that is not occurring. +3. Discuss the priority and potential solutions with the maintainers in the issue. The maintainers would review the issue and add a label "Accepting Contributions" once the issue is ready for accepting contributions. +4. Open a PR only if the issue is labeled with "Accepting Contributions", ensure the PR description clearly describes the problem and solution. Note that an open PR without an issue labeled with "Accepting Contributions" will not be accepted. ## Contributor License Agreement From 58fc09cc6d3fd175cfc9e12cd30ac0d6a2ed6347 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 26 Feb 2024 12:15:04 -0500 Subject: [PATCH 03/24] chore: Fix lint issues --- .../showcase/v1beta1/it/ITOtelMetrics.java | 248 +++++------------- 1 file changed, 59 insertions(+), 189 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index ec03c18d2e..4bdc0647dc 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -20,7 +20,6 @@ import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.core.ApiFuture; -import com.google.api.core.InternalApi; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.NoCredentialsProvider; import com.google.api.gax.retrying.RetrySettings; @@ -52,7 +51,6 @@ import io.opentelemetry.sdk.metrics.data.HistogramData; import io.opentelemetry.sdk.metrics.data.LongPointData; import io.opentelemetry.sdk.metrics.data.MetricData; -import io.opentelemetry.sdk.metrics.data.MetricDataType; import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.data.SumData; import io.opentelemetry.sdk.resources.Resource; @@ -150,17 +148,12 @@ private void verifyMetricData( } } + /** + * Extract the attributes from the Metric Data and ensure that the attributes recorded match the + * keys and values stored inside the attributeMapping + */ private void verifyMetricAttributes(MetricData metricData, Map attributeMapping) { - List pointDataList; - if (metricData.getType().equals(MetricDataType.HISTOGRAM)) { - HistogramData histogramData = metricData.getHistogramData(); - pointDataList = new ArrayList<>(histogramData.getPoints()); - } else if (metricData.getType().equals(MetricDataType.LONG_SUM)) { - SumData longSumData = metricData.getLongSumData(); - pointDataList = new ArrayList<>(longSumData.getPoints()); - } else { - pointDataList = new ArrayList<>(); - } + List pointDataList = extractPointData(metricData); Truth.assertThat(pointDataList.size()).isEqualTo(1); PointData pointData = pointDataList.get(0); Attributes attributes = pointData.getAttributes(); @@ -172,6 +165,24 @@ private void verifyMetricAttributes(MetricData metricData, Map a } } + private static List extractPointData(MetricData metricData) { + List pointDataList; + switch (metricData.getType()) { + case HISTOGRAM: + HistogramData histogramData = metricData.getHistogramData(); + pointDataList = new ArrayList<>(histogramData.getPoints()); + break; + case LONG_SUM: + SumData longSumData = metricData.getLongSumData(); + pointDataList = new ArrayList<>(longSumData.getPoints()); + break; + default: + pointDataList = new ArrayList<>(); + break; + } + return pointDataList; + } + @Test public void testGrpc_operationSucceeded_recordsMetrics() { int operationCount = 1; @@ -183,18 +194,8 @@ public void testGrpc_operationSucceeded_recordsMetrics() { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", "OK"); + ImmutableMap.of("method_name", "Echo.Echo", "status", "OK", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("Echo.Echo"); - // Truth.assertThat(status).isEqualTo("OK"); - // } - // } } @Test @@ -208,23 +209,9 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Echo", "status", "OK"); + ImmutableMap.of( + "method_name", "google.showcase.v1beta1.Echo/Echo", "status", "OK", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Echo"); - // Truth.assertThat(status).isEqualTo("OK"); - // } - // } - // } } @Test @@ -244,24 +231,8 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Block", "status", "CANCELLED"); + ImmutableMap.of("method_name", "Echo.Block", "status", "CANCELLED", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("Echo.Block"); - // Truth.assertThat(status).isEqualTo("CANCELLED"); - // } - // } - // } } @Test @@ -281,24 +252,14 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "CANCELLED"); + ImmutableMap.of( + "method_name", + "google.showcase.v1beta1.Echo/Block", + "status", + "CANCELLED", + "language", + "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); - // Truth.assertThat(status).isEqualTo("CANCELLED"); - // } - // } - // } } @Test @@ -320,24 +281,9 @@ public void testGrpc_operationFailed_recordsMetrics() { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Block", "status", "INVALID_ARGUMENT"); + ImmutableMap.of( + "method_name", "Echo.Block", "status", "INVALID_ARGUMENT", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("Echo.Block"); - // Truth.assertThat(status).isEqualTo("INVALID_ARGUMENT"); - // } - // } - // } } @Test @@ -359,24 +305,14 @@ public void testHttpJson_operationFailed_recordsMetrics() { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "UNKNOWN"); + ImmutableMap.of( + "method_name", + "google.showcase.v1beta1.Echo/Block", + "status", + "UNKNOWN", + "language", + "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); - // Truth.assertThat(status).isEqualTo("UNKNOWN"); - // } - // } - // } } @Test @@ -423,32 +359,8 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", "UNAVAILABLE"); + ImmutableMap.of("method_name", "Echo.Echo", "status", "UNAVAILABLE", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // - // System.out.println(pointData); - // - // // add a comment why I am doing this - // double max = pointData.getMax(); - // double min = pointData.getMin(); - // if (max != min) { - // Truth.assertThat(pointData.getCount()).isEqualTo(3); - // } - // Truth.assertThat(method).isEqualTo("Echo.Echo"); - // Truth.assertThat(status).isEqualTo("UNKNOWN"); - // } - // } - // } grpcClient.close(); grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } @@ -502,26 +414,13 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E ImmutableMap attributeMapping = ImmutableMap.of( - "method_name", "google.showcase.v1beta1.Echo/Echo", "status", "UNAVAILABLE"); + "method_name", + "google.showcase.v1beta1.Echo/Echo", + "status", + "UNAVAILABLE", + "language", + "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Echo"); - // Truth.assertThat(language).isEqualTo("Java"); - // Truth.assertThat(status).isEqualTo("UNKNOWN"); - // Truth.assertThat(pointData.getCount()).isEqualTo(3); - // } - // } - // } httpClient.close(); httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } @@ -543,26 +442,9 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Exception { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Block", "status", "INVALID_ARGUMENT"); + ImmutableMap.of( + "method_name", "Echo.Block", "status", "INVALID_ARGUMENT", "language", "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("Echo.Block"); - // Truth.assertThat(language).isEqualTo("Java"); - // Truth.assertThat(status).isEqualTo("INVALID_ARGUMENT"); - // Truth.assertThat(pointData.getCount()).isEqualTo(1); - // } - // } - // } } @Test @@ -582,25 +464,13 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Excepti verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "google.showcase.v1beta1.Echo/Block", "status", "UNKNOWN"); + ImmutableMap.of( + "method_name", + "google.showcase.v1beta1.Echo/Block", + "status", + "UNKNOWN", + "language", + "Java"); verifyMetricAttributes(metricDataList.get(0), attributeMapping); - // inMemoryMetricReader.forceFlush(); - // List metricDataList = new - // ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - // for (MetricData metricData : metricDataList) { - // HistogramData histogramData = metricData.getHistogramData(); - // if (!histogramData.getPoints().isEmpty()) { - // for (HistogramPointData pointData : histogramData.getPoints()) { - // String method = - // pointData.getAttributes().get(AttributeKey.stringKey("method_name")); - // String language = pointData.getAttributes().get(AttributeKey.stringKey("language")); - // String status = pointData.getAttributes().get(AttributeKey.stringKey("status")); - // Truth.assertThat(method).isEqualTo("google.showcase.v1beta1.Echo/Block"); - // Truth.assertThat(language).isEqualTo("Java"); - // Truth.assertThat(status).isEqualTo("UNKNOWN"); - // Truth.assertThat(pointData.getCount()).isEqualTo(3); - // } - // } - // } } } From d29ad1afea69d2bd0779e0ee1ae2d682dff2a2bd Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 26 Feb 2024 15:37:52 -0500 Subject: [PATCH 04/24] chore: Clean up ITOTelMetrics test --- CONTRIBUTING.md | 1 + gax-java/gax/pom.xml | 8 --- .../google/api/gax/rpc/ClientSettings.java | 16 ++++-- .../tracing/OpentelemetryMetricsRecorder.java | 55 +++++++++++++------ .../showcase/v1beta1/it/ITOtelMetrics.java | 35 ++++++++---- 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a203e7b1b6..f2b357f2ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,6 +6,7 @@ We'd love to accept your patches and contributions to this project. There are ju 3. Discuss the priority and potential solutions with the maintainers in the issue. The maintainers would review the issue and add a label "Accepting Contributions" once the issue is ready for accepting contributions. 4. Open a PR only if the issue is labeled with "Accepting Contributions", ensure the PR description clearly describes the problem and solution. Note that an open PR without an issue labeled with "Accepting Contributions" will not be accepted. + ## Contributor License Agreement Contributions to this project must be accompanied by a Contributor License diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 950d4388f9..140e954ac1 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -61,14 +61,6 @@ io.opentelemetry opentelemetry-api - - io.opentelemetry - opentelemetry-sdk - - - io.opentelemetry - opentelemetry-exporter-otlp - diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index f986e2be81..d6b59f336f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -31,6 +31,7 @@ import com.google.api.core.ApiClock; import com.google.api.core.ApiFunction; +import com.google.api.core.BetaApi; import com.google.api.gax.core.CredentialsProvider; import com.google.api.gax.core.ExecutorProvider; import com.google.api.gax.tracing.ApiTracerFactory; @@ -121,6 +122,16 @@ public final String getGdchApiAudience() { return stubSettings.getGdchApiAudience(); } + /** + * Gets the configured {@link ApiTracerFactory} that will be used to generate traces for + * operations. + */ + @BetaApi("The surface for tracing is not stable yet and may change in the future.") + @Nonnull + public ApiTracerFactory getTracerFactory() { + return stubSettings.getTracerFactory(); + } + public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) @@ -362,11 +373,6 @@ public Duration getWatchdogCheckInterval() { return stubSettings.getStreamWatchdogCheckInterval(); } - /** Gets the TracerFactory that was previously set in this Builder */ - public ApiTracerFactory getTracerFactory() { - return stubSettings.getTracerFactory(); - } - /** Gets the GDCH API audience that was previously set in this Builder */ @Nullable public String getGdchApiAudience() { diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index 3297c7a69c..8c1ba11848 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -31,6 +31,7 @@ package com.google.api.gax.tracing; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; @@ -39,23 +40,16 @@ import java.util.Map; public class OpentelemetryMetricsRecorder implements MetricsRecorder { - - private Meter meter; - private DoubleHistogram attemptLatencyRecorder; - private DoubleHistogram operationLatencyRecorder; - private LongCounter operationCountRecorder; - private LongCounter attemptCountRecorder; public OpentelemetryMetricsRecorder(Meter meter) { - this.meter = meter; this.attemptLatencyRecorder = meter .histogramBuilder("attempt_latency") - .setDescription("Duration of an individual attempt") + .setDescription("Time an individual attempt took") .setUnit("ms") .build(); this.operationLatencyRecorder = @@ -68,40 +62,67 @@ public OpentelemetryMetricsRecorder(Meter meter) { this.operationCountRecorder = meter .counterBuilder("operation_count") - .setDescription("Count of Operations") - .setUnit("1") + .setDescription("Number of operations made") .build(); this.attemptCountRecorder = meter .counterBuilder("attempt_count") - .setDescription("Count of Attempts") - .setUnit("1") + .setDescription("Number of attempts made") .build(); } + /** + * Record the latency for an individual attempt. Data is stored in a Histogram. + * + * @param attemptLatency Attempt Latency in ms + * @param attributes Map of the attributes to store + */ + @Override public void recordAttemptLatency(double attemptLatency, Map attributes) { attemptLatencyRecorder.record(attemptLatency, toOtelAttributes(attributes)); } + /** + * Record an attempt made. The attempt count number is stored in a LongCounter. + * + *

The count should be set as 1 every time this is invoked (each retry attempt) + * + * @param count The number of attempts made + * @param attributes Map of the attributes to store + */ + @Override public void recordAttemptCount(long count, Map attributes) { attemptCountRecorder.add(count, toOtelAttributes(attributes)); } + /** + * Record the latency for the entire operation. This is the latency for the entire RPC, including + * all the retry attempts + * + * @param operationLatency Operation Latency in ms + * @param attributes Map of the attributes to store + */ + @Override public void recordOperationLatency(double operationLatency, Map attributes) { operationLatencyRecorder.record(operationLatency, toOtelAttributes(attributes)); } + /** + * Record an operation made. The operation count number is stored in a LongCounter. + * + *

The operation count should always be 1 and this should be invoked once. + * + * @param count The number of operations made + * @param attributes Map of the attributes to store + */ + @Override public void recordOperationCount(long count, Map attributes) { operationCountRecorder.add(count, toOtelAttributes(attributes)); } @VisibleForTesting Attributes toOtelAttributes(Map attributes) { - - if (attributes == null) { - throw new IllegalArgumentException("Input attributes map cannot be null"); - } - + Preconditions.checkNotNull(attributes, "Attributes map cannot be null"); AttributesBuilder attributesBuilder = Attributes.builder(); attributes.forEach(attributesBuilder::put); return attributesBuilder.build(); diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 4bdc0647dc..2d43157c37 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -1,17 +1,31 @@ /* - * Copyright 2023 Google LLC + * Copyright 2024 Google LLC * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: * - * https://www.apache.org/licenses/LICENSE-2.0 + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ package com.google.showcase.v1beta1.it; @@ -78,7 +92,6 @@ public class ITOtelMetrics { private OpentelemetryMetricsRecorder createOtelMetricsRecorder( InMemoryMetricReader inMemoryMetricReader) { - Resource resource = Resource.getDefault(); SdkMeterProvider sdkMeterProvider = SdkMeterProvider.builder() From df7c611abc1006700a00c6a962237e29d1868c2e Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 26 Feb 2024 16:23:27 -0500 Subject: [PATCH 05/24] chore: Clean up ITOTelMetrics test --- .../google/showcase/v1beta1/it/ITOtelMetrics.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 2d43157c37..ad53920827 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -163,7 +163,7 @@ private void verifyMetricData( /** * Extract the attributes from the Metric Data and ensure that the attributes recorded match the - * keys and values stored inside the attributeMapping + * keys and values stored inside the attributeMapping. */ private void verifyMetricAttributes(MetricData metricData, Map attributeMapping) { List pointDataList = extractPointData(metricData); @@ -202,7 +202,6 @@ public void testGrpc_operationSucceeded_recordsMetrics() { int attemptCount = 1; grpcClient.echo(EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build()); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -217,7 +216,6 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { int attemptCount = 1; httpClient.echo(EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build()); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -236,10 +234,10 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { UnaryCallable blockCallable = grpcClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + // Sleep 1s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -257,10 +255,10 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { UnaryCallable blockCallable = httpClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); + // Sleep 1s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -289,7 +287,6 @@ public void testGrpc_operationFailed_recordsMetrics() { ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); assertThrows(ExecutionException.class, blockResponseApiFuture::get); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -313,7 +310,6 @@ public void testHttpJson_operationFailed_recordsMetrics() { ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); assertThrows(ExecutionException.class, blockResponseApiFuture::get); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -367,7 +363,6 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -421,7 +416,6 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -450,7 +444,6 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Exception { assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); @@ -472,7 +465,6 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Excepti assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); - inMemoryMetricReader.forceFlush(); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); From 9a3fc3cb31fa648488e5df12f4ee787a4f09fc04 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 10:55:05 -0500 Subject: [PATCH 06/24] chore: Ignore failing Otel tests --- .../tracing/OpentelemetryMetricsRecorder.java | 10 +-- .../showcase/v1beta1/it/ITOtelMetrics.java | 76 +++++++++++-------- 2 files changed, 47 insertions(+), 39 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index 8c1ba11848..d4d89aaa36 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -60,15 +60,9 @@ public OpentelemetryMetricsRecorder(Meter meter) { .setUnit("ms") .build(); this.operationCountRecorder = - meter - .counterBuilder("operation_count") - .setDescription("Number of operations made") - .build(); + meter.counterBuilder("operation_count").setDescription("Number of operations made").build(); this.attemptCountRecorder = - meter - .counterBuilder("attempt_count") - .setDescription("Number of attempts made") - .build(); + meter.counterBuilder("attempt_count").setDescription("Number of attempts made").build(); } /** diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index ad53920827..39093ee63b 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -67,7 +67,6 @@ import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.data.PointData; import io.opentelemetry.sdk.metrics.data.SumData; -import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.util.ArrayList; import java.util.List; @@ -86,16 +85,15 @@ public class ITOtelMetrics { private static final String ATTEMPT_COUNT = "attempt_count"; private static final String OPERATION_COUNT = "operation_count"; + private static final String DEFAULT_LANGUAGE = "Java"; private InMemoryMetricReader inMemoryMetricReader; private EchoClient grpcClient; private EchoClient httpClient; private OpentelemetryMetricsRecorder createOtelMetricsRecorder( InMemoryMetricReader inMemoryMetricReader) { - Resource resource = Resource.getDefault(); SdkMeterProvider sdkMeterProvider = SdkMeterProvider.builder() - .setResource(resource) .registerMetricReader(inMemoryMetricReader) .build(); @@ -105,7 +103,7 @@ private OpentelemetryMetricsRecorder createOtelMetricsRecorder( // Meter Creation Meter meter = openTelemetry - .meterBuilder("gax") + .meterBuilder("ShowcaseOtelMetrics") .setInstrumentationVersion(GaxProperties.getGaxVersion()) .build(); // OpenTelemetry Metrics Recorder @@ -136,6 +134,7 @@ public void cleanup() throws InterruptedException { httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } + // Extract the value recorded from the PointData private void verifyLongSumCount(MetricData metricData, long count) { SumData longSumData = metricData.getLongSumData(); List points = new ArrayList<>(longSumData.getPoints()); @@ -144,6 +143,7 @@ private void verifyLongSumCount(MetricData metricData, long count) { Truth.assertThat(pointData.getValue()).isEqualTo(count); } + // Verify that Operation Count and Attempt Count values are being recorded correctly private void verifyMetricData( List metricDataList, int operationCount, int attemptCount) { for (MetricData metricData : metricDataList) { @@ -200,13 +200,15 @@ private static List extractPointData(MetricData metricData) { public void testGrpc_operationSucceeded_recordsMetrics() { int operationCount = 1; int attemptCount = 1; - grpcClient.echo(EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build()); + Code statusCode = Code.OK; + EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build(); + grpcClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", "OK", "language", "Java"); + ImmutableMap.of("method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -214,14 +216,16 @@ public void testGrpc_operationSucceeded_recordsMetrics() { public void testHttpJson_operationSucceeded_recordsMetrics() { int operationCount = 1; int attemptCount = 1; - httpClient.echo(EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build()); + Code statusCode = Code.OK; + EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); + httpClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = ImmutableMap.of( - "method_name", "google.showcase.v1beta1.Echo/Echo", "status", "OK", "language", "Java"); + "method_name", "google.showcase.v1beta1.Echo/Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -229,6 +233,7 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { public void testGrpc_operationCancelled_recordsMetrics() throws Exception { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.CANCELLED; BlockRequest blockRequest = BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); @@ -242,7 +247,7 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Block", "status", "CANCELLED", "language", "Java"); + ImmutableMap.of("method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -250,6 +255,7 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.CANCELLED; BlockRequest blockRequest = BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); @@ -267,9 +273,9 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { "method_name", "google.showcase.v1beta1.Echo/Block", "status", - "CANCELLED", + statusCode.name(), "language", - "Java"); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -277,10 +283,11 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { public void testGrpc_operationFailed_recordsMetrics() { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = BlockRequest.newBuilder() .setResponseDelay(Duration.newBuilder().setSeconds(2)) - .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal())) + .setError(Status.newBuilder().setCode(statusCode.ordinal())) .build(); UnaryCallable blockCallable = grpcClient.blockCallable(); @@ -292,18 +299,20 @@ public void testGrpc_operationFailed_recordsMetrics() { ImmutableMap attributeMapping = ImmutableMap.of( - "method_name", "Echo.Block", "status", "INVALID_ARGUMENT", "language", "Java"); + "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_operationFailed_recordsMetrics() { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = BlockRequest.newBuilder() .setResponseDelay(Duration.newBuilder().setSeconds(2)) - .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal())) + .setError(Status.newBuilder().setCode(statusCode.ordinal())) .build(); UnaryCallable blockCallable = httpClient.blockCallable(); @@ -318,9 +327,9 @@ public void testHttpJson_operationFailed_recordsMetrics() { "method_name", "google.showcase.v1beta1.Echo/Block", "status", - "UNKNOWN", + statusCode.name(), "language", - "Java"); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -328,6 +337,7 @@ public void testHttpJson_operationFailed_recordsMetrics() { public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { int operationCount = 1; int attemptCount = 3; + Code statusCode = Code.UNAVAILABLE; // A custom EchoClient is used in this test because retries have jitter, and we cannot // predict the number of attempts that are scheduled for an RPC invocation otherwise. // The custom retrySettings limit to a set number of attempts before the call gives up. @@ -341,7 +351,7 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep grpcEchoSettingsBuilder .echoSettings() .setRetrySettings(retrySettings) - .setRetryableCodes(ImmutableSet.of(Code.UNAVAILABLE)); + .setRetryableCodes(ImmutableSet.of(statusCode)); EchoSettings grpcEchoSettings = EchoSettings.create(grpcEchoSettingsBuilder.build()); grpcEchoSettings = grpcEchoSettings @@ -358,7 +368,7 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep EchoClient grpcClient = EchoClient.create(grpcEchoSettings); EchoRequest echoRequest = EchoRequest.newBuilder() - .setError(Status.newBuilder().setCode(Code.UNAVAILABLE.ordinal()).build()) + .setError(Status.newBuilder().setCode(statusCode.ordinal()).build()) .build(); assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); @@ -367,17 +377,18 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", "UNAVAILABLE", "language", "Java"); + ImmutableMap.of("method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); grpcClient.close(); grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - @Ignore("Temp ignore while investigating this failure") + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { int operationCount = 1; int attemptCount = 3; + Code statusCode = Code.UNAVAILABLE; // A custom EchoClient is used in this test because retries have jitter, and we cannot // predict the number of attempts that are scheduled for an RPC invocation otherwise. // The custom retrySettings limit to a set number of attempts before the call gives up. @@ -391,7 +402,7 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E httpJsonEchoSettingsBuilder .echoSettings() .setRetrySettings(retrySettings) - .setRetryableCodes(ImmutableSet.of(Code.UNAVAILABLE)); + .setRetryableCodes(ImmutableSet.of(statusCode)); EchoSettings httpJsonEchoSettings = EchoSettings.create(httpJsonEchoSettingsBuilder.build()); httpJsonEchoSettings = httpJsonEchoSettings @@ -411,7 +422,7 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E EchoRequest echoRequest = EchoRequest.newBuilder() - .setError(Status.newBuilder().setCode(Code.UNAVAILABLE.ordinal()).build()) + .setError(Status.newBuilder().setCode(statusCode.ordinal()).build()) .build(); assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); @@ -424,22 +435,23 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E "method_name", "google.showcase.v1beta1.Echo/Echo", "status", - "UNAVAILABLE", + statusCode.name(), "language", - "Java"); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); httpClient.close(); httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } @Test - public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Exception { + public void testGrpc_attemptPermanentFailure_recordsMetrics() { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = BlockRequest.newBuilder() .setResponseDelay(Duration.newBuilder().setSeconds(2).build()) - .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal()).build()) + .setError(Status.newBuilder().setCode(statusCode.ordinal()).build()) .build(); assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); @@ -449,18 +461,20 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Exception { ImmutableMap attributeMapping = ImmutableMap.of( - "method_name", "Echo.Block", "status", "INVALID_ARGUMENT", "language", "Java"); + "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test - public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Exception { + public void testHttpJson_attemptPermanentFailure_recordsMetrics() { int operationCount = 1; int attemptCount = 1; + Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = BlockRequest.newBuilder() .setResponseDelay(Duration.newBuilder().setSeconds(2).build()) - .setError(Status.newBuilder().setCode(Code.INVALID_ARGUMENT.ordinal()).build()) + .setError(Status.newBuilder().setCode(statusCode.ordinal()).build()) .build(); assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); @@ -473,9 +487,9 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Excepti "method_name", "google.showcase.v1beta1.Echo/Block", "status", - "UNKNOWN", + statusCode.name(), "language", - "Java"); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } } From 9121cb6a373e33ec1bd8fd842053da2a1a76b0ac Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 12:47:44 -0500 Subject: [PATCH 07/24] chore: Update OpentelemetryMetricsRecorderTest to use InMemoryMetricExporter --- gax-java/gax/pom.xml | 5 + .../tracing/OpentelemetryMetricsRecorder.java | 4 +- .../OpentelemetryMetricsRecorderTest.java | 307 +++++++++--------- 3 files changed, 163 insertions(+), 153 deletions(-) diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 140e954ac1..81928ed495 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -61,6 +61,11 @@ io.opentelemetry opentelemetry-api + + io.opentelemetry + opentelemetry-sdk-testing + test + diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index d4d89aaa36..1dc5b77ac8 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -60,9 +60,9 @@ public OpentelemetryMetricsRecorder(Meter meter) { .setUnit("ms") .build(); this.operationCountRecorder = - meter.counterBuilder("operation_count").setDescription("Number of operations made").build(); + meter.counterBuilder("operation_count").setDescription("Number of Operations").build(); this.attemptCountRecorder = - meter.counterBuilder("attempt_count").setDescription("Number of attempts made").build(); + meter.counterBuilder("attempt_count").setDescription("Number of Attempts").build(); } /** diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java index 8bf9162315..8529665b10 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java @@ -30,129 +30,224 @@ package com.google.api.gax.tracing; import static org.junit.Assert.assertThrows; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.google.api.gax.core.GaxProperties; import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.DoubleHistogram; -import io.opentelemetry.api.metrics.DoubleHistogramBuilder; -import io.opentelemetry.api.metrics.LongCounter; -import io.opentelemetry.api.metrics.LongCounterBuilder; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.Data; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.data.PointData; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricExporter; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; -import org.junit.Rule; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; -import org.mockito.quality.Strictness; @RunWith(JUnit4.class) public class OpentelemetryMetricsRecorderTest { - - // stricter way of testing for early detection of unused stubs and argument mismatches - @Rule - public final MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); - private OpentelemetryMetricsRecorder otelMetricsRecorder; + private static InMemoryMetricExporter exporter; + private static PeriodicMetricReader periodicMetricReader; - @Mock private Meter meter; - @Mock private LongCounter attemptCountRecorder; - @Mock private LongCounterBuilder attemptCountRecorderBuilder; - @Mock private DoubleHistogramBuilder attemptLatencyRecorderBuilder; - @Mock private DoubleHistogram attemptLatencyRecorder; - @Mock private DoubleHistogram operationLatencyRecorder; - @Mock private DoubleHistogramBuilder operationLatencyRecorderBuilder; - @Mock private LongCounter operationCountRecorder; - @Mock private LongCounterBuilder operationCountRecorderBuilder; + @BeforeClass + public static void setUpBeforeClass() { + exporter = InMemoryMetricExporter.create(); + periodicMetricReader = PeriodicMetricReader.create(exporter); + } @Before public void setUp() { + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder().registerMetricReader(periodicMetricReader).build(); + OpenTelemetry openTelemetry = + OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); + + // Meter Creation + Meter meter = + openTelemetry + .meterBuilder("OtelMetricsRecorderTest") + .setInstrumentationVersion(GaxProperties.getGaxVersion()) + .build(); - meter = Mockito.mock(Meter.class); + otelMetricsRecorder = new OpentelemetryMetricsRecorder(meter); + } - // setup mocks for all the recorders using chained mocking - setupAttemptCountRecorder(); - setupAttemptLatencyRecorder(); - setupOperationLatencyRecorder(); - setupOperationCountRecorder(); + @After + public void cleanUp() { + exporter.reset(); + } - otelMetricsRecorder = new OpentelemetryMetricsRecorder(meter); + @AfterClass + public static void cleanUpAfterClass() { + exporter.shutdown(); } - @Test - public void testAttemptCountRecorder_recordsAttributes() { + private void verifyAttributesRecorded( + Attributes pointDataAttributes, Map attributes) { + Map, Object> attributesMap = pointDataAttributes.asMap(); + for (Map.Entry, Object> entrySet : attributesMap.entrySet()) { + String key = entrySet.getKey().getKey(); + String value = entrySet.getValue().toString(); + Truth.assertThat(attributes.containsKey(key)).isTrue(); + Truth.assertThat(attributes.get(key)).isEqualTo(value); + } + } - ImmutableMap attributes = + private List extractPointDataList() { + List finishedMetricItems = exporter.getFinishedMetricItems(); + Truth.assertThat(finishedMetricItems.size()).isEqualTo(1); + MetricData metricData = finishedMetricItems.get(0); + Data data = metricData.getData(); + return new ArrayList<>(data.getPoints()); + } + + @Test + public void recordAttemptCount_singleAttempt() { + Map attributes = ImmutableMap.of( "status", "OK", "method_name", "fake_service.fake_method", "language", "Java"); - Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); otelMetricsRecorder.recordAttemptCount(1, attributes); + periodicMetricReader.forceFlush(); + + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); + LongPointData longPointData = (LongPointData) pointData.get(0); + Truth.assertThat(longPointData.getValue()).isEqualTo(1); - verify(attemptCountRecorder).add(1, otelAttributes); - verifyNoMoreInteractions(attemptCountRecorder); + Attributes pointDataAttributes = longPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); } @Test - public void testAttemptLatencyRecorder_recordsAttributes() { + public void recordAttemptCount_multipleAttempt() { + Map attributes = + ImmutableMap.of( + "status", "OK", + "method_name", "fake_service.fake_method", + "language", "Java"); - ImmutableMap attributes = + otelMetricsRecorder.recordAttemptCount(1, attributes); + otelMetricsRecorder.recordAttemptCount(1, attributes); + otelMetricsRecorder.recordAttemptCount(1, attributes); + periodicMetricReader.forceFlush(); + + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); + LongPointData longPointData = (LongPointData) pointData.get(0); + Truth.assertThat(longPointData.getValue()).isEqualTo(3); + + Attributes pointDataAttributes = longPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); + } + + @Test + public void recordAttemptLatency_singleAttempt() { + Map attributes = ImmutableMap.of( "status", "NOT_FOUND", "method_name", "fake_service.fake_method", "language", "Java"); - Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); otelMetricsRecorder.recordAttemptLatency(1.1, attributes); + periodicMetricReader.forceFlush(); - verify(attemptLatencyRecorder).record(1.1, otelAttributes); - verifyNoMoreInteractions(attemptLatencyRecorder); + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); + HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); + Truth.assertThat(histogramPointData.getCount()).isEqualTo(1); + + Attributes pointDataAttributes = histogramPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); } @Test - public void testOperationCountRecorder_recordsAttributes() { + public void recordAttemptLatency_multipleAttempts() { + Map attributes = + ImmutableMap.of( + "status", "NOT_FOUND", + "method_name", "fake_service.fake_method", + "language", "Java"); - ImmutableMap attributes = + otelMetricsRecorder.recordAttemptLatency(1.1, attributes); + otelMetricsRecorder.recordAttemptLatency(1.2, attributes); + otelMetricsRecorder.recordAttemptLatency(1.3, attributes); + periodicMetricReader.forceFlush(); + + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); + HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); + Truth.assertThat(histogramPointData.getCount()).isEqualTo(3); + + Attributes pointDataAttributes = histogramPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); + } + + @Test + public void recordOperationCount() { + Map attributes = ImmutableMap.of( "status", "OK", "method_name", "fake_service.fake_method", "language", "Java"); - Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); otelMetricsRecorder.recordOperationCount(1, attributes); + periodicMetricReader.forceFlush(); + + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); + LongPointData longPointData = (LongPointData) pointData.get(0); + Truth.assertThat(longPointData.getValue()).isEqualTo(1); - verify(operationCountRecorder).add(1, otelAttributes); - verifyNoMoreInteractions(operationCountRecorder); + Attributes pointDataAttributes = longPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); } @Test - public void testOperationLatencyRecorder_recordsAttributes() { - - ImmutableMap attributes = + public void recordOperationLatency() { + Map attributes = ImmutableMap.of( "status", "INVALID_ARGUMENT", "method_name", "fake_service.fake_method", "language", "Java"); - Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); otelMetricsRecorder.recordOperationLatency(1.7, attributes); + periodicMetricReader.forceFlush(); - verify(operationLatencyRecorder).record(1.7, otelAttributes); - verifyNoMoreInteractions(operationLatencyRecorder); + List pointData = extractPointDataList(); + Truth.assertThat(pointData.size()).isEqualTo(1); + Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); + HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); + Truth.assertThat(histogramPointData.getCount()).isEqualTo(1); + + Attributes pointDataAttributes = histogramPointData.getAttributes(); + verifyAttributesRecorded(pointDataAttributes, attributes); } @Test - public void testToOtelAttributes_correctConversion() { - + public void toOtelAttributes_correctConversion() { ImmutableMap attributes = ImmutableMap.of( "status", "OK", @@ -168,99 +263,9 @@ public void testToOtelAttributes_correctConversion() { } @Test - public void testToOtelAttributes_nullInput() { - - Throwable thrown = - assertThrows( - IllegalArgumentException.class, - () -> { - otelMetricsRecorder.toOtelAttributes(null); - }); - Truth.assertThat(thrown).hasMessageThat().contains("Input attributes map cannot be null"); - } - - /// this is a potential candidate for test in the future when we enforce non-null values in the - // attributes map - // will remove this before merging the PR - // @Test - // public void testToOtelAttributes_nullKeyValuePair() { - // - // - // Map attributes = new HashMap<>(); - // attributes.put("status", "OK"); - // attributes.put("method_name", "fake_service.fake_method"); - // attributes.put("language", "Java"); - // // try to insert a key-value pair with a null value - // attributes.put("fakeDatabaseId", null); - // - // Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); - // - // //only 3 attributes should be added to the Attributes object - // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); - // - // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))).isEqualTo("fake_service.fake_method"); - // Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); - // - // // attributes should only have 3 entries since the 4th attribute value is null - // Truth.assertThat(otelAttributes.size()).isEqualTo(3); - // } - - private void setupAttemptCountRecorder() { - - attemptCountRecorderBuilder = Mockito.mock(LongCounterBuilder.class); - attemptCountRecorder = Mockito.mock(LongCounter.class); - - // Configure chained mocking for AttemptCountRecorder - Mockito.when(meter.counterBuilder("attempt_count")).thenReturn(attemptCountRecorderBuilder); - Mockito.when(attemptCountRecorderBuilder.setDescription("Count of Attempts")) - .thenReturn(attemptCountRecorderBuilder); - Mockito.when(attemptCountRecorderBuilder.setUnit("1")).thenReturn(attemptCountRecorderBuilder); - Mockito.when(attemptCountRecorderBuilder.build()).thenReturn(attemptCountRecorder); - } - - private void setupOperationCountRecorder() { - - operationCountRecorderBuilder = Mockito.mock(LongCounterBuilder.class); - operationCountRecorder = Mockito.mock(LongCounter.class); - - // Configure chained mocking for operationCountRecorder - Mockito.when(meter.counterBuilder("operation_count")).thenReturn(operationCountRecorderBuilder); - Mockito.when(operationCountRecorderBuilder.setDescription("Count of Operations")) - .thenReturn(operationCountRecorderBuilder); - Mockito.when(operationCountRecorderBuilder.setUnit("1")) - .thenReturn(operationCountRecorderBuilder); - Mockito.when(operationCountRecorderBuilder.build()).thenReturn(operationCountRecorder); - } - - private void setupAttemptLatencyRecorder() { - - attemptLatencyRecorderBuilder = Mockito.mock(DoubleHistogramBuilder.class); - attemptLatencyRecorder = Mockito.mock(DoubleHistogram.class); - - // Configure chained mocking for attemptLatencyRecorder - Mockito.when(meter.histogramBuilder("attempt_latency")) - .thenReturn(attemptLatencyRecorderBuilder); - Mockito.when(attemptLatencyRecorderBuilder.setDescription("Duration of an individual attempt")) - .thenReturn(attemptLatencyRecorderBuilder); - Mockito.when(attemptLatencyRecorderBuilder.setUnit("ms")) - .thenReturn(attemptLatencyRecorderBuilder); - Mockito.when(attemptLatencyRecorderBuilder.build()).thenReturn(attemptLatencyRecorder); - } - - private void setupOperationLatencyRecorder() { - - operationLatencyRecorderBuilder = Mockito.mock(DoubleHistogramBuilder.class); - operationLatencyRecorder = Mockito.mock(DoubleHistogram.class); - - // Configure chained mocking for operationLatencyRecorder - Mockito.when(meter.histogramBuilder("operation_latency")) - .thenReturn(operationLatencyRecorderBuilder); - Mockito.when( - operationLatencyRecorderBuilder.setDescription( - "Total time until final operation success or failure, including retries and backoff.")) - .thenReturn(operationLatencyRecorderBuilder); - Mockito.when(operationLatencyRecorderBuilder.setUnit("ms")) - .thenReturn(operationLatencyRecorderBuilder); - Mockito.when(operationLatencyRecorderBuilder.build()).thenReturn(operationLatencyRecorder); + public void toOtelAttributes_nullInput() { + NullPointerException exception = + assertThrows(NullPointerException.class, () -> otelMetricsRecorder.toOtelAttributes(null)); + Truth.assertThat(exception).hasMessageThat().contains("Attributes map cannot be null"); } } From 9d099f63f371ad1ff22adc4de0c1ee6c07994b00 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 13:20:07 -0500 Subject: [PATCH 08/24] chore: Update showcase tests --- .../showcase/v1beta1/it/ITOtelMetrics.java | 109 +++++++++--------- 1 file changed, 52 insertions(+), 57 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 39093ee63b..82728b587f 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -62,11 +62,11 @@ import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.data.HistogramData; +import io.opentelemetry.sdk.metrics.data.Data; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; import io.opentelemetry.sdk.metrics.data.LongPointData; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.data.PointData; -import io.opentelemetry.sdk.metrics.data.SumData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.util.ArrayList; import java.util.List; @@ -85,6 +85,8 @@ public class ITOtelMetrics { private static final String ATTEMPT_COUNT = "attempt_count"; private static final String OPERATION_COUNT = "operation_count"; + private static final String ATTEMPT_LATENCY = "attempt_latency"; + private static final String OPERATION_LATENCY = "operation_latency"; private static final String DEFAULT_LANGUAGE = "Java"; private InMemoryMetricReader inMemoryMetricReader; private EchoClient grpcClient; @@ -93,9 +95,7 @@ public class ITOtelMetrics { private OpentelemetryMetricsRecorder createOtelMetricsRecorder( InMemoryMetricReader inMemoryMetricReader) { SdkMeterProvider sdkMeterProvider = - SdkMeterProvider.builder() - .registerMetricReader(inMemoryMetricReader) - .build(); + SdkMeterProvider.builder().registerMetricReader(inMemoryMetricReader).build(); OpenTelemetry openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); @@ -134,28 +134,31 @@ public void cleanup() throws InterruptedException { httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - // Extract the value recorded from the PointData - private void verifyLongSumCount(MetricData metricData, long count) { - SumData longSumData = metricData.getLongSumData(); - List points = new ArrayList<>(longSumData.getPoints()); - Truth.assertThat(points.size()).isEqualTo(1); - LongPointData pointData = points.get(0); - Truth.assertThat(pointData.getValue()).isEqualTo(count); - } - - // Verify that Operation Count and Attempt Count values are being recorded correctly private void verifyMetricData( List metricDataList, int operationCount, int attemptCount) { for (MetricData metricData : metricDataList) { + Data data = metricData.getData(); + List points = new ArrayList<>(data.getPoints()); + Truth.assertThat(points.size()).isEqualTo(1); + PointData pointData = points.get(0); switch (metricData.getName()) { case OPERATION_COUNT: - verifyLongSumCount(metricData, operationCount); + Truth.assertThat(((LongPointData) pointData).getValue()).isEqualTo(operationCount); break; case ATTEMPT_COUNT: - verifyLongSumCount(metricData, attemptCount); + Truth.assertThat(((LongPointData) pointData).getValue()).isEqualTo(attemptCount); + break; + case OPERATION_LATENCY: + // It is difficult to verify the actual latency values (operation or attempt) + // without flaky behavior. Test that the number of data points recorded matches. + Truth.assertThat(((HistogramPointData) pointData).getCount()).isEqualTo(operationCount); + break; + case ATTEMPT_LATENCY: + // It is difficult to verify the actual latency values (operation or attempt) + // without flaky behavior. Test that the number of data points recorded matches. + Truth.assertThat(((HistogramPointData) pointData).getCount()).isEqualTo(attemptCount); break; default: - // It is difficult to verify latency (operation or attempt) without flaky behavior break; } } @@ -166,49 +169,33 @@ private void verifyMetricData( * keys and values stored inside the attributeMapping. */ private void verifyMetricAttributes(MetricData metricData, Map attributeMapping) { - List pointDataList = extractPointData(metricData); + List pointDataList = (List) metricData.getData().getPoints(); Truth.assertThat(pointDataList.size()).isEqualTo(1); - PointData pointData = pointDataList.get(0); - Attributes attributes = pointData.getAttributes(); - for (Map.Entry, Object> entrySet : attributes.asMap().entrySet()) { - if (attributeMapping.containsKey(entrySet.getKey().getKey())) { - String key = entrySet.getKey().getKey(); - Truth.assertThat(entrySet.getValue()).isEqualTo(attributeMapping.get(key)); - } + Attributes attributes = pointDataList.get(0).getAttributes(); + Map, Object> attributesMap = attributes.asMap(); + for (Map.Entry, Object> entrySet : attributesMap.entrySet()) { + String key = entrySet.getKey().getKey(); + String value = entrySet.getValue().toString(); + Truth.assertThat(attributeMapping.containsKey(key)).isTrue(); + Truth.assertThat(attributeMapping.get(key)).isEqualTo(value); } } - private static List extractPointData(MetricData metricData) { - List pointDataList; - switch (metricData.getType()) { - case HISTOGRAM: - HistogramData histogramData = metricData.getHistogramData(); - pointDataList = new ArrayList<>(histogramData.getPoints()); - break; - case LONG_SUM: - SumData longSumData = metricData.getLongSumData(); - pointDataList = new ArrayList<>(longSumData.getPoints()); - break; - default: - pointDataList = new ArrayList<>(); - break; - } - return pointDataList; - } - @Test public void testGrpc_operationSucceeded_recordsMetrics() { int operationCount = 1; int attemptCount = 1; Code statusCode = Code.OK; - EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build(); + EchoRequest echoRequest = + EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build(); grpcClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); + ImmutableMap.of( + "method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -217,7 +204,8 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { int operationCount = 1; int attemptCount = 1; Code statusCode = Code.OK; - EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); + EchoRequest echoRequest = + EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); httpClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); @@ -225,7 +213,12 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { ImmutableMap attributeMapping = ImmutableMap.of( - "method_name", "google.showcase.v1beta1.Echo/Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); + "method_name", + "google.showcase.v1beta1.Echo/Echo", + "status", + statusCode.name(), + "language", + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -247,7 +240,8 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); + ImmutableMap.of( + "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -273,9 +267,9 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { "method_name", "google.showcase.v1beta1.Echo/Block", "status", - statusCode.name(), + statusCode.name(), "language", - DEFAULT_LANGUAGE); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -327,9 +321,9 @@ public void testHttpJson_operationFailed_recordsMetrics() { "method_name", "google.showcase.v1beta1.Echo/Block", "status", - statusCode.name(), + statusCode.name(), "language", - DEFAULT_LANGUAGE); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } @@ -377,7 +371,8 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep verifyMetricData(metricDataList, operationCount, attemptCount); ImmutableMap attributeMapping = - ImmutableMap.of("method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); + ImmutableMap.of( + "method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); grpcClient.close(); grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); @@ -435,9 +430,9 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E "method_name", "google.showcase.v1beta1.Echo/Echo", "status", - statusCode.name(), + statusCode.name(), "language", - DEFAULT_LANGUAGE); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); httpClient.close(); httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); @@ -489,7 +484,7 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() { "status", statusCode.name(), "language", - DEFAULT_LANGUAGE); + DEFAULT_LANGUAGE); verifyMetricAttributes(metricDataList.get(0), attributeMapping); } } From 03acef3dde4afc9b97ba380833cfdebaa048f615 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 14:16:24 -0500 Subject: [PATCH 09/24] chore: Add missing otel dependencies.properties values --- gax-java/dependencies.properties | 4 ++++ gax-java/gax/BUILD.bazel | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/gax-java/dependencies.properties b/gax-java/dependencies.properties index 64691d8ad7..beed26c663 100644 --- a/gax-java/dependencies.properties +++ b/gax-java/dependencies.properties @@ -40,6 +40,10 @@ maven.com_google_api_grpc_grpc_google_common_protos=com.google.api.grpc:grpc-goo maven.com_google_auth_google_auth_library_oauth2_http=com.google.auth:google-auth-library-oauth2-http:1.23.0 maven.com_google_auth_google_auth_library_credentials=com.google.auth:google-auth-library-credentials:1.23.0 maven.io_opentelemetry_opentelemetry_api=io.opentelemetry:opentelemetry-api:1.34.1 +maven.io_opentelemetry_opentelemetry_sdk=io.opentelemetry:opentelemetry-sdk:1.34.1 +maven.io_opentelemetry_opentelemetry_sdk_common=io.opentelemetry:opentelemetry-sdk-common:1.34.1 +maven.io_opentelemetry_opentelemetry-sdk-metrics=io.opentelemetry:opentelemetry-sdk-metrics:1.34.1 +maven.io_opentelemetry_opentelemetry-sdk-testing=io.opentelemetry:opentelemetry-sdk-testing:1.34.1 maven.io_opencensus_opencensus_api=io.opencensus:opencensus-api:0.31.1 maven.io_opencensus_opencensus_contrib_grpc_metrics=io.opencensus:opencensus-contrib-grpc-metrics:0.31.1 maven.io_opencensus_opencensus_contrib_http_util=io.opencensus:opencensus-contrib-http-util:0.31.1 diff --git a/gax-java/gax/BUILD.bazel b/gax-java/gax/BUILD.bazel index 309564c87f..ea9a2a9518 100644 --- a/gax-java/gax/BUILD.bazel +++ b/gax-java/gax/BUILD.bazel @@ -19,6 +19,7 @@ _COMPILE_DEPS = [ "@com_google_errorprone_error_prone_annotations//jar", "@com_google_guava_guava//jar", "@io_opentelemetry_opentelemetry_api//jar", + "@io_opentelemetry_opentelemetry-sdk-metrics//jar", "@io_opencensus_opencensus_api//jar", "@io_opencensus_opencensus_contrib_http_util//jar", "@io_grpc_grpc_java//context:context", @@ -39,6 +40,9 @@ _TEST_COMPILE_DEPS = [ "@net_bytebuddy_byte_buddy//jar", "@org_objenesis_objenesis//jar", "@com_googlecode_java_diff_utils_diffutils//jar", + "@io_opentelemetry_opentelemetry_sdk//jar", + "@io_opentelemetry_opentelemetry-sdk-testing//jar", + "@io_opentelemetry_opentelemetry_sdk_common//jar" ] java_library( From a2d979b5eba630946de0e3b020494213a4498b88 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 15:39:12 -0500 Subject: [PATCH 10/24] chore: Add javadocs for OpentelemetryMetricsRecorder --- .../tracing/OpentelemetryMetricsRecorder.java | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index 1dc5b77ac8..e888dc2f90 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -39,12 +39,33 @@ import io.opentelemetry.api.metrics.Meter; import java.util.Map; +/** + * OpenTelemetry implementation of recording metrics. This implementation collections the + * measurements related to the lifecyle of an RPC. + * + *

For the Otel implementation, an attempt is a single RPC invocation and an operation is the + * collection of all the attempts made before a response is returned (either as a success or an + * error). A single call (i.e. `EchoClient.echo()`) should have an operation_count of 1 and may have + * an attempt_count of 1+ (depending on the retry configurations). + */ public class OpentelemetryMetricsRecorder implements MetricsRecorder { - private DoubleHistogram attemptLatencyRecorder; - private DoubleHistogram operationLatencyRecorder; - private LongCounter operationCountRecorder; - private LongCounter attemptCountRecorder; + private final DoubleHistogram attemptLatencyRecorder; + private final DoubleHistogram operationLatencyRecorder; + private final LongCounter operationCountRecorder; + private final LongCounter attemptCountRecorder; + /** + * Creates the following instruments for the following metrics. + * + *

    + *
  • Attempt Latency: Histogram + *
  • Operation Latency: Histogram + *
  • Attempt Count: Counter + *
  • Operation Count: Counter + *
+ * + * @param meter Otel Meter used to create the instruments of measurements + */ public OpentelemetryMetricsRecorder(Meter meter) { this.attemptLatencyRecorder = meter @@ -59,10 +80,10 @@ public OpentelemetryMetricsRecorder(Meter meter) { "Total time until final operation success or failure, including retries and backoff.") .setUnit("ms") .build(); - this.operationCountRecorder = - meter.counterBuilder("operation_count").setDescription("Number of Operations").build(); this.attemptCountRecorder = meter.counterBuilder("attempt_count").setDescription("Number of Attempts").build(); + this.operationCountRecorder = + meter.counterBuilder("operation_count").setDescription("Number of Operations").build(); } /** From 4edded1f4025457f6eb92df1e15e2a1f5254eec1 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 27 Feb 2024 16:43:02 -0500 Subject: [PATCH 11/24] chore: Use otel v1.34.1 --- gax-java/pom.xml | 2 +- showcase/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 4ebfbd34a0..ed22e224a1 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -161,7 +161,7 @@ io.opentelemetry opentelemetry-bom - 1.27.0 + 1.34.1 pom import diff --git a/showcase/pom.xml b/showcase/pom.xml index 5ee87583a4..76553ecd5a 100644 --- a/showcase/pom.xml +++ b/showcase/pom.xml @@ -41,7 +41,7 @@ io.opentelemetry opentelemetry-bom - 1.35.0 + 1.34.1 pom import From 78cc64ab9fef2ae380a4750ab72b928dd4dc0b25 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 28 Feb 2024 18:13:33 -0500 Subject: [PATCH 12/24] chore: Ignore the HttpJson Otel tests --- .../google/api/gax/tracing/MetricsTracer.java | 11 +- .../showcase/v1beta1/it/ITOtelMetrics.java | 395 +++++++++++++----- 2 files changed, 307 insertions(+), 99 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index af7b97664f..8bcfe7c53d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -52,9 +52,10 @@ @BetaApi @InternalApi public class MetricsTracer implements ApiTracer { - - private static final String STATUS_ATTRIBUTE = "status"; - private static final String LANGUAGE = "Java"; + public static final String METHOD_NAME_ATTRIBUTE = "method_name"; + public static final String LANGUAGE_ATTRIBUTE = "language"; + public static final String STATUS_ATTRIBUTE = "status"; + public static final String DEFAULT_LANGUAGE = "Java"; private Stopwatch attemptTimer; @@ -65,8 +66,8 @@ public class MetricsTracer implements ApiTracer { private MetricsRecorder metricsRecorder; public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) { - this.attributes.put("method_name", methodName.toString()); - this.attributes.put("language", LANGUAGE); + this.attributes.put(METHOD_NAME_ATTRIBUTE, methodName.toString()); + this.attributes.put(LANGUAGE_ATTRIBUTE, DEFAULT_LANGUAGE); this.metricsRecorder = metricsRecorder; } diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 82728b587f..43ccac1a37 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -41,8 +41,10 @@ import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.gax.rpc.UnavailableException; +import com.google.api.gax.tracing.MetricsTracer; import com.google.api.gax.tracing.MetricsTracerFactory; import com.google.api.gax.tracing.OpentelemetryMetricsRecorder; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.truth.Truth; @@ -71,6 +73,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import org.junit.After; @@ -83,15 +86,37 @@ * recorded. Utilizes an in-memory metric reader to collect the data. */ public class ITOtelMetrics { + private static final int DEFAULT_OPERATION_COUNT = 1; private static final String ATTEMPT_COUNT = "attempt_count"; private static final String OPERATION_COUNT = "operation_count"; private static final String ATTEMPT_LATENCY = "attempt_latency"; private static final String OPERATION_LATENCY = "operation_latency"; - private static final String DEFAULT_LANGUAGE = "Java"; private InMemoryMetricReader inMemoryMetricReader; private EchoClient grpcClient; private EchoClient httpClient; + private static class StatusCount { + private final Code statusCode; + private final int count; + + public StatusCount(Code statusCode) { + this(statusCode, 1); + } + + public StatusCount(Code statusCode, int count) { + this.statusCode = statusCode; + this.count = count; + } + + public Code getStatusCode() { + return statusCode; + } + + public int getCount() { + return count; + } + } + private OpentelemetryMetricsRecorder createOtelMetricsRecorder( InMemoryMetricReader inMemoryMetricReader) { SdkMeterProvider sdkMeterProvider = @@ -134,29 +159,34 @@ public void cleanup() throws InterruptedException { httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - private void verifyMetricData( - List metricDataList, int operationCount, int attemptCount) { + private void verifyMetricData(List metricDataList, int attemptCount) { for (MetricData metricData : metricDataList) { Data data = metricData.getData(); List points = new ArrayList<>(data.getPoints()); - Truth.assertThat(points.size()).isEqualTo(1); - PointData pointData = points.get(0); switch (metricData.getName()) { case OPERATION_COUNT: - Truth.assertThat(((LongPointData) pointData).getValue()).isEqualTo(operationCount); + long operationCountSum = + points.stream().map(x -> ((LongPointData) x).getValue()).reduce(0L, Long::sum); + Truth.assertThat(operationCountSum).isEqualTo(DEFAULT_OPERATION_COUNT); break; case ATTEMPT_COUNT: - Truth.assertThat(((LongPointData) pointData).getValue()).isEqualTo(attemptCount); + long attemptCountSum = + points.stream().map(x -> ((LongPointData) x).getValue()).reduce(0L, Long::sum); + Truth.assertThat(attemptCountSum).isEqualTo(attemptCount); break; case OPERATION_LATENCY: + long operationLatencyCountSum = + points.stream().map(x -> ((HistogramPointData) x).getCount()).reduce(0L, Long::sum); // It is difficult to verify the actual latency values (operation or attempt) // without flaky behavior. Test that the number of data points recorded matches. - Truth.assertThat(((HistogramPointData) pointData).getCount()).isEqualTo(operationCount); + Truth.assertThat(operationLatencyCountSum).isEqualTo(DEFAULT_OPERATION_COUNT); break; case ATTEMPT_LATENCY: + long attemptLatencyCountSum = + points.stream().map(x -> ((HistogramPointData) x).getCount()).reduce(0L, Long::sum); // It is difficult to verify the actual latency values (operation or attempt) // without flaky behavior. Test that the number of data points recorded matches. - Truth.assertThat(((HistogramPointData) pointData).getCount()).isEqualTo(attemptCount); + Truth.assertThat(attemptLatencyCountSum).isEqualTo(attemptCount); break; default: break; @@ -165,117 +195,160 @@ private void verifyMetricData( } /** - * Extract the attributes from the Metric Data and ensure that the attributes recorded match the - * keys and values stored inside the attributeMapping. + * Extract the attributes from MetricData and ensures that default attributes are recorded. Uses + * the `OPERATION_COUNT` MetricData to test the attributes. The `OPERATION_COUNT` is only recorded + * once and should only have element of PointData. + * + *

Although the Status attribute is recorded by default on every operation, this helper method + * does not verify it. This is because every individual attempt (retry) may have a different + * status. {@link #verifyStatusAttribute(List, List)} is used to verify the statuses for every + * attempt. */ - private void verifyMetricAttributes(MetricData metricData, Map attributeMapping) { - List pointDataList = (List) metricData.getData().getPoints(); + private void verifyDefaultMetricsAttributes( + List metricDataList, Map attributeMapping) { + Optional metricDataOptional = + metricDataList.stream().filter(x -> x.getName().equals(OPERATION_COUNT)).findAny(); + Truth.assertThat(metricDataOptional.isPresent()).isTrue(); + MetricData operationCountMetricData = metricDataOptional.get(); + + List pointDataList = new ArrayList<>(operationCountMetricData.getData().getPoints()); Truth.assertThat(pointDataList.size()).isEqualTo(1); Attributes attributes = pointDataList.get(0).getAttributes(); Map, Object> attributesMap = attributes.asMap(); - for (Map.Entry, Object> entrySet : attributesMap.entrySet()) { - String key = entrySet.getKey().getKey(); - String value = entrySet.getValue().toString(); - Truth.assertThat(attributeMapping.containsKey(key)).isTrue(); - Truth.assertThat(attributeMapping.get(key)).isEqualTo(value); + for (Map.Entry entrySet : attributeMapping.entrySet()) { + String key = entrySet.getKey(); + String value = entrySet.getValue(); + AttributeKey stringAttributeKey = AttributeKey.stringKey(key); + Truth.assertThat(attributesMap.containsKey(stringAttributeKey)).isTrue(); + Truth.assertThat(attributesMap.get(stringAttributeKey)).isEqualTo(value); + } + } + + /** + * Extract the attributes from MetricData and ensures that default attributes are recorded. Uses + * the `ATTEMPT_COUNT` MetricData to test the attributes. The `ATTEMPT_COUNT` is recorded for + * every retry attempt and should record the status received that each attempt made. + */ + private void verifyStatusAttribute( + List metricDataList, List statusCountList) { + Optional metricDataOptional = + metricDataList.stream().filter(x -> x.getName().equals(ATTEMPT_COUNT)).findAny(); + Truth.assertThat(metricDataOptional.isPresent()).isTrue(); + MetricData attemptCountMetricData = metricDataOptional.get(); + + List pointDataList = new ArrayList<>(attemptCountMetricData.getData().getPoints()); + Truth.assertThat(pointDataList.size()).isEqualTo(statusCountList.size()); + for (int i = 0; i < pointDataList.size(); i++) { + LongPointData longPointData = (LongPointData) pointDataList.get(i); + Truth.assertThat(longPointData.getValue()).isEqualTo(statusCountList.get(i).getCount()); + Attributes attributes = longPointData.getAttributes(); + Truth.assertThat(attributes.get(AttributeKey.stringKey(MetricsTracer.STATUS_ATTRIBUTE))) + .isEqualTo(statusCountList.get(i).getStatusCode().toString()); } } @Test public void testGrpc_operationSucceeded_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; - Code statusCode = Code.OK; EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build(); grpcClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Echo", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(Code.OK)); + verifyStatusAttribute(metricDataList, statusCountList); } + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_operationSucceeded_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; - Code statusCode = Code.OK; EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); httpClient.echo(echoRequest); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", + MetricsTracer.METHOD_NAME_ATTRIBUTE, "google.showcase.v1beta1.Echo/Echo", - "status", - statusCode.name(), - "language", - DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(Code.OK)); + verifyStatusAttribute(metricDataList, statusCountList); } @Test public void testGrpc_operationCancelled_recordsMetrics() throws Exception { - int operationCount = 1; int attemptCount = 1; - Code statusCode = Code.CANCELLED; BlockRequest blockRequest = BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); UnaryCallable blockCallable = grpcClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); - // Sleep 1s before cancelling to let the request go through + // Sleep 3s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Block", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(Code.CANCELLED)); + verifyStatusAttribute(metricDataList, statusCountList); } + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { - int operationCount = 1; int attemptCount = 1; - Code statusCode = Code.CANCELLED; BlockRequest blockRequest = BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); UnaryCallable blockCallable = httpClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); - // Sleep 1s before cancelling to let the request go through + // Sleep 3s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", + MetricsTracer.METHOD_NAME_ATTRIBUTE, "google.showcase.v1beta1.Echo/Block", - "status", - statusCode.name(), - "language", - DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(Code.CANCELLED)); + verifyStatusAttribute(metricDataList, statusCountList); } @Test public void testGrpc_operationFailed_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -289,18 +362,23 @@ public void testGrpc_operationFailed_recordsMetrics() { assertThrows(ExecutionException.class, blockResponseApiFuture::get); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Block", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode)); + verifyStatusAttribute(metricDataList, statusCountList); } @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_operationFailed_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -314,22 +392,22 @@ public void testHttpJson_operationFailed_recordsMetrics() { assertThrows(ExecutionException.class, blockResponseApiFuture::get); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", + MetricsTracer.METHOD_NAME_ATTRIBUTE, "google.showcase.v1beta1.Echo/Block", - "status", - statusCode.name(), - "language", - DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode)); + verifyStatusAttribute(metricDataList, statusCountList); } @Test public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { - int operationCount = 1; int attemptCount = 3; Code statusCode = Code.UNAVAILABLE; // A custom EchoClient is used in this test because retries have jitter, and we cannot @@ -368,12 +446,19 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", "Echo.Echo", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Echo", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode, 3)); + verifyStatusAttribute(metricDataList, statusCountList); + grpcClient.close(); grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } @@ -381,7 +466,6 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { - int operationCount = 1; int attemptCount = 3; Code statusCode = Code.UNAVAILABLE; // A custom EchoClient is used in this test because retries have jitter, and we cannot @@ -423,24 +507,25 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", + MetricsTracer.METHOD_NAME_ATTRIBUTE, "google.showcase.v1beta1.Echo/Echo", - "status", - statusCode.name(), - "language", - DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode, 3)); + verifyStatusAttribute(metricDataList, statusCountList); + httpClient.close(); httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } @Test public void testGrpc_attemptPermanentFailure_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -452,18 +537,23 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() { assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", "Echo.Block", "status", statusCode.name(), "language", DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Block", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode)); + verifyStatusAttribute(metricDataList, statusCountList); } @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test public void testHttpJson_attemptPermanentFailure_recordsMetrics() { - int operationCount = 1; int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -475,16 +565,133 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() { assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); - verifyMetricData(metricDataList, operationCount, attemptCount); + verifyMetricData(metricDataList, attemptCount); + + Map attributeMapping = + ImmutableMap.of( + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "google.showcase.v1beta1.Echo/Block", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = ImmutableList.of(new StatusCount(statusCode)); + verifyStatusAttribute(metricDataList, statusCountList); + } + + @Test + public void testGrpc_multipleFailedAttempts_successfulOperation() throws Exception { + int attemptCount = 3; + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(org.threeten.bp.Duration.ofMillis(500L)) + .setRpcTimeoutMultiplier(2.0) + .setMaxRpcTimeout(org.threeten.bp.Duration.ofMillis(2000L)) + .setTotalTimeout(org.threeten.bp.Duration.ofMillis(6000L)) + .setJittered(false) + .build(); + + EchoStubSettings.Builder grpcEchoSettingsBuilder = EchoStubSettings.newBuilder(); + grpcEchoSettingsBuilder + .blockSettings() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of(Code.DEADLINE_EXCEEDED)); + EchoSettings grpcEchoSettings = EchoSettings.create(grpcEchoSettingsBuilder.build()); + grpcEchoSettings = + grpcEchoSettings + .toBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .build()) + .setEndpoint("localhost:7469") + .build(); + EchoClient grpcClient = EchoClient.create(grpcEchoSettings); + + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(1)) + .setSuccess(BlockResponse.newBuilder().setContent("grpcBlockResponse")) + .build(); + + grpcClient.block(blockRequest); + + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, attemptCount); + + Map attributeMapping = + ImmutableMap.of( + MetricsTracer.METHOD_NAME_ATTRIBUTE, + "Echo.Block", + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + + List statusCountList = + ImmutableList.of(new StatusCount(Code.DEADLINE_EXCEEDED, 2), new StatusCount(Code.OK)); + verifyStatusAttribute(metricDataList, statusCountList); + + grpcClient.close(); + grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); + } + + @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") + @Test + public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exception { + int attemptCount = 3; + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(org.threeten.bp.Duration.ofMillis(500L)) + .setRpcTimeoutMultiplier(2.0) + .setMaxRpcTimeout(org.threeten.bp.Duration.ofMillis(2000L)) + .setTotalTimeout(org.threeten.bp.Duration.ofMillis(6000L)) + .setJittered(false) + .build(); + + EchoStubSettings.Builder httpJsonEchoSettingsBuilder = EchoStubSettings.newHttpJsonBuilder(); + httpJsonEchoSettingsBuilder + .echoSettings() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of(Code.DEADLINE_EXCEEDED)); + EchoSettings httpJsonEchoSettings = EchoSettings.create(httpJsonEchoSettingsBuilder.build()); + httpJsonEchoSettings = + httpJsonEchoSettings + .toBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .setTransportChannelProvider( + EchoSettings.defaultHttpJsonTransportProviderBuilder() + .setHttpTransport( + new NetHttpTransport.Builder().doNotValidateCertificate().build()) + .setEndpoint("http://localhost:7469") + .build()) + .build(); + + EchoClient httpClient = EchoClient.create(httpJsonEchoSettings); + + BlockRequest blockRequest = + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(1)) + .setSuccess(BlockResponse.newBuilder().setContent("httpjsonBlockResponse")) + .build(); + + grpcClient.block(blockRequest); + + List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + verifyMetricData(metricDataList, attemptCount); - ImmutableMap attributeMapping = + Map attributeMapping = ImmutableMap.of( - "method_name", + MetricsTracer.METHOD_NAME_ATTRIBUTE, "google.showcase.v1beta1.Echo/Block", - "status", - statusCode.name(), - "language", - DEFAULT_LANGUAGE); - verifyMetricAttributes(metricDataList.get(0), attributeMapping); + MetricsTracer.LANGUAGE_ATTRIBUTE, + MetricsTracer.DEFAULT_LANGUAGE); + verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + httpClient.close(); + httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } } From b620ffc15bd96ed4abd93b92edc451b01a0c0779 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 4 Mar 2024 12:32:15 -0500 Subject: [PATCH 13/24] chore: Fix showcase tests --- .../tracing/OpentelemetryMetricsRecorder.java | 12 ++- .../showcase/v1beta1/it/ITOtelMetrics.java | 96 +++++++++++++------ 2 files changed, 78 insertions(+), 30 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index e888dc2f90..f6ead5a65e 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -81,9 +81,17 @@ public OpentelemetryMetricsRecorder(Meter meter) { .setUnit("ms") .build(); this.attemptCountRecorder = - meter.counterBuilder("attempt_count").setDescription("Number of Attempts").build(); + meter + .counterBuilder("attempt_count") + .setDescription("Number of Attempts") + .setUnit("1") + .build(); this.operationCountRecorder = - meter.counterBuilder("operation_count").setDescription("Number of Operations").build(); + meter + .counterBuilder("operation_count") + .setDescription("Number of Operations") + .setUnit("1") + .build(); } /** diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 43ccac1a37..e70551bc45 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -76,6 +76,7 @@ import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.function.Predicate; import org.junit.After; import org.junit.Before; import org.junit.Ignore; @@ -91,10 +92,15 @@ public class ITOtelMetrics { private static final String OPERATION_COUNT = "operation_count"; private static final String ATTEMPT_LATENCY = "attempt_latency"; private static final String OPERATION_LATENCY = "operation_latency"; + private static final int NUM_METRICS = 4; + private static final int NUM_COLLECTION_RETRY_ATTEMPTS = 10; private InMemoryMetricReader inMemoryMetricReader; private EchoClient grpcClient; private EchoClient httpClient; + /** + * Internal class in the Otel Showcases test used to assert that number of status codes recorded. + */ private static class StatusCount { private final Code statusCode; private final int count; @@ -197,7 +203,7 @@ private void verifyMetricData(List metricDataList, int attemptCount) /** * Extract the attributes from MetricData and ensures that default attributes are recorded. Uses * the `OPERATION_COUNT` MetricData to test the attributes. The `OPERATION_COUNT` is only recorded - * once and should only have element of PointData. + * once and should only have one element of PointData. * *

Although the Status attribute is recorded by default on every operation, this helper method * does not verify it. This is because every individual attempt (retry) may have a different @@ -238,23 +244,49 @@ private void verifyStatusAttribute( List pointDataList = new ArrayList<>(attemptCountMetricData.getData().getPoints()); Truth.assertThat(pointDataList.size()).isEqualTo(statusCountList.size()); - for (int i = 0; i < pointDataList.size(); i++) { - LongPointData longPointData = (LongPointData) pointDataList.get(i); - Truth.assertThat(longPointData.getValue()).isEqualTo(statusCountList.get(i).getCount()); - Attributes attributes = longPointData.getAttributes(); - Truth.assertThat(attributes.get(AttributeKey.stringKey(MetricsTracer.STATUS_ATTRIBUTE))) - .isEqualTo(statusCountList.get(i).getStatusCode().toString()); + + // The data for attempt count can come in any order (i.e. the last data point recorded may + // not be the first element in the PointData list). Search for the expected StatusCode from + // the statusCountList and match with the data inside the pointDataList + for (StatusCount statusCount : statusCountList) { + Code statusCode = statusCount.getStatusCode(); + Predicate pointDataPredicate = + x -> + x.getAttributes() + .get(AttributeKey.stringKey(MetricsTracer.STATUS_ATTRIBUTE)) + .equals(statusCode.toString()); + Optional pointDataOptional = + pointDataList.stream().filter(pointDataPredicate).findFirst(); + Truth.assertThat(pointDataOptional.isPresent()).isTrue(); + LongPointData longPointData = (LongPointData) pointDataOptional.get(); + Truth.assertThat(longPointData.getValue()).isEqualTo(statusCount.getCount()); + } + } + + /** + * Attempts to retrieve the metrics from the InMemoryMetricsReader. Sleep every second for at most + * 10s to try and retrieve all the metrics available. If it is unable to retrieve all the metrics, + * return an empty List. + */ + private List getMetricDataList() throws InterruptedException { + for (int i = 0; i < NUM_COLLECTION_RETRY_ATTEMPTS; i++) { + Thread.sleep(1000L); + ArrayList metricData = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + if (metricData.size() == NUM_METRICS) { + return metricData; + } } + return new ArrayList<>(); } @Test - public void testGrpc_operationSucceeded_recordsMetrics() { + public void testGrpc_operationSucceeded_recordsMetrics() throws InterruptedException { int attemptCount = 1; EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_grpc_operation_succeeded").build(); grpcClient.echo(echoRequest); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -271,13 +303,13 @@ public void testGrpc_operationSucceeded_recordsMetrics() { @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test - public void testHttpJson_operationSucceeded_recordsMetrics() { + public void testHttpJson_operationSucceeded_recordsMetrics() throws InterruptedException { int attemptCount = 1; EchoRequest echoRequest = EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); httpClient.echo(echoRequest); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -296,15 +328,18 @@ public void testHttpJson_operationSucceeded_recordsMetrics() { public void testGrpc_operationCancelled_recordsMetrics() throws Exception { int attemptCount = 1; BlockRequest blockRequest = - BlockRequest.newBuilder().setResponseDelay(Duration.newBuilder().setSeconds(5)).build(); + BlockRequest.newBuilder() + .setResponseDelay(Duration.newBuilder().setSeconds(5)) + .setSuccess(BlockResponse.newBuilder().setContent("grpc_operationCancelled")) + .build(); UnaryCallable blockCallable = grpcClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); - // Sleep 3s before cancelling to let the request go through + // Sleep 1s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -328,11 +363,11 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { UnaryCallable blockCallable = httpClient.blockCallable(); ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); - // Sleep 3s before cancelling to let the request go through + // Sleep 1s before cancelling to let the request go through Thread.sleep(1000); blockResponseApiFuture.cancel(true); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -348,7 +383,7 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { } @Test - public void testGrpc_operationFailed_recordsMetrics() { + public void testGrpc_operationFailed_recordsMetrics() throws InterruptedException { int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -361,7 +396,7 @@ public void testGrpc_operationFailed_recordsMetrics() { ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); assertThrows(ExecutionException.class, blockResponseApiFuture::get); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -378,7 +413,7 @@ public void testGrpc_operationFailed_recordsMetrics() { @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test - public void testHttpJson_operationFailed_recordsMetrics() { + public void testHttpJson_operationFailed_recordsMetrics() throws InterruptedException { int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -391,7 +426,7 @@ public void testHttpJson_operationFailed_recordsMetrics() { ApiFuture blockResponseApiFuture = blockCallable.futureCall(blockRequest); assertThrows(ExecutionException.class, blockResponseApiFuture::get); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -445,7 +480,7 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -506,7 +541,7 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -525,7 +560,7 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E } @Test - public void testGrpc_attemptPermanentFailure_recordsMetrics() { + public void testGrpc_attemptPermanentFailure_recordsMetrics() throws InterruptedException { int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -536,7 +571,7 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() { assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -553,7 +588,7 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() { @Ignore("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test - public void testHttpJson_attemptPermanentFailure_recordsMetrics() { + public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws InterruptedException { int attemptCount = 1; Code statusCode = Code.INVALID_ARGUMENT; BlockRequest blockRequest = @@ -564,7 +599,7 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() { assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -582,6 +617,11 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() { @Test public void testGrpc_multipleFailedAttempts_successfulOperation() throws Exception { int attemptCount = 3; + // Disable Jitter on this test to try and ensure that the there are 3 attempts made + // for test. The first two calls should result in a DEADLINE_EXCEEDED exception as + // 0.5s and 1s are too short for the 1s blocking call (1s still requires time for + // the showcase server to respond back to the client). The 3rd and final call (2s) + // should result in an OK Status Code. RetrySettings retrySettings = RetrySettings.newBuilder() .setInitialRpcTimeout(org.threeten.bp.Duration.ofMillis(500L)) @@ -619,7 +659,7 @@ public void testGrpc_multipleFailedAttempts_successfulOperation() throws Excepti grpcClient.block(blockRequest); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = @@ -681,7 +721,7 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc grpcClient.block(blockRequest); - List metricDataList = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricDataList = getMetricDataList(); verifyMetricData(metricDataList, attemptCount); Map attributeMapping = From 07eeb3b1b6c8f0697485678b6ba9a28149030395 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Mon, 4 Mar 2024 15:18:55 -0500 Subject: [PATCH 14/24] chore: Refactor showcase tests --- .../showcase/v1beta1/it/ITOtelMetrics.java | 71 ++++++++++++------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index e70551bc45..1fe33328ca 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -78,6 +78,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -85,6 +86,13 @@ /** * Showcase Test to confirm that metrics are being collected and that the correct metrics are being * recorded. Utilizes an in-memory metric reader to collect the data. + * + *

Every test flows through the same way and runs through the same assertions. First, all th + * metrics are pulled in via {@link #getMetricDataList()} which are polled until all the metrics are + * collected. Then the test will attempt check that reader collected the correct number of data + * points in {@link #verifyPointDataSum(List, int)}. Then, check that the attributes to be collected + * via {@link #verifyStatusAttribute(List, List)}. Finally, check that the status for each attempt + * is correct. */ public class ITOtelMetrics { private static final int DEFAULT_OPERATION_COUNT = 1; @@ -93,7 +101,7 @@ public class ITOtelMetrics { private static final String ATTEMPT_LATENCY = "attempt_latency"; private static final String OPERATION_LATENCY = "operation_latency"; private static final int NUM_METRICS = 4; - private static final int NUM_COLLECTION_RETRY_ATTEMPTS = 10; + private static final int NUM_COLLECTION_FLUSH_ATTEMPTS = 10; private InMemoryMetricReader inMemoryMetricReader; private EchoClient grpcClient; private EchoClient httpClient; @@ -165,7 +173,14 @@ public void cleanup() throws InterruptedException { httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - private void verifyMetricData(List metricDataList, int attemptCount) { + /** + * Iterate through all MetricData elements and check that the number of PointData values matches + * the expected value. A PointData element may have multiple values/ counts inside, so this + * extracts the value/ count from the PointData before summing. + * + *

The expected sum for an operation is `1`. Expected sum for an attempt may be 1+. + */ + private void verifyPointDataSum(List metricDataList, int attemptCount) { for (MetricData metricData : metricDataList) { Data data = metricData.getData(); List points = new ArrayList<>(data.getPoints()); @@ -211,22 +226,23 @@ private void verifyMetricData(List metricDataList, int attemptCount) * attempt. */ private void verifyDefaultMetricsAttributes( - List metricDataList, Map attributeMapping) { + List metricDataList, Map defaultAttributeMapping) { Optional metricDataOptional = metricDataList.stream().filter(x -> x.getName().equals(OPERATION_COUNT)).findAny(); Truth.assertThat(metricDataOptional.isPresent()).isTrue(); MetricData operationCountMetricData = metricDataOptional.get(); List pointDataList = new ArrayList<>(operationCountMetricData.getData().getPoints()); + // Operation Metrics should only have a 1 data point Truth.assertThat(pointDataList.size()).isEqualTo(1); - Attributes attributes = pointDataList.get(0).getAttributes(); - Map, Object> attributesMap = attributes.asMap(); - for (Map.Entry entrySet : attributeMapping.entrySet()) { + Attributes recordedAttributes = pointDataList.get(0).getAttributes(); + Map, Object> recordedAttributesMap = recordedAttributes.asMap(); + for (Map.Entry entrySet : defaultAttributeMapping.entrySet()) { String key = entrySet.getKey(); String value = entrySet.getValue(); AttributeKey stringAttributeKey = AttributeKey.stringKey(key); - Truth.assertThat(attributesMap.containsKey(stringAttributeKey)).isTrue(); - Truth.assertThat(attributesMap.get(stringAttributeKey)).isEqualTo(value); + Truth.assertThat(recordedAttributesMap.containsKey(stringAttributeKey)).isTrue(); + Truth.assertThat(recordedAttributesMap.get(stringAttributeKey)).isEqualTo(value); } } @@ -245,9 +261,10 @@ private void verifyStatusAttribute( List pointDataList = new ArrayList<>(attemptCountMetricData.getData().getPoints()); Truth.assertThat(pointDataList.size()).isEqualTo(statusCountList.size()); - // The data for attempt count can come in any order (i.e. the last data point recorded may - // not be the first element in the PointData list). Search for the expected StatusCode from - // the statusCountList and match with the data inside the pointDataList + // The data for attempt count may not be ordered (i.e. the last data point recorded may be the + // first element in the PointData list). Search for the expected StatusCode from the + // statusCountList + // and match with the data inside the pointDataList for (StatusCount statusCount : statusCountList) { Code statusCode = statusCount.getStatusCode(); Predicate pointDataPredicate = @@ -266,16 +283,17 @@ private void verifyStatusAttribute( /** * Attempts to retrieve the metrics from the InMemoryMetricsReader. Sleep every second for at most * 10s to try and retrieve all the metrics available. If it is unable to retrieve all the metrics, - * return an empty List. + * fail the test. */ private List getMetricDataList() throws InterruptedException { - for (int i = 0; i < NUM_COLLECTION_RETRY_ATTEMPTS; i++) { + for (int i = 0; i < NUM_COLLECTION_FLUSH_ATTEMPTS; i++) { Thread.sleep(1000L); - ArrayList metricData = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); + List metricData = new ArrayList<>(inMemoryMetricReader.collectAllMetrics()); if (metricData.size() == NUM_METRICS) { return metricData; } } + Assert.fail("Unable to collect all the metrics required for the test"); return new ArrayList<>(); } @@ -287,7 +305,7 @@ public void testGrpc_operationSucceeded_recordsMetrics() throws InterruptedExcep grpcClient.echo(echoRequest); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -310,7 +328,7 @@ public void testHttpJson_operationSucceeded_recordsMetrics() throws InterruptedE httpClient.echo(echoRequest); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -340,7 +358,7 @@ public void testGrpc_operationCancelled_recordsMetrics() throws Exception { blockResponseApiFuture.cancel(true); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -368,7 +386,7 @@ public void testHttpJson_operationCancelled_recordsMetrics() throws Exception { blockResponseApiFuture.cancel(true); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -397,7 +415,7 @@ public void testGrpc_operationFailed_recordsMetrics() throws InterruptedExceptio assertThrows(ExecutionException.class, blockResponseApiFuture::get); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -427,7 +445,7 @@ public void testHttpJson_operationFailed_recordsMetrics() throws InterruptedExce assertThrows(ExecutionException.class, blockResponseApiFuture::get); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -481,7 +499,7 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep assertThrows(UnavailableException.class, () -> grpcClient.echo(echoRequest)); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -542,7 +560,7 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E assertThrows(UnavailableException.class, () -> httpClient.echo(echoRequest)); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -572,7 +590,7 @@ public void testGrpc_attemptPermanentFailure_recordsMetrics() throws Interrupted assertThrows(InvalidArgumentException.class, () -> grpcClient.block(blockRequest)); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -600,7 +618,7 @@ public void testHttpJson_attemptPermanentFailure_recordsMetrics() throws Interru assertThrows(InvalidArgumentException.class, () -> httpClient.block(blockRequest)); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -660,7 +678,7 @@ public void testGrpc_multipleFailedAttempts_successfulOperation() throws Excepti grpcClient.block(blockRequest); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -722,7 +740,7 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc grpcClient.block(blockRequest); List metricDataList = getMetricDataList(); - verifyMetricData(metricDataList, attemptCount); + verifyPointDataSum(metricDataList, attemptCount); Map attributeMapping = ImmutableMap.of( @@ -731,6 +749,7 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc MetricsTracer.LANGUAGE_ATTRIBUTE, MetricsTracer.DEFAULT_LANGUAGE); verifyDefaultMetricsAttributes(metricDataList, attributeMapping); + httpClient.close(); httpClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } From 6df9640fd2ff70dcd1036c06e977b53e4e83fb2a Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 11:45:15 -0500 Subject: [PATCH 15/24] chore: Address PR comments --- .../tracing/OpentelemetryMetricsRecorder.java | 22 +++++++++++++------ .../OpentelemetryMetricsRecorderTest.java | 12 +--------- .../showcase/v1beta1/it/ITOtelMetrics.java | 10 +-------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java index f6ead5a65e..4125693d8f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java @@ -30,8 +30,10 @@ package com.google.api.gax.tracing; +import com.google.api.gax.core.GaxProperties; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; @@ -55,7 +57,7 @@ public class OpentelemetryMetricsRecorder implements MetricsRecorder { private final LongCounter attemptCountRecorder; /** - * Creates the following instruments for the following metrics. + * Creates the following instruments for the following metrics: * *

    *
  • Attempt Latency: Histogram @@ -64,31 +66,37 @@ public class OpentelemetryMetricsRecorder implements MetricsRecorder { *
  • Operation Count: Counter *
* - * @param meter Otel Meter used to create the instruments of measurements + * @param openTelemetry OpenTelemetry instance + * @param serviceName Service Name */ - public OpentelemetryMetricsRecorder(Meter meter) { + public OpentelemetryMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { + Meter meter = + openTelemetry + .meterBuilder("Gax-OtelMetrics") + .setInstrumentationVersion(GaxProperties.getGaxVersion()) + .build(); this.attemptLatencyRecorder = meter - .histogramBuilder("attempt_latency") + .histogramBuilder(serviceName + "/attempt_latency") .setDescription("Time an individual attempt took") .setUnit("ms") .build(); this.operationLatencyRecorder = meter - .histogramBuilder("operation_latency") + .histogramBuilder(serviceName + "/operation_latency") .setDescription( "Total time until final operation success or failure, including retries and backoff.") .setUnit("ms") .build(); this.attemptCountRecorder = meter - .counterBuilder("attempt_count") + .counterBuilder(serviceName + "/attempt_count") .setDescription("Number of Attempts") .setUnit("1") .build(); this.operationCountRecorder = meter - .counterBuilder("operation_count") + .counterBuilder(serviceName + "/operation_count") .setDescription("Number of Operations") .setUnit("1") .build(); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java index 8529665b10..10a71a17f7 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java @@ -31,13 +31,11 @@ import static org.junit.Assert.assertThrows; -import com.google.api.gax.core.GaxProperties; import com.google.common.collect.ImmutableMap; import com.google.common.truth.Truth; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.data.Data; @@ -76,15 +74,7 @@ public void setUp() { SdkMeterProvider.builder().registerMetricReader(periodicMetricReader).build(); OpenTelemetry openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - - // Meter Creation - Meter meter = - openTelemetry - .meterBuilder("OtelMetricsRecorderTest") - .setInstrumentationVersion(GaxProperties.getGaxVersion()) - .build(); - - otelMetricsRecorder = new OpentelemetryMetricsRecorder(meter); + otelMetricsRecorder = new OpentelemetryMetricsRecorder(openTelemetry, "MetricsRecorderTest"); } @After diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 1fe33328ca..cb9c2a9d7b 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -138,15 +138,7 @@ private OpentelemetryMetricsRecorder createOtelMetricsRecorder( OpenTelemetry openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - - // Meter Creation - Meter meter = - openTelemetry - .meterBuilder("ShowcaseOtelMetrics") - .setInstrumentationVersion(GaxProperties.getGaxVersion()) - .build(); - // OpenTelemetry Metrics Recorder - return new OpentelemetryMetricsRecorder(meter); + return new OpentelemetryMetricsRecorder(openTelemetry, "ShowcaseTest"); } @Before From edd7ceaf342e2d54d23b3078685e46d4f2a1831c Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 12:45:14 -0500 Subject: [PATCH 16/24] chore: Throw exception if operation completion call has been invoked 1+ times --- .../google/api/gax/tracing/MetricsTracer.java | 31 ++++--- .../api/gax/tracing/MetricsTracerTest.java | 88 ++++++++----------- 2 files changed, 56 insertions(+), 63 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java index 8bcfe7c53d..7938bde82b 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracer.java @@ -40,6 +40,7 @@ import java.util.Map; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -56,28 +57,33 @@ public class MetricsTracer implements ApiTracer { public static final String LANGUAGE_ATTRIBUTE = "language"; public static final String STATUS_ATTRIBUTE = "status"; public static final String DEFAULT_LANGUAGE = "Java"; - + private static final String OPERATION_FINISHED_STATUS_MESSAGE = + "Operation has already been completed"; private Stopwatch attemptTimer; - private final Stopwatch operationTimer = Stopwatch.createStarted(); - private final Map attributes = new HashMap<>(); - - private MetricsRecorder metricsRecorder; + private final MetricsRecorder metricsRecorder; + private final AtomicBoolean operationFinished; public MetricsTracer(MethodName methodName, MetricsRecorder metricsRecorder) { this.attributes.put(METHOD_NAME_ATTRIBUTE, methodName.toString()); this.attributes.put(LANGUAGE_ATTRIBUTE, DEFAULT_LANGUAGE); this.metricsRecorder = metricsRecorder; + this.operationFinished = new AtomicBoolean(); } /** * Signals that the overall operation has finished successfully. The tracer is now considered * closed and should no longer be used. Successful operation adds "OK" value to the status * attribute key. + * + * @throws IllegalStateException if an operation completion call has already been invoked */ @Override public void operationSucceeded() { + if (operationFinished.getAndSet(true)) { + throw new IllegalStateException(OPERATION_FINISHED_STATUS_MESSAGE); + } attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); metricsRecorder.recordOperationLatency( operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); @@ -88,9 +94,14 @@ public void operationSucceeded() { * Signals that the operation was cancelled by the user. The tracer is now considered closed and * should no longer be used. Cancelled operation adds "CANCELLED" value to the status attribute * key. + * + * @throws IllegalStateException if an operation completion call has already been invoked */ @Override public void operationCancelled() { + if (operationFinished.getAndSet(true)) { + throw new IllegalStateException(OPERATION_FINISHED_STATUS_MESSAGE); + } attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString()); metricsRecorder.recordOperationLatency( operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); @@ -101,9 +112,14 @@ public void operationCancelled() { * Signals that the operation was cancelled by the user. The tracer is now considered closed and * should no longer be used. Failed operation extracts the error from the throwable and adds it to * the status attribute key. + * + * @throws IllegalStateException if an operation completion call has already been invoked */ @Override public void operationFailed(Throwable error) { + if (operationFinished.getAndSet(true)) { + throw new IllegalStateException(OPERATION_FINISHED_STATUS_MESSAGE); + } attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); metricsRecorder.recordOperationLatency( operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); @@ -130,7 +146,6 @@ public void attemptStarted(Object request, int attemptNumber) { */ @Override public void attemptSucceeded() { - attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); @@ -142,7 +157,6 @@ public void attemptSucceeded() { */ @Override public void attemptCancelled() { - attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString()); metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); @@ -158,7 +172,6 @@ public void attemptCancelled() { */ @Override public void attemptFailed(Throwable error, Duration delay) { - attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); @@ -173,7 +186,6 @@ public void attemptFailed(Throwable error, Duration delay) { */ @Override public void attemptFailedRetriesExhausted(Throwable error) { - attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); @@ -188,7 +200,6 @@ public void attemptFailedRetriesExhausted(Throwable error) { */ @Override public void attemptPermanentFailure(Throwable error) { - attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); metricsRecorder.recordAttemptLatency(attemptTimer.elapsed(TimeUnit.MILLISECONDS), attributes); metricsRecorder.recordAttemptCount(1, attributes); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java index e30d7f1662..817c53656c 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/MetricsTracerTest.java @@ -30,6 +30,7 @@ package com.google.api.gax.tracing; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.anyDouble; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.verify; @@ -41,7 +42,6 @@ import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.gax.rpc.testing.FakeStatusCode; import com.google.common.collect.ImmutableMap; -import com.google.common.truth.Truth; import java.util.Map; import org.junit.Before; import org.junit.Rule; @@ -56,6 +56,7 @@ @RunWith(JUnit4.class) public class MetricsTracerTest { + private static final String DEFAULT_METHOD_NAME = "fake_service.fake_method"; // stricter way of testing for early detection of unused stubs and argument mismatches @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); @@ -69,16 +70,21 @@ public void setUp() { new MetricsTracer(MethodName.of("fake_service", "fake_method"), metricsRecorder); } + private ImmutableMap getAttributes(Code statusCode) { + return ImmutableMap.of( + "status", + statusCode.toString(), + "method_name", + DEFAULT_METHOD_NAME, + "language", + MetricsTracer.DEFAULT_LANGUAGE); + } + @Test public void testOperationSucceeded_recordsAttributes() { - metricsTracer.operationSucceeded(); - Map attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.OK); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -88,17 +94,12 @@ public void testOperationSucceeded_recordsAttributes() { @Test public void testOperationFailed_recordsAttributes() { - ApiException error0 = new NotFoundException( "invalid argument", null, new FakeStatusCode(Code.INVALID_ARGUMENT), false); metricsTracer.operationFailed(error0); - Map attributes = - ImmutableMap.of( - "status", "INVALID_ARGUMENT", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.INVALID_ARGUMENT); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -108,14 +109,9 @@ public void testOperationFailed_recordsAttributes() { @Test public void testOperationCancelled_recordsAttributes() { - metricsTracer.operationCancelled(); - Map attributes = - ImmutableMap.of( - "status", "CANCELLED", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.CANCELLED); verify(metricsRecorder).recordOperationCount(1, attributes); verify(metricsRecorder).recordOperationLatency(anyDouble(), eq(attributes)); @@ -132,11 +128,7 @@ public void testAttemptSucceeded_recordsAttributes() { metricsTracer.attemptStarted(mockSuccessfulRequest, 0); metricsTracer.attemptSucceeded(); - Map attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.OK); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -156,11 +148,7 @@ public void testAttemptFailed_recordsAttributes() { "invalid argument", null, new FakeStatusCode(Code.INVALID_ARGUMENT), false); metricsTracer.attemptFailed(error0, Duration.ofMillis(2)); - Map attributes = - ImmutableMap.of( - "status", "INVALID_ARGUMENT", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.INVALID_ARGUMENT); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -176,11 +164,7 @@ public void testAttemptCancelled_recordsAttributes() { metricsTracer.attemptStarted(mockCancelledRequest, 0); metricsTracer.attemptCancelled(); - Map attributes = - ImmutableMap.of( - "status", "CANCELLED", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.CANCELLED); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -199,11 +183,7 @@ public void testAttemptFailedRetriesExhausted_recordsAttributes() { "deadline exceeded", null, new FakeStatusCode(Code.DEADLINE_EXCEEDED), false); metricsTracer.attemptFailedRetriesExhausted(error0); - Map attributes = - ImmutableMap.of( - "status", "DEADLINE_EXCEEDED", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.DEADLINE_EXCEEDED); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -213,7 +193,6 @@ public void testAttemptFailedRetriesExhausted_recordsAttributes() { @Test public void testAttemptPermanentFailure_recordsAttributes() { - // initialize mock-request Object mockRequest = new Object(); // Attempt #1 @@ -222,11 +201,7 @@ public void testAttemptPermanentFailure_recordsAttributes() { new NotFoundException("not found", null, new FakeStatusCode(Code.NOT_FOUND), false); metricsTracer.attemptFailedRetriesExhausted(error0); - Map attributes = - ImmutableMap.of( - "status", "NOT_FOUND", - "method_name", "fake_service.fake_method", - "language", "Java"); + Map attributes = getAttributes(Code.NOT_FOUND); verify(metricsRecorder).recordAttemptCount(1, attributes); verify(metricsRecorder).recordAttemptLatency(anyDouble(), eq(attributes)); @@ -235,35 +210,42 @@ public void testAttemptPermanentFailure_recordsAttributes() { } @Test - public void testAddAttributes_recordsAttributes() { + public void testMultipleOperationCalls_throwsError() { + metricsTracer.operationSucceeded(); + IllegalStateException exception1 = + assertThrows(IllegalStateException.class, () -> metricsTracer.operationCancelled()); + assertThat(exception1.getMessage()).isEqualTo("Operation has already been completed"); + IllegalStateException exception2 = + assertThrows(IllegalStateException.class, () -> metricsTracer.operationSucceeded()); + assertThat(exception2.getMessage()).isEqualTo("Operation has already been completed"); + } + @Test + public void testAddAttributes_recordsAttributes() { metricsTracer.addAttributes("FakeTableId", "12345"); - Truth.assertThat(metricsTracer.getAttributes().get("FakeTableId").equals("12345")); + assertThat(metricsTracer.getAttributes().get("FakeTableId")).isEqualTo("12345"); } @Test public void testExtractStatus_errorConversion_apiExceptions() { - ApiException error = new ApiException("fake_error", null, new FakeStatusCode(Code.INVALID_ARGUMENT), false); String errorCode = metricsTracer.extractStatus(error); - assertThat(errorCode).isEqualTo("INVALID_ARGUMENT"); + assertThat(errorCode).isEqualTo(Code.INVALID_ARGUMENT.toString()); } @Test public void testExtractStatus_errorConversion_noError() { - // test "OK", which corresponds to a "null" error. String successCode = metricsTracer.extractStatus(null); - assertThat(successCode).isEqualTo("OK"); + assertThat(successCode).isEqualTo(Code.OK.toString()); } @Test public void testExtractStatus_errorConversion_unknownException() { - // test "UNKNOWN" Throwable unknownException = new RuntimeException(); String errorCode2 = metricsTracer.extractStatus(unknownException); - assertThat(errorCode2).isEqualTo("UNKNOWN"); + assertThat(errorCode2).isEqualTo(Code.UNKNOWN.toString()); } } From 32ee2fe28b5d01c401e5bcb5bc5de8158802503e Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 16:35:01 -0500 Subject: [PATCH 17/24] chore: Address PR comments --- gapic-generator-java-pom-parent/pom.xml | 1 + gax-java/dependencies.properties | 4 - gax-java/gax/BUILD.bazel | 4 - gax-java/gax/pom.xml | 5 - ...java => OpenTelemetryMetricsRecorder.java} | 4 +- .../OpenTelemetryMetricsRecorderTest.java | 212 ++++++++++++++ .../OpentelemetryMetricsRecorderTest.java | 261 ------------------ gax-java/pom.xml | 2 +- .../third-party-dependencies/pom.xml | 10 - showcase/gapic-showcase/pom.xml | 2 + .../showcase/v1beta1/it/ITOtelMetrics.java | 19 +- showcase/pom.xml | 7 - 12 files changed, 227 insertions(+), 304 deletions(-) rename gax-java/gax/src/main/java/com/google/api/gax/tracing/{OpentelemetryMetricsRecorder.java => OpenTelemetryMetricsRecorder.java} (98%) create mode 100644 gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java delete mode 100644 gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java diff --git a/gapic-generator-java-pom-parent/pom.xml b/gapic-generator-java-pom-parent/pom.xml index d4529348c7..883f97629a 100644 --- a/gapic-generator-java-pom-parent/pom.xml +++ b/gapic-generator-java-pom-parent/pom.xml @@ -32,6 +32,7 @@ 2.10.1 32.1.3-jre 3.25.2 + 1.35.0 8 diff --git a/gax-java/dependencies.properties b/gax-java/dependencies.properties index 55f6b8e0f9..3f8c1e8cf8 100644 --- a/gax-java/dependencies.properties +++ b/gax-java/dependencies.properties @@ -40,10 +40,6 @@ maven.com_google_api_grpc_grpc_google_common_protos=com.google.api.grpc:grpc-goo maven.com_google_auth_google_auth_library_oauth2_http=com.google.auth:google-auth-library-oauth2-http:1.23.0 maven.com_google_auth_google_auth_library_credentials=com.google.auth:google-auth-library-credentials:1.23.0 maven.io_opentelemetry_opentelemetry_api=io.opentelemetry:opentelemetry-api:1.34.1 -maven.io_opentelemetry_opentelemetry_sdk=io.opentelemetry:opentelemetry-sdk:1.34.1 -maven.io_opentelemetry_opentelemetry_sdk_common=io.opentelemetry:opentelemetry-sdk-common:1.34.1 -maven.io_opentelemetry_opentelemetry-sdk-metrics=io.opentelemetry:opentelemetry-sdk-metrics:1.34.1 -maven.io_opentelemetry_opentelemetry-sdk-testing=io.opentelemetry:opentelemetry-sdk-testing:1.34.1 maven.io_opencensus_opencensus_api=io.opencensus:opencensus-api:0.31.1 maven.io_opencensus_opencensus_contrib_grpc_metrics=io.opencensus:opencensus-contrib-grpc-metrics:0.31.1 maven.io_opencensus_opencensus_contrib_http_util=io.opencensus:opencensus-contrib-http-util:0.31.1 diff --git a/gax-java/gax/BUILD.bazel b/gax-java/gax/BUILD.bazel index ea9a2a9518..309564c87f 100644 --- a/gax-java/gax/BUILD.bazel +++ b/gax-java/gax/BUILD.bazel @@ -19,7 +19,6 @@ _COMPILE_DEPS = [ "@com_google_errorprone_error_prone_annotations//jar", "@com_google_guava_guava//jar", "@io_opentelemetry_opentelemetry_api//jar", - "@io_opentelemetry_opentelemetry-sdk-metrics//jar", "@io_opencensus_opencensus_api//jar", "@io_opencensus_opencensus_contrib_http_util//jar", "@io_grpc_grpc_java//context:context", @@ -40,9 +39,6 @@ _TEST_COMPILE_DEPS = [ "@net_bytebuddy_byte_buddy//jar", "@org_objenesis_objenesis//jar", "@com_googlecode_java_diff_utils_diffutils//jar", - "@io_opentelemetry_opentelemetry_sdk//jar", - "@io_opentelemetry_opentelemetry-sdk-testing//jar", - "@io_opentelemetry_opentelemetry_sdk_common//jar" ] java_library( diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 1c2082311a..5b82f001bc 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -61,11 +61,6 @@ io.opentelemetry opentelemetry-api - - io.opentelemetry - opentelemetry-sdk-testing - test - diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java similarity index 98% rename from gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java rename to gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java index 4125693d8f..82bd872cd0 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java @@ -50,7 +50,7 @@ * error). A single call (i.e. `EchoClient.echo()`) should have an operation_count of 1 and may have * an attempt_count of 1+ (depending on the retry configurations). */ -public class OpentelemetryMetricsRecorder implements MetricsRecorder { +public class OpenTelemetryMetricsRecorder implements MetricsRecorder { private final DoubleHistogram attemptLatencyRecorder; private final DoubleHistogram operationLatencyRecorder; private final LongCounter operationCountRecorder; @@ -69,7 +69,7 @@ public class OpentelemetryMetricsRecorder implements MetricsRecorder { * @param openTelemetry OpenTelemetry instance * @param serviceName Service Name */ - public OpentelemetryMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { + public OpenTelemetryMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { Meter meter = openTelemetry .meterBuilder("Gax-OtelMetrics") diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java new file mode 100644 index 0000000000..a97fbba9b0 --- /dev/null +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java @@ -0,0 +1,212 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.tracing; + +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import com.google.common.collect.ImmutableMap; +import com.google.common.truth.Truth; +import com.google.rpc.Code; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.DoubleHistogramBuilder; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongCounterBuilder; +import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.MeterBuilder; +import java.util.Map; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockito.quality.Strictness; + +@RunWith(JUnit4.class) +public class OpenTelemetryMetricsRecorderTest { + private static final String SERVICE_NAME = "OtelRecorderTest"; + private static final String ATTEMPT_COUNT = SERVICE_NAME + "/attempt_count"; + private static final String OPERATION_COUNT = SERVICE_NAME + "/operation_count"; + private static final String ATTEMPT_LATENCY = SERVICE_NAME + "/attempt_latency"; + private static final String OPERATION_LATENCY = SERVICE_NAME + "/operation_latency"; + // stricter way of testing for early detection of unused stubs and argument mismatches + @Rule + public final MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); + + private OpenTelemetryMetricsRecorder otelMetricsRecorder; + @Mock private OpenTelemetry openTelemetry; + @Mock private Meter meter; + @Mock private MeterBuilder meterBuilder; + @Mock private LongCounter attemptCountRecorder; + @Mock private LongCounterBuilder attemptCountRecorderBuilder; + @Mock private DoubleHistogramBuilder attemptLatencyRecorderBuilder; + @Mock private DoubleHistogram attemptLatencyRecorder; + @Mock private DoubleHistogram operationLatencyRecorder; + @Mock private DoubleHistogramBuilder operationLatencyRecorderBuilder; + @Mock private LongCounter operationCountRecorder; + @Mock private LongCounterBuilder operationCountRecorderBuilder; + + @Before + public void setUp() { + Mockito.when(openTelemetry.meterBuilder(Mockito.anyString())).thenReturn(meterBuilder); + Mockito.when(meterBuilder.setInstrumentationVersion(Mockito.anyString())) + .thenReturn(meterBuilder); + Mockito.when(meterBuilder.build()).thenReturn(meter); + // setup mocks for all the recorders using chained mocking + setupAttemptCountRecorder(); + setupAttemptLatencyRecorder(); + setupOperationLatencyRecorder(); + setupOperationCountRecorder(); + + otelMetricsRecorder = new OpenTelemetryMetricsRecorder(openTelemetry, SERVICE_NAME); + } + + private Map getAttributes(Code statusCode) { + return ImmutableMap.of( + "status", + statusCode.toString(), + "method_name", + "fake_service.fake_method", + "language", + MetricsTracer.DEFAULT_LANGUAGE); + } + + @Test + public void testAttemptCountRecorder_recordsAttributes() { + Map attributes = getAttributes(Code.OK); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordAttemptCount(1, attributes); + + verify(attemptCountRecorder).add(1, otelAttributes); + verifyNoMoreInteractions(attemptCountRecorder); + } + + @Test + public void testAttemptLatencyRecorder_recordsAttributes() { + Map attributes = getAttributes(Code.NOT_FOUND); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordAttemptLatency(1.1, attributes); + + verify(attemptLatencyRecorder).record(1.1, otelAttributes); + verifyNoMoreInteractions(attemptLatencyRecorder); + } + + @Test + public void testOperationCountRecorder_recordsAttributes() { + Map attributes = getAttributes(Code.OK); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordOperationCount(1, attributes); + + verify(operationCountRecorder).add(1, otelAttributes); + verifyNoMoreInteractions(operationCountRecorder); + } + + @Test + public void testOperationLatencyRecorder_recordsAttributes() { + Map attributes = getAttributes(Code.INVALID_ARGUMENT); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + otelMetricsRecorder.recordOperationLatency(1.7, attributes); + + verify(operationLatencyRecorder).record(1.7, otelAttributes); + verifyNoMoreInteractions(operationLatencyRecorder); + } + + @Test + public void testToOtelAttributes_correctConversion() { + Map attributes = getAttributes(Code.OK); + + Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); + + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))) + .isEqualTo("fake_service.fake_method"); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); + } + + @Test + public void testToOtelAttributes_nullInput() { + Throwable thrown = + assertThrows(NullPointerException.class, () -> otelMetricsRecorder.toOtelAttributes(null)); + Truth.assertThat(thrown).hasMessageThat().contains("Attributes map cannot be null"); + } + + private void setupAttemptCountRecorder() { + // Configure chained mocking for AttemptCountRecorder + Mockito.when(meter.counterBuilder(ATTEMPT_COUNT)).thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.setDescription(Mockito.anyString())) + .thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.setUnit(Mockito.anyString())) + .thenReturn(attemptCountRecorderBuilder); + Mockito.when(attemptCountRecorderBuilder.build()).thenReturn(attemptCountRecorder); + } + + private void setupOperationCountRecorder() { + // Configure chained mocking for operationCountRecorder + Mockito.when(meter.counterBuilder(OPERATION_COUNT)).thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.setDescription(Mockito.anyString())) + .thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.setUnit("1")) + .thenReturn(operationCountRecorderBuilder); + Mockito.when(operationCountRecorderBuilder.build()).thenReturn(operationCountRecorder); + } + + private void setupAttemptLatencyRecorder() { + // Configure chained mocking for attemptLatencyRecorder + Mockito.when(meter.histogramBuilder(ATTEMPT_LATENCY)).thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.setDescription(Mockito.anyString())) + .thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.setUnit(Mockito.anyString())) + .thenReturn(attemptLatencyRecorderBuilder); + Mockito.when(attemptLatencyRecorderBuilder.build()).thenReturn(attemptLatencyRecorder); + } + + private void setupOperationLatencyRecorder() { + // Configure chained mocking for operationLatencyRecorder + Mockito.when(meter.histogramBuilder(OPERATION_LATENCY)) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when(operationLatencyRecorderBuilder.setDescription(Mockito.anyString())) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when(operationLatencyRecorderBuilder.setUnit("ms")) + .thenReturn(operationLatencyRecorderBuilder); + Mockito.when(operationLatencyRecorderBuilder.build()).thenReturn(operationLatencyRecorder); + } +} diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java deleted file mode 100644 index 10a71a17f7..0000000000 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpentelemetryMetricsRecorderTest.java +++ /dev/null @@ -1,261 +0,0 @@ -/* - * Copyright 2024 Google LLC - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google LLC nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package com.google.api.gax.tracing; - -import static org.junit.Assert.assertThrows; - -import com.google.common.collect.ImmutableMap; -import com.google.common.truth.Truth; -import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.data.Data; -import io.opentelemetry.sdk.metrics.data.HistogramPointData; -import io.opentelemetry.sdk.metrics.data.LongPointData; -import io.opentelemetry.sdk.metrics.data.MetricData; -import io.opentelemetry.sdk.metrics.data.PointData; -import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; -import io.opentelemetry.sdk.testing.exporter.InMemoryMetricExporter; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class OpentelemetryMetricsRecorderTest { - private OpentelemetryMetricsRecorder otelMetricsRecorder; - private static InMemoryMetricExporter exporter; - private static PeriodicMetricReader periodicMetricReader; - - @BeforeClass - public static void setUpBeforeClass() { - exporter = InMemoryMetricExporter.create(); - periodicMetricReader = PeriodicMetricReader.create(exporter); - } - - @Before - public void setUp() { - SdkMeterProvider sdkMeterProvider = - SdkMeterProvider.builder().registerMetricReader(periodicMetricReader).build(); - OpenTelemetry openTelemetry = - OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - otelMetricsRecorder = new OpentelemetryMetricsRecorder(openTelemetry, "MetricsRecorderTest"); - } - - @After - public void cleanUp() { - exporter.reset(); - } - - @AfterClass - public static void cleanUpAfterClass() { - exporter.shutdown(); - } - - private void verifyAttributesRecorded( - Attributes pointDataAttributes, Map attributes) { - Map, Object> attributesMap = pointDataAttributes.asMap(); - for (Map.Entry, Object> entrySet : attributesMap.entrySet()) { - String key = entrySet.getKey().getKey(); - String value = entrySet.getValue().toString(); - Truth.assertThat(attributes.containsKey(key)).isTrue(); - Truth.assertThat(attributes.get(key)).isEqualTo(value); - } - } - - private List extractPointDataList() { - List finishedMetricItems = exporter.getFinishedMetricItems(); - Truth.assertThat(finishedMetricItems.size()).isEqualTo(1); - MetricData metricData = finishedMetricItems.get(0); - Data data = metricData.getData(); - return new ArrayList<>(data.getPoints()); - } - - @Test - public void recordAttemptCount_singleAttempt() { - Map attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordAttemptCount(1, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); - LongPointData longPointData = (LongPointData) pointData.get(0); - Truth.assertThat(longPointData.getValue()).isEqualTo(1); - - Attributes pointDataAttributes = longPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void recordAttemptCount_multipleAttempt() { - Map attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordAttemptCount(1, attributes); - otelMetricsRecorder.recordAttemptCount(1, attributes); - otelMetricsRecorder.recordAttemptCount(1, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); - LongPointData longPointData = (LongPointData) pointData.get(0); - Truth.assertThat(longPointData.getValue()).isEqualTo(3); - - Attributes pointDataAttributes = longPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void recordAttemptLatency_singleAttempt() { - Map attributes = - ImmutableMap.of( - "status", "NOT_FOUND", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordAttemptLatency(1.1, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); - HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); - Truth.assertThat(histogramPointData.getCount()).isEqualTo(1); - - Attributes pointDataAttributes = histogramPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void recordAttemptLatency_multipleAttempts() { - Map attributes = - ImmutableMap.of( - "status", "NOT_FOUND", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordAttemptLatency(1.1, attributes); - otelMetricsRecorder.recordAttemptLatency(1.2, attributes); - otelMetricsRecorder.recordAttemptLatency(1.3, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); - HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); - Truth.assertThat(histogramPointData.getCount()).isEqualTo(3); - - Attributes pointDataAttributes = histogramPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void recordOperationCount() { - Map attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordOperationCount(1, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(LongPointData.class); - LongPointData longPointData = (LongPointData) pointData.get(0); - Truth.assertThat(longPointData.getValue()).isEqualTo(1); - - Attributes pointDataAttributes = longPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void recordOperationLatency() { - Map attributes = - ImmutableMap.of( - "status", "INVALID_ARGUMENT", - "method_name", "fake_service.fake_method", - "language", "Java"); - - otelMetricsRecorder.recordOperationLatency(1.7, attributes); - periodicMetricReader.forceFlush(); - - List pointData = extractPointDataList(); - Truth.assertThat(pointData.size()).isEqualTo(1); - Truth.assertThat(pointData.get(0)).isInstanceOf(HistogramPointData.class); - HistogramPointData histogramPointData = (HistogramPointData) pointData.get(0); - Truth.assertThat(histogramPointData.getCount()).isEqualTo(1); - - Attributes pointDataAttributes = histogramPointData.getAttributes(); - verifyAttributesRecorded(pointDataAttributes, attributes); - } - - @Test - public void toOtelAttributes_correctConversion() { - ImmutableMap attributes = - ImmutableMap.of( - "status", "OK", - "method_name", "fake_service.fake_method", - "language", "Java"); - - Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); - - Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); - Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))) - .isEqualTo("fake_service.fake_method"); - Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); - } - - @Test - public void toOtelAttributes_nullInput() { - NullPointerException exception = - assertThrows(NullPointerException.class, () -> otelMetricsRecorder.toOtelAttributes(null)); - Truth.assertThat(exception).hasMessageThat().contains("Attributes map cannot be null"); - } -} diff --git a/gax-java/pom.xml b/gax-java/pom.xml index 4c8656e8ac..c7f2295445 100644 --- a/gax-java/pom.xml +++ b/gax-java/pom.xml @@ -161,7 +161,7 @@ io.opentelemetry opentelemetry-bom - 1.34.1 + ${opentelemetry.version} pom import diff --git a/java-shared-dependencies/third-party-dependencies/pom.xml b/java-shared-dependencies/third-party-dependencies/pom.xml index 67177a9439..9e6580d4a6 100644 --- a/java-shared-dependencies/third-party-dependencies/pom.xml +++ b/java-shared-dependencies/third-party-dependencies/pom.xml @@ -182,16 +182,6 @@ j2objc-annotations ${j2objc-annotations.version} - - io.opentelemetry - opentelemetry-api - ${opentelemetry.version} - - - io.opentelemetry - opentelemetry-context - ${opentelemetry.version} - \ No newline at end of file diff --git a/showcase/gapic-showcase/pom.xml b/showcase/gapic-showcase/pom.xml index a38b53bc69..c9bf276a6e 100644 --- a/showcase/gapic-showcase/pom.xml +++ b/showcase/gapic-showcase/pom.xml @@ -182,6 +182,8 @@ grpc-google-iam-v1 test + + io.opentelemetry opentelemetry-api diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index cb9c2a9d7b..13228e84a4 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -34,7 +34,6 @@ import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.core.ApiFuture; -import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.NoCredentialsProvider; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.InvalidArgumentException; @@ -43,7 +42,7 @@ import com.google.api.gax.rpc.UnavailableException; import com.google.api.gax.tracing.MetricsTracer; import com.google.api.gax.tracing.MetricsTracerFactory; -import com.google.api.gax.tracing.OpentelemetryMetricsRecorder; +import com.google.api.gax.tracing.OpenTelemetryMetricsRecorder; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -61,7 +60,6 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.data.Data; @@ -96,10 +94,11 @@ */ public class ITOtelMetrics { private static final int DEFAULT_OPERATION_COUNT = 1; - private static final String ATTEMPT_COUNT = "attempt_count"; - private static final String OPERATION_COUNT = "operation_count"; - private static final String ATTEMPT_LATENCY = "attempt_latency"; - private static final String OPERATION_LATENCY = "operation_latency"; + private static final String SERVICE_NAME = "ShowcaseTest"; + private static final String ATTEMPT_COUNT = SERVICE_NAME + "/attempt_count"; + private static final String OPERATION_COUNT = SERVICE_NAME + "/operation_count"; + private static final String ATTEMPT_LATENCY = SERVICE_NAME+ "/attempt_latency"; + private static final String OPERATION_LATENCY = SERVICE_NAME + "/operation_latency"; private static final int NUM_METRICS = 4; private static final int NUM_COLLECTION_FLUSH_ATTEMPTS = 10; private InMemoryMetricReader inMemoryMetricReader; @@ -131,20 +130,20 @@ public int getCount() { } } - private OpentelemetryMetricsRecorder createOtelMetricsRecorder( + private OpenTelemetryMetricsRecorder createOtelMetricsRecorder( InMemoryMetricReader inMemoryMetricReader) { SdkMeterProvider sdkMeterProvider = SdkMeterProvider.builder().registerMetricReader(inMemoryMetricReader).build(); OpenTelemetry openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); - return new OpentelemetryMetricsRecorder(openTelemetry, "ShowcaseTest"); + return new OpenTelemetryMetricsRecorder(openTelemetry, SERVICE_NAME); } @Before public void setup() throws Exception { inMemoryMetricReader = InMemoryMetricReader.create(); - OpentelemetryMetricsRecorder otelMetricsRecorder = + OpenTelemetryMetricsRecorder otelMetricsRecorder = createOtelMetricsRecorder(inMemoryMetricReader); grpcClient = TestClientInitializer.createGrpcEchoClientOpentelemetry( diff --git a/showcase/pom.xml b/showcase/pom.xml index f33f9565aa..2ea361f577 100644 --- a/showcase/pom.xml +++ b/showcase/pom.xml @@ -38,13 +38,6 @@ pom import - - io.opentelemetry - opentelemetry-bom - 1.34.1 - pom - import - com.google.api.grpc proto-gapic-showcase-v1beta1 From 825175e5e55afefc9d7ef44b0692d6822b35e4fb Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 16:49:07 -0500 Subject: [PATCH 18/24] chore: Add otel bom to gapic-generator-java bom --- gapic-generator-java-bom/pom.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gapic-generator-java-bom/pom.xml b/gapic-generator-java-bom/pom.xml index 026e0b016b..6d44de60f6 100644 --- a/gapic-generator-java-bom/pom.xml +++ b/gapic-generator-java-bom/pom.xml @@ -70,6 +70,13 @@ pom import + + io.opentelemetry + opentelemetry-bom + ${opentelemetry.version} + pom + import + From 09ea967df0584736f482be758adf380a6196e3c8 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 17:03:59 -0500 Subject: [PATCH 19/24] chore: Address PR comments --- .../google/api/gax/rpc/ClientSettings.java | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java index d6b59f336f..25929756f5 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientSettings.java @@ -31,10 +31,8 @@ import com.google.api.core.ApiClock; import com.google.api.core.ApiFunction; -import com.google.api.core.BetaApi; import com.google.api.gax.core.CredentialsProvider; import com.google.api.gax.core.ExecutorProvider; -import com.google.api.gax.tracing.ApiTracerFactory; import com.google.common.base.MoreObjects; import java.io.IOException; import java.util.concurrent.Executor; @@ -122,16 +120,6 @@ public final String getGdchApiAudience() { return stubSettings.getGdchApiAudience(); } - /** - * Gets the configured {@link ApiTracerFactory} that will be used to generate traces for - * operations. - */ - @BetaApi("The surface for tracing is not stable yet and may change in the future.") - @Nonnull - public ApiTracerFactory getTracerFactory() { - return stubSettings.getTracerFactory(); - } - public String toString() { return MoreObjects.toStringHelper(this) .add("executorProvider", getExecutorProvider()) @@ -296,16 +284,6 @@ public B setGdchApiAudience(@Nullable String gdchApiAudience) { return self(); } - /** - * Sets the ApiTracerFactory for the client instance. To enable default metrics, users need to - * create an instance of metricsRecorder and pass it to the metricsTracerFactory, and set it - * here. - */ - public B setTracerFactory(@Nullable ApiTracerFactory tracerFactory) { - stubSettings.setTracerFactory(tracerFactory); - return self(); - } - /** * Gets the ExecutorProvider that was previously set on this Builder. This ExecutorProvider is * to use for running asynchronous API call logic (such as retries and long-running operations), From ea5a776925407cd37891b3d7ebd13c932bfb9f91 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Tue, 5 Mar 2024 18:01:59 -0500 Subject: [PATCH 20/24] chore: Fix showcase tests --- .../showcase/v1beta1/it/ITOtelMetrics.java | 60 +++++++++++++++---- .../it/util/TestClientInitializer.java | 53 ++++++++++------ 2 files changed, 82 insertions(+), 31 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 13228e84a4..317897710b 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -55,6 +55,7 @@ import com.google.showcase.v1beta1.EchoRequest; import com.google.showcase.v1beta1.EchoSettings; import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import com.google.showcase.v1beta1.stub.EchoStub; import com.google.showcase.v1beta1.stub.EchoStubSettings; import io.grpc.ManagedChannelBuilder; import io.opentelemetry.api.OpenTelemetry; @@ -97,7 +98,7 @@ public class ITOtelMetrics { private static final String SERVICE_NAME = "ShowcaseTest"; private static final String ATTEMPT_COUNT = SERVICE_NAME + "/attempt_count"; private static final String OPERATION_COUNT = SERVICE_NAME + "/operation_count"; - private static final String ATTEMPT_LATENCY = SERVICE_NAME+ "/attempt_latency"; + private static final String ATTEMPT_LATENCY = SERVICE_NAME + "/attempt_latency"; private static final String OPERATION_LATENCY = SERVICE_NAME + "/operation_latency"; private static final int NUM_METRICS = 4; private static final int NUM_COLLECTION_FLUSH_ATTEMPTS = 10; @@ -473,15 +474,24 @@ public void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Excep grpcEchoSettings .toBuilder() .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory( - new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) .setTransportChannelProvider( EchoSettings.defaultGrpcTransportProviderBuilder() .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) .build()) .setEndpoint("localhost:7469") .build(); - EchoClient grpcClient = EchoClient.create(grpcEchoSettings); + + EchoStubSettings echoStubSettings = + (EchoStubSettings) + grpcEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .build(); + EchoStub stub = echoStubSettings.createStub(); + EchoClient grpcClient = EchoClient.create(stub); + EchoRequest echoRequest = EchoRequest.newBuilder() .setError(Status.newBuilder().setCode(statusCode.ordinal()).build()) @@ -531,8 +541,6 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E httpJsonEchoSettings .toBuilder() .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory( - new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) .setTransportChannelProvider( EchoSettings.defaultHttpJsonTransportProviderBuilder() .setHttpTransport( @@ -541,7 +549,16 @@ public void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws E .build()) .build(); - EchoClient httpClient = EchoClient.create(httpJsonEchoSettings); + EchoStubSettings echoStubSettings = + (EchoStubSettings) + httpJsonEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .build(); + EchoStub stub = echoStubSettings.createStub(); + EchoClient httpClient = EchoClient.create(stub); EchoRequest echoRequest = EchoRequest.newBuilder() @@ -650,15 +667,24 @@ public void testGrpc_multipleFailedAttempts_successfulOperation() throws Excepti grpcEchoSettings .toBuilder() .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory( - new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) .setTransportChannelProvider( EchoSettings.defaultGrpcTransportProviderBuilder() .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) .build()) .setEndpoint("localhost:7469") .build(); - EchoClient grpcClient = EchoClient.create(grpcEchoSettings); + + EchoStubSettings echoStubSettings = + (EchoStubSettings) + grpcEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .build(); + EchoStub stub = echoStubSettings.createStub(); + + EchoClient grpcClient = EchoClient.create(stub); BlockRequest blockRequest = BlockRequest.newBuilder() @@ -710,8 +736,6 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc httpJsonEchoSettings .toBuilder() .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory( - new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) .setTransportChannelProvider( EchoSettings.defaultHttpJsonTransportProviderBuilder() .setHttpTransport( @@ -720,7 +744,17 @@ public void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exc .build()) .build(); - EchoClient httpClient = EchoClient.create(httpJsonEchoSettings); + EchoStubSettings echoStubSettings = + (EchoStubSettings) + httpJsonEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory( + new MetricsTracerFactory(createOtelMetricsRecorder(inMemoryMetricReader))) + .build(); + EchoStub stub = echoStubSettings.createStub(); + + EchoClient httpClient = EchoClient.create(stub); BlockRequest blockRequest = BlockRequest.newBuilder() diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java index ff3fb0c82d..f396a0fc6f 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/util/TestClientInitializer.java @@ -33,6 +33,7 @@ import com.google.showcase.v1beta1.IdentityClient; import com.google.showcase.v1beta1.IdentitySettings; import com.google.showcase.v1beta1.WaitRequest; +import com.google.showcase.v1beta1.stub.EchoStub; import com.google.showcase.v1beta1.stub.EchoStubSettings; import io.grpc.ClientInterceptor; import io.grpc.ManagedChannelBuilder; @@ -286,13 +287,37 @@ public static ComplianceClient createHttpJsonComplianceClient( return ComplianceClient.create(httpJsonComplianceSettings); } + public static EchoClient createGrpcEchoClientOpentelemetry(ApiTracerFactory metricsTracerFactory) + throws Exception { + + EchoSettings grpcEchoSettings = + EchoSettings.newBuilder() + .setCredentialsProvider(NoCredentialsProvider.create()) + .setTransportChannelProvider( + EchoSettings.defaultGrpcTransportProviderBuilder() + .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) + .build()) + .setEndpoint("localhost:7469") + .build(); + + EchoStubSettings echoStubSettings = + (EchoStubSettings) + grpcEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory(metricsTracerFactory) + .build(); + EchoStub stub = echoStubSettings.createStub(); + + return EchoClient.create(stub); + } + public static EchoClient createHttpJsonEchoClientOpentelemetry( ApiTracerFactory metricsTracerFactory) throws Exception { EchoSettings httpJsonEchoSettings = EchoSettings.newHttpJsonBuilder() .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory(metricsTracerFactory) .setTransportChannelProvider( EchoSettings.defaultHttpJsonTransportProviderBuilder() .setHttpTransport( @@ -301,23 +326,15 @@ public static EchoClient createHttpJsonEchoClientOpentelemetry( .build()) .build(); - return EchoClient.create(httpJsonEchoSettings); - } - - public static EchoClient createGrpcEchoClientOpentelemetry(ApiTracerFactory metricsTracerFactory) - throws Exception { - - EchoSettings grpcEchoSettings = - EchoSettings.newBuilder() - .setCredentialsProvider(NoCredentialsProvider.create()) - .setTracerFactory(metricsTracerFactory) - .setTransportChannelProvider( - EchoSettings.defaultGrpcTransportProviderBuilder() - .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) - .build()) - .setEndpoint("localhost:7469") - .build(); + EchoStubSettings echoStubSettings = + (EchoStubSettings) + httpJsonEchoSettings + .getStubSettings() + .toBuilder() + .setTracerFactory(metricsTracerFactory) + .build(); + EchoStub stub = echoStubSettings.createStub(); - return EchoClient.create(grpcEchoSettings); + return EchoClient.create(stub); } } From 998eb6859e74d2d405c08cfe1a2569068fb7df6a Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 6 Mar 2024 10:41:14 -0500 Subject: [PATCH 21/24] Update gax-java/dependencies.properties Co-authored-by: Blake Li --- gax-java/dependencies.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/dependencies.properties b/gax-java/dependencies.properties index 3f8c1e8cf8..22cbb4b158 100644 --- a/gax-java/dependencies.properties +++ b/gax-java/dependencies.properties @@ -39,7 +39,7 @@ maven.com_google_api_grpc_proto_google_common_protos=com.google.api.grpc:proto-g maven.com_google_api_grpc_grpc_google_common_protos=com.google.api.grpc:grpc-google-common-protos:2.33.0 maven.com_google_auth_google_auth_library_oauth2_http=com.google.auth:google-auth-library-oauth2-http:1.23.0 maven.com_google_auth_google_auth_library_credentials=com.google.auth:google-auth-library-credentials:1.23.0 -maven.io_opentelemetry_opentelemetry_api=io.opentelemetry:opentelemetry-api:1.34.1 +maven.io_opentelemetry_opentelemetry_api=io.opentelemetry:opentelemetry-api:1.35.0 maven.io_opencensus_opencensus_api=io.opencensus:opencensus-api:0.31.1 maven.io_opencensus_opencensus_contrib_grpc_metrics=io.opencensus:opencensus-contrib-grpc-metrics:0.31.1 maven.io_opencensus_opencensus_contrib_http_util=io.opencensus:opencensus-contrib-http-util:0.31.1 From 6c7bca360a33938539ab43056b94316f1fc47dfa Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Wed, 6 Mar 2024 10:44:36 -0500 Subject: [PATCH 22/24] chore: Use gax-java as instrument scope name --- .../api/gax/tracing/OpenTelemetryMetricsRecorder.java | 2 +- .../gax/tracing/OpenTelemetryMetricsRecorderTest.java | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java index 82bd872cd0..dd12becc6f 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java @@ -72,7 +72,7 @@ public class OpenTelemetryMetricsRecorder implements MetricsRecorder { public OpenTelemetryMetricsRecorder(OpenTelemetry openTelemetry, String serviceName) { Meter meter = openTelemetry - .meterBuilder("Gax-OtelMetrics") + .meterBuilder("gax-java") .setInstrumentationVersion(GaxProperties.getGaxVersion()) .build(); this.attemptLatencyRecorder = diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java index a97fbba9b0..b0ce0cb927 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorderTest.java @@ -64,6 +64,7 @@ public class OpenTelemetryMetricsRecorderTest { private static final String OPERATION_COUNT = SERVICE_NAME + "/operation_count"; private static final String ATTEMPT_LATENCY = SERVICE_NAME + "/attempt_latency"; private static final String OPERATION_LATENCY = SERVICE_NAME + "/operation_latency"; + private static final String DEFAULT_METHOD_NAME = "fake_service.fake_method"; // stricter way of testing for early detection of unused stubs and argument mismatches @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); @@ -101,7 +102,7 @@ private Map getAttributes(Code statusCode) { "status", statusCode.toString(), "method_name", - "fake_service.fake_method", + DEFAULT_METHOD_NAME, "language", MetricsTracer.DEFAULT_LANGUAGE); } @@ -156,10 +157,12 @@ public void testToOtelAttributes_correctConversion() { Attributes otelAttributes = otelMetricsRecorder.toOtelAttributes(attributes); - Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))).isEqualTo("OK"); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("status"))) + .isEqualTo(Code.OK.toString()); Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("method_name"))) - .isEqualTo("fake_service.fake_method"); - Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))).isEqualTo("Java"); + .isEqualTo(DEFAULT_METHOD_NAME); + Truth.assertThat(otelAttributes.get(AttributeKey.stringKey("language"))) + .isEqualTo(MetricsTracer.DEFAULT_LANGUAGE); } @Test From fad8094e52fe4534ee34c324480f6be70dc44d6e Mon Sep 17 00:00:00 2001 From: Blake Li Date: Thu, 14 Mar 2024 15:14:17 -0400 Subject: [PATCH 23/24] Update gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java --- .../google/api/gax/tracing/OpenTelemetryMetricsRecorder.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java index dd12becc6f..5458616a09 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java @@ -50,6 +50,8 @@ * error). A single call (i.e. `EchoClient.echo()`) should have an operation_count of 1 and may have * an attempt_count of 1+ (depending on the retry configurations). */ +@BetaApi +@InternalApi public class OpenTelemetryMetricsRecorder implements MetricsRecorder { private final DoubleHistogram attemptLatencyRecorder; private final DoubleHistogram operationLatencyRecorder; From a79e4e72611b76ad46a91040afaf4aaa3ae14a2c Mon Sep 17 00:00:00 2001 From: Blake Li Date: Thu, 14 Mar 2024 15:28:06 -0400 Subject: [PATCH 24/24] Update gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java --- .../google/api/gax/tracing/OpenTelemetryMetricsRecorder.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java index 5458616a09..fdf1dd2d09 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryMetricsRecorder.java @@ -30,6 +30,8 @@ package com.google.api.gax.tracing; +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; import com.google.api.gax.core.GaxProperties; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions;