From d3abd2abdd06b27650b0e12a6aca2354e1b39a92 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Mar 2024 03:05:19 +0000 Subject: [PATCH 1/3] Remove use of createWithClient in src code --- .../cloud/opentelemetry/trace/InternalTraceExporter.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java index dfd84fd9..0cc13dba 100644 --- a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java +++ b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java @@ -27,6 +27,7 @@ import com.google.cloud.trace.v2.TraceServiceClient; import com.google.cloud.trace.v2.TraceServiceSettings; import com.google.cloud.trace.v2.stub.TraceServiceStub; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.devtools.cloudtrace.v2.AttributeValue; import com.google.devtools.cloudtrace.v2.ProjectName; @@ -56,7 +57,8 @@ class InternalTraceExporter implements SpanExporter { Map.of("User-Agent", "opentelemetry-operations-java/" + TraceVersions.EXPORTER_VERSION); private static final HeaderProvider HEADER_PROVIDER = () -> HEADERS; - private static InternalTraceExporter createWithClient( + @VisibleForTesting + static InternalTraceExporter createWithClient( String projectId, CloudTraceClient cloudTraceClient, ImmutableMap attributeMappings, @@ -104,7 +106,7 @@ static SpanExporter createWithConfiguration(TraceConfiguration configuration) th configuration.getAttributeMapping(), configuration.getFixedAttributes()); } - return InternalTraceExporter.createWithClient( + return new InternalTraceExporter( projectId, new CloudTraceClientImpl(TraceServiceClient.create(stub)), configuration.getAttributeMapping(), From afc4aaf8afebb082833b60b146e3d3df940fc92e Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Mar 2024 19:25:13 +0000 Subject: [PATCH 2/3] Remove usgae of deprecated service stub API --- .../trace/InternalTraceExporter.java | 63 ++++++++----------- .../trace/TraceConfiguration.java | 20 ------ .../trace/TraceConfigurationTest.java | 6 +- .../trace/TraceExporterTest.java | 28 +++++++-- 4 files changed, 54 insertions(+), 63 deletions(-) diff --git a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java index 0cc13dba..d14143ae 100644 --- a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java +++ b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/InternalTraceExporter.java @@ -26,7 +26,6 @@ import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.trace.v2.TraceServiceClient; import com.google.cloud.trace.v2.TraceServiceSettings; -import com.google.cloud.trace.v2.stub.TraceServiceStub; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.devtools.cloudtrace.v2.AttributeValue; @@ -69,46 +68,36 @@ static InternalTraceExporter createWithClient( static SpanExporter createWithConfiguration(TraceConfiguration configuration) throws IOException { String projectId = configuration.getProjectId(); - TraceServiceStub stub = configuration.getTraceServiceStub(); + TraceServiceSettings.Builder builder = TraceServiceSettings.newBuilder(); - // TODO: Remove stub - tracked in issue #198 - if (stub == null) { - TraceServiceSettings.Builder builder = TraceServiceSettings.newBuilder(); - - // We only use the batchWriteSpans API in this exporter. - builder - .batchWriteSpansSettings() - .setSimpleTimeoutNoRetries( - org.threeten.bp.Duration.ofMillis(configuration.getDeadline().toMillis())); - // For testing, we need to hack around our gRPC config. - if (configuration.getInsecureEndpoint()) { - builder.setCredentialsProvider(NoCredentialsProvider.create()); - builder.setTransportChannelProvider( - FixedTransportChannelProvider.create( - GrpcTransportChannel.create( - ManagedChannelBuilder.forTarget(configuration.getTraceServiceEndpoint()) - .usePlaintext() - .build()))); - } else { - Credentials credentials = - configuration.getCredentials() == null - ? GoogleCredentials.getApplicationDefault() - : configuration.getCredentials(); - builder.setCredentialsProvider( - FixedCredentialsProvider.create(checkNotNull(credentials, "credentials"))); - builder.setEndpoint(configuration.getTraceServiceEndpoint()); - builder.setHeaderProvider(HEADER_PROVIDER); - } - - return new InternalTraceExporter( - projectId, - new CloudTraceClientImpl(TraceServiceClient.create(builder.build())), - configuration.getAttributeMapping(), - configuration.getFixedAttributes()); + // We only use the batchWriteSpans API in this exporter. + builder + .batchWriteSpansSettings() + .setSimpleTimeoutNoRetries( + org.threeten.bp.Duration.ofMillis(configuration.getDeadline().toMillis())); + // For testing, we need to hack around our gRPC config. + if (configuration.getInsecureEndpoint()) { + builder.setCredentialsProvider(NoCredentialsProvider.create()); + builder.setTransportChannelProvider( + FixedTransportChannelProvider.create( + GrpcTransportChannel.create( + ManagedChannelBuilder.forTarget(configuration.getTraceServiceEndpoint()) + .usePlaintext() + .build()))); + } else { + Credentials credentials = + configuration.getCredentials() == null + ? GoogleCredentials.getApplicationDefault() + : configuration.getCredentials(); + builder.setCredentialsProvider( + FixedCredentialsProvider.create(checkNotNull(credentials, "credentials"))); + builder.setEndpoint(configuration.getTraceServiceEndpoint()); + builder.setHeaderProvider(HEADER_PROVIDER); } + return new InternalTraceExporter( projectId, - new CloudTraceClientImpl(TraceServiceClient.create(stub)), + new CloudTraceClientImpl(TraceServiceClient.create(builder.build())), configuration.getAttributeMapping(), configuration.getFixedAttributes()); } diff --git a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/TraceConfiguration.java b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/TraceConfiguration.java index af42e9e5..1d93c656 100644 --- a/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/TraceConfiguration.java +++ b/exporters/trace/src/main/java/com/google/cloud/opentelemetry/trace/TraceConfiguration.java @@ -19,7 +19,6 @@ import com.google.auth.Credentials; import com.google.auto.value.AutoValue; import com.google.cloud.ServiceOptions; -import com.google.cloud.trace.v2.stub.TraceServiceStub; import com.google.cloud.trace.v2.stub.TraceServiceStubSettings; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -89,15 +88,6 @@ public final String getProjectId() { return getProjectIdSupplier().get(); } - /** - * Returns a TraceServiceStub instance used to make RPC calls. - * - * @return the trace service stub. - */ - @Nullable - @Deprecated - public abstract TraceServiceStub getTraceServiceStub(); - /** * Returns the endpoint where to write traces. * @@ -186,16 +176,6 @@ public final Builder setProjectId(String projectId) { return this; } - /** - * Sets the trace service stub used to send gRPC calls. - * - * @deprecated("Use setTraceServiceEndpoint") - * @param traceServiceStub the {@code TraceServiceStub}. - * @return this. - */ - @Deprecated - public abstract Builder setTraceServiceStub(TraceServiceStub traceServiceStub); - /** Sets the endpoint where to write traces. Defaults to tracing.googleapis.com:443. */ public abstract Builder setTraceServiceEndpoint(String endpoint); diff --git a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceConfigurationTest.java b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceConfigurationTest.java index a1962b7f..0ddcfd69 100644 --- a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceConfigurationTest.java +++ b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceConfigurationTest.java @@ -16,6 +16,7 @@ package com.google.cloud.opentelemetry.trace; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; @@ -25,6 +26,7 @@ import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.ServiceOptions; +import com.google.cloud.trace.v2.TraceServiceSettings; import com.google.devtools.cloudtrace.v2.AttributeValue; import java.time.Duration; import java.util.Collections; @@ -53,7 +55,9 @@ public void defaultConfiguration() { assertNull(configuration.getCredentials()); assertNotNull(configuration.getProjectId()); - assertNull(configuration.getTraceServiceStub()); + assertFalse(configuration.getInsecureEndpoint()); + assertEquals( + configuration.getTraceServiceEndpoint(), TraceServiceSettings.getDefaultEndpoint()); assertTrue(configuration.getFixedAttributes().isEmpty()); assertEquals(TraceConfiguration.DEFAULT_DEADLINE, configuration.getDeadline()); } diff --git a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java index 4ff755e2..13f95c9f 100644 --- a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java +++ b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java @@ -18,11 +18,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.ServiceOptions; import com.google.cloud.trace.v2.TraceServiceClient; import com.google.cloud.trace.v2.TraceServiceSettings; -import com.google.cloud.trace.v2.stub.TraceServiceStub; import com.google.devtools.cloudtrace.v2.ProjectName; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -33,6 +33,7 @@ import io.opentelemetry.sdk.trace.export.SpanExporter; import java.io.IOException; import java.util.Collections; +import java.util.Date; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,26 +46,42 @@ public class TraceExporterTest { private static final String PROJECT_ID = "test-id"; + private static final String FAKE_ENDPOINT = "fake.endpoint.com:443"; + private static final GoogleCredentials FAKE_CREDENTIAL = + GoogleCredentials.newBuilder().setAccessToken(new AccessToken("fake", new Date(100))).build(); + @Mock private TraceServiceClient mockedTraceServiceClient; - @Mock private TraceServiceStub mockedTraceServiceStub; @After public void tearDown() { GlobalOpenTelemetry.resetForTest(); } + @Test + public void testCreateWithConfigurationSucceeds() { + TraceConfiguration configuration = + TraceConfiguration.builder() + .setCredentials(FAKE_CREDENTIAL) + .setProjectId(PROJECT_ID) + .setTraceServiceEndpoint(FAKE_ENDPOINT) + .build(); + SpanExporter exporter = TraceExporter.createWithConfiguration(configuration); + assertNotNull(exporter); + } + @SuppressWarnings("ResultOfMethodCallIgnored") @Test public void verifyExporterWorksWithConfiguration() { try (MockedStatic mockedTraceServiceClient = Mockito.mockStatic(TraceServiceClient.class)) { mockedTraceServiceClient - .when(() -> TraceServiceClient.create(Mockito.eq(mockedTraceServiceStub))) + .when(() -> TraceServiceClient.create(Mockito.any(TraceServiceSettings.class))) .thenReturn(this.mockedTraceServiceClient); TraceConfiguration configuration = TraceConfiguration.builder() - .setTraceServiceStub(mockedTraceServiceStub) + .setCredentials(FAKE_CREDENTIAL) + .setTraceServiceEndpoint(FAKE_ENDPOINT) .setProjectId(PROJECT_ID) .build(); SpanExporter exporter = TraceExporter.createWithConfiguration(configuration); @@ -73,7 +90,8 @@ public void verifyExporterWorksWithConfiguration() { simulateExport(exporter); mockedTraceServiceClient.verify( - () -> TraceServiceClient.create(Mockito.eq(mockedTraceServiceStub))); + Mockito.times(1), + () -> TraceServiceClient.create(Mockito.any(TraceServiceSettings.class))); Mockito.verify(this.mockedTraceServiceClient) .batchWriteSpans((ProjectName) Mockito.any(), Mockito.anyList()); } From fac0efb082118bbb433084699188b431e4aceef6 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Wed, 13 Mar 2024 23:31:30 +0000 Subject: [PATCH 3/3] Add unit test to verify exported span attributes --- .../cloud/opentelemetry/trace/FakeData.java | 49 +++++++++++++++++ .../trace/TraceExporterTest.java | 55 ++++++++++++++++++- 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/FakeData.java diff --git a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/FakeData.java b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/FakeData.java new file mode 100644 index 00000000..fabce242 --- /dev/null +++ b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/FakeData.java @@ -0,0 +1,49 @@ +/* + * 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 + * + * http://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.cloud.opentelemetry.trace; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.sdk.testing.trace.TestSpanData; +import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.sdk.trace.data.StatusData; +import java.util.concurrent.TimeUnit; + +public class FakeData { + private static final long START_EPOCH_NANOS = TimeUnit.SECONDS.toNanos(3000) + 200; + private static final long END_EPOCH_NANOS = TimeUnit.SECONDS.toNanos(3001) + 255; + + static final String aTraceId = "00000000000000000000000000000001"; + static final String aSpanId = "0000000000000002"; + static final SpanContext aSpanContext = + SpanContext.create(aTraceId, aSpanId, TraceFlags.getDefault(), TraceState.getDefault()); + + static final SpanData aSpanData = + TestSpanData.builder() + .setHasEnded(true) + .setSpanContext(aSpanContext) + .setName("FakeData.Span") + .setStatus(StatusData.ok()) + .setStartEpochNanos(START_EPOCH_NANOS) + .setEndEpochNanos(END_EPOCH_NANOS) + .setKind(SpanKind.SERVER) + .setAttributes(Attributes.of(AttributeKey.stringKey("desc"), "fake_span")) + .build(); +} diff --git a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java index 13f95c9f..675bf94b 100644 --- a/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java +++ b/exporters/trace/src/test/java/com/google/cloud/opentelemetry/trace/TraceExporterTest.java @@ -15,15 +15,21 @@ */ package com.google.cloud.opentelemetry.trace; +import static com.google.cloud.opentelemetry.trace.FakeData.aSpanData; +import static com.google.cloud.opentelemetry.trace.TraceConfiguration.DEFAULT_ATTRIBUTE_MAPPING; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.ServiceOptions; import com.google.cloud.trace.v2.TraceServiceClient; import com.google.cloud.trace.v2.TraceServiceSettings; +import com.google.devtools.cloudtrace.v2.AttributeValue; import com.google.devtools.cloudtrace.v2.ProjectName; +import com.google.devtools.cloudtrace.v2.Span; +import com.google.devtools.cloudtrace.v2.TruncatableString; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.common.CompletableResultCode; @@ -34,13 +40,18 @@ import java.io.IOException; import java.util.Collections; import java.util.Date; +import java.util.List; +import java.util.Map; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.testcontainers.shaded.com.google.common.collect.ImmutableList; @RunWith(MockitoJUnitRunner.class) public class TraceExporterTest { @@ -51,6 +62,9 @@ public class TraceExporterTest { GoogleCredentials.newBuilder().setAccessToken(new AccessToken("fake", new Date(100))).build(); @Mock private TraceServiceClient mockedTraceServiceClient; + @Mock private CloudTraceClient mockCloudTraceClient; + + @Captor private ArgumentCaptor> spanListCaptor; @After public void tearDown() { @@ -69,7 +83,46 @@ public void testCreateWithConfigurationSucceeds() { assertNotNull(exporter); } - @SuppressWarnings("ResultOfMethodCallIgnored") + @Test + public void testExportSucceeds() { + Map fixedSpanAttributes = + Collections.singletonMap( + "test_span", AttributeValue.newBuilder().setBoolValue(true).build()); + SpanExporter exporter = + InternalTraceExporter.createWithClient( + PROJECT_ID, mockCloudTraceClient, DEFAULT_ATTRIBUTE_MAPPING, fixedSpanAttributes); + + // export fake span data + CompletableResultCode result = exporter.export(ImmutableList.of(aSpanData)); + Mockito.verify(mockCloudTraceClient, Mockito.times(1)) + .batchWriteSpans(Mockito.eq(ProjectName.of(PROJECT_ID)), spanListCaptor.capture()); + + assertTrue(result.isSuccess()); + // check if the spans being exported have the expected span attributes + spanListCaptor + .getValue() + .forEach( + span -> { + assertEquals(aSpanData.getSpanId(), span.getSpanId()); + // Check if agent label key is added as a span attribute + assertTrue( + span.getAttributes() + .getAttributeMapMap() + .get("g.co/agent") + .getStringValue() + .toString() + .contains("google-cloud-trace-exporter")); + assertEquals( + span.getAttributes().getAttributeMapMap().get("test_span"), + AttributeValue.newBuilder().setBoolValue(true).build()); + assertEquals( + span.getAttributes().getAttributeMapMap().get("desc"), + AttributeValue.newBuilder() + .setStringValue(TruncatableString.newBuilder().setValue("fake_span").build()) + .build()); + }); + } + @Test public void verifyExporterWorksWithConfiguration() { try (MockedStatic mockedTraceServiceClient =