From d3f808e0088cb6202f824effffaba34c7e8bc81c Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Sat, 26 Oct 2024 10:39:00 -0700 Subject: [PATCH] Fix azure-core nested HTTP suppression, update libs (#12489) Co-authored-by: Trask Stalnaker --- CHANGELOG.md | 4 + .../javaagent/build.gradle.kts | 8 ++ .../v1_36/AzureHttpClientInstrumentation.java | 15 ++- .../v1_36/SuppressNestedClientHelper.java | 12 +- .../azurecore/v1_36/AzureSdkTest.java | 106 ++++++++++++++++++ .../build.gradle.kts | 4 +- 6 files changed, 140 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d34915588364..92e019ebd390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +- Update azure-core-tracing-opentelemetry version and improve HTTP suppression to back off + when Azure SDK tracing was disabled. + ([#12489](https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/12489)) + ## Version 2.9.0 (2024-10-17) ### 📈 Enhancements diff --git a/instrumentation/azure-core/azure-core-1.36/javaagent/build.gradle.kts b/instrumentation/azure-core/azure-core-1.36/javaagent/build.gradle.kts index 3a8fd040b87c..5144db03647f 100644 --- a/instrumentation/azure-core/azure-core-1.36/javaagent/build.gradle.kts +++ b/instrumentation/azure-core/azure-core-1.36/javaagent/build.gradle.kts @@ -33,6 +33,12 @@ dependencies { val latestDepTest = findProperty("testLatestDeps") as Boolean +tasks { + withType().configureEach { + systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean) + } +} + testing { suites { // using a test suite to ensure that classes from library-instrumentation-shaded that were @@ -41,8 +47,10 @@ testing { dependencies { if (latestDepTest) { implementation("com.azure:azure-core:+") + implementation("com.azure:azure-core-test:+") } else { implementation("com.azure:azure-core:1.36.0") + implementation("com.azure:azure-core-test:1.16.2") } } } diff --git a/instrumentation/azure-core/azure-core-1.36/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureHttpClientInstrumentation.java b/instrumentation/azure-core/azure-core-1.36/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureHttpClientInstrumentation.java index 2e1a68f2affe..9cce8f85e7fb 100644 --- a/instrumentation/azure-core/azure-core-1.36/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureHttpClientInstrumentation.java +++ b/instrumentation/azure-core/azure-core-1.36/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureHttpClientInstrumentation.java @@ -12,6 +12,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.azure.core.http.HttpResponse; import io.opentelemetry.context.Scope; @@ -35,12 +36,14 @@ public void transform(TypeTransformer transformer) { isMethod() .and(isPublic()) .and(named("send")) + .and(takesArgument(1, named("com.azure.core.util.Context"))) .and(returns(named("reactor.core.publisher.Mono"))), this.getClass().getName() + "$SuppressNestedClientMonoAdvice"); transformer.applyAdviceToMethod( isMethod() .and(isPublic()) .and(named("sendSync")) + .and(takesArgument(1, named("com.azure.core.util.Context"))) .and(returns(named("com.azure.core.http.HttpResponse"))), this.getClass().getName() + "$SuppressNestedClientSyncAdvice"); } @@ -48,8 +51,10 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class SuppressNestedClientMonoAdvice { @Advice.OnMethodExit(suppress = Throwable.class) - public static void asyncSendExit(@Advice.Return(readOnly = false) Mono mono) { - mono = disallowNestedClientSpanMono(mono); + public static void asyncSendExit( + @Advice.Argument(1) com.azure.core.util.Context azContext, + @Advice.Return(readOnly = false) Mono mono) { + mono = disallowNestedClientSpanMono(mono, azContext); } } @@ -57,8 +62,10 @@ public static void asyncSendExit(@Advice.Return(readOnly = false) Mono Mono disallowNestedClientSpanMono(Mono delegate) { + public static Mono disallowNestedClientSpanMono( + Mono delegate, com.azure.core.util.Context azContext) { return new Mono() { @Override public void subscribe(CoreSubscriber coreSubscriber) { Context parentContext = currentContext(); - if (doesNotHaveClientSpan(parentContext)) { + + boolean hasAzureClientSpan = azContext.getData("client-method-call-flag").isPresent(); + if (doesNotHaveClientSpan(parentContext) && hasAzureClientSpan) { try (Scope ignored = disallowNestedClientSpan(parentContext).makeCurrent()) { delegate.subscribe(coreSubscriber); } diff --git a/instrumentation/azure-core/azure-core-1.36/javaagent/src/testAzure/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureSdkTest.java b/instrumentation/azure-core/azure-core-1.36/javaagent/src/testAzure/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureSdkTest.java index 4661a9cf7ea0..f3be2c2129d0 100644 --- a/instrumentation/azure-core/azure-core-1.36/javaagent/src/testAzure/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureSdkTest.java +++ b/instrumentation/azure-core/azure-core-1.36/javaagent/src/testAzure/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureSdkTest.java @@ -7,14 +7,35 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.azure.core.annotation.ExpectedResponses; +import com.azure.core.annotation.Get; +import com.azure.core.annotation.Host; +import com.azure.core.annotation.ServiceInterface; +import com.azure.core.http.HttpClient; +import com.azure.core.http.HttpPipeline; +import com.azure.core.http.HttpPipelineBuilder; +import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.core.http.policy.HttpPolicyProviders; +import com.azure.core.http.rest.Response; +import com.azure.core.http.rest.RestProxy; +import com.azure.core.test.http.MockHttpResponse; +import com.azure.core.util.ClientOptions; import com.azure.core.util.Context; +import com.azure.core.util.TracingOptions; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.internal.SpanKey; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.sdk.trace.data.StatusData; +import io.opentelemetry.semconv.HttpAttributes; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; class AzureSdkTest { @@ -48,9 +69,94 @@ void testSpan() { .hasAttributesSatisfying(Attributes::isEmpty))); } + @Test + void testPipelineAndSuppression() { + AtomicBoolean hasClientAndHttpKeys = new AtomicBoolean(false); + + HttpClient mockClient = + request -> + Mono.defer( + () -> { + // check if suppression is working + hasClientAndHttpKeys.set(hasClientAndHttpSpans()); + return Mono.just(new MockHttpResponse(request, 200)); + }); + + StepVerifier.create(createService(mockClient, true).testMethod()) + .expectNextCount(1) + .expectComplete() + .verify(); + + assertThat(hasClientAndHttpKeys.get()).isTrue(); + testing.waitAndAssertTracesWithoutScopeVersionVerification( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("myService.testMethod") + .hasKind(SpanKind.INTERNAL) + .hasStatus(StatusData.unset()) + .hasAttributes(Attributes.empty()), + span -> + span.hasKind(SpanKind.CLIENT) + .hasName(Boolean.getBoolean("testLatestDeps") ? "GET" : "HTTP GET") + .hasStatus(StatusData.unset()) + .hasAttribute(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200L))); + } + + @Test + void testDisabledTracingNoSuppression() { + AtomicBoolean hasClientAndHttpKeys = new AtomicBoolean(false); + + HttpClient mockClient = + request -> + Mono.defer( + () -> { + // check no suppression + hasClientAndHttpKeys.set(hasClientAndHttpSpans()); + return Mono.just(new MockHttpResponse(request, 200)); + }); + + StepVerifier.create(createService(mockClient, false).testMethod()) + .expectNextCount(1) + .expectComplete() + .verify(); + + assertThat(hasClientAndHttpKeys.get()).isFalse(); + } + private static com.azure.core.util.tracing.Tracer createAzTracer() { com.azure.core.util.tracing.TracerProvider azProvider = com.azure.core.util.tracing.TracerProvider.getDefaultProvider(); return azProvider.createTracer("test-lib", "test-version", "otel.tests", null); } + + private static TestInterface createService(HttpClient httpClient, boolean tracingEnabled) { + List policies = new ArrayList<>(); + HttpPolicyProviders.addAfterRetryPolicies(policies); + + ClientOptions clientOptions = + new ClientOptions().setTracingOptions(new TracingOptions().setEnabled(tracingEnabled)); + HttpPipeline pipeline = + new HttpPipelineBuilder() + .policies(policies.toArray(new HttpPipelinePolicy[0])) + .httpClient(httpClient) + .clientOptions(clientOptions) + .build(); + + return RestProxy.create(TestInterface.class, pipeline); + } + + private static boolean hasClientAndHttpSpans() { + io.opentelemetry.context.Context ctx = io.opentelemetry.context.Context.current(); + return SpanKey.KIND_CLIENT.fromContextOrNull(ctx) != null + && SpanKey.HTTP_CLIENT.fromContextOrNull(ctx) != null; + } + + @Host("https://azure.com") + @ServiceInterface(name = "myService") + interface TestInterface { + @Get("path") + @ExpectedResponses({200}) + Mono> testMethod(); + } } diff --git a/instrumentation/azure-core/azure-core-1.36/library-instrumentation-shaded/build.gradle.kts b/instrumentation/azure-core/azure-core-1.36/library-instrumentation-shaded/build.gradle.kts index 423f391f7e29..f3ffaf42e612 100644 --- a/instrumentation/azure-core/azure-core-1.36/library-instrumentation-shaded/build.gradle.kts +++ b/instrumentation/azure-core/azure-core-1.36/library-instrumentation-shaded/build.gradle.kts @@ -6,7 +6,9 @@ plugins { group = "io.opentelemetry.javaagent.instrumentation" dependencies { - implementation("com.azure:azure-core-tracing-opentelemetry:1.0.0-beta.42") + // this is the last good version that works with indy build + // update to 1.49 or latest once https://github.com/Azure/azure-sdk-for-java/pull/42586 is released. + implementation("com.azure:azure-core-tracing-opentelemetry:1.0.0-beta.45") } tasks {