Fix azure-core nested HTTP suppression, update libs (#12489)
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
This commit is contained in:
parent
f1c75fab3a
commit
d3f808e008
|
@ -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
|
||||
|
|
|
@ -33,6 +33,12 @@ dependencies {
|
|||
|
||||
val latestDepTest = findProperty("testLatestDeps") as Boolean
|
||||
|
||||
tasks {
|
||||
withType<Test>().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")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -12,6 +12,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod;
|
|||
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 class AzureHttpClientInstrumentation implements TypeInstrumentation {
|
|||
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 class AzureHttpClientInstrumentation implements TypeInstrumentation {
|
|||
@SuppressWarnings("unused")
|
||||
public static class SuppressNestedClientMonoAdvice {
|
||||
@Advice.OnMethodExit(suppress = Throwable.class)
|
||||
public static void asyncSendExit(@Advice.Return(readOnly = false) Mono<HttpResponse> mono) {
|
||||
mono = disallowNestedClientSpanMono(mono);
|
||||
public static void asyncSendExit(
|
||||
@Advice.Argument(1) com.azure.core.util.Context azContext,
|
||||
@Advice.Return(readOnly = false) Mono<HttpResponse> mono) {
|
||||
mono = disallowNestedClientSpanMono(mono, azContext);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -57,8 +62,10 @@ public class AzureHttpClientInstrumentation implements TypeInstrumentation {
|
|||
public static class SuppressNestedClientSyncAdvice {
|
||||
|
||||
@Advice.OnMethodEnter(suppress = Throwable.class)
|
||||
public static void syncSendEnter(@Advice.Local("otelScope") Scope scope) {
|
||||
scope = disallowNestedClientSpanSync();
|
||||
public static void syncSendEnter(
|
||||
@Advice.Argument(1) com.azure.core.util.Context azContext,
|
||||
@Advice.Local("otelScope") Scope scope) {
|
||||
scope = disallowNestedClientSpanSync(azContext);
|
||||
}
|
||||
|
||||
@Advice.OnMethodExit(suppress = Throwable.class)
|
||||
|
|
|
@ -16,20 +16,24 @@ import reactor.core.publisher.Mono;
|
|||
|
||||
public class SuppressNestedClientHelper {
|
||||
|
||||
public static Scope disallowNestedClientSpanSync() {
|
||||
public static Scope disallowNestedClientSpanSync(com.azure.core.util.Context azContext) {
|
||||
Context parentContext = currentContext();
|
||||
if (doesNotHaveClientSpan(parentContext)) {
|
||||
boolean hasAzureClientSpan = azContext.getData("client-method-call-flag").isPresent();
|
||||
if (doesNotHaveClientSpan(parentContext) && hasAzureClientSpan) {
|
||||
return disallowNestedClientSpan(parentContext).makeCurrent();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
public static <T> Mono<T> disallowNestedClientSpanMono(Mono<T> delegate) {
|
||||
public static <T> Mono<T> disallowNestedClientSpanMono(
|
||||
Mono<T> delegate, com.azure.core.util.Context azContext) {
|
||||
return new Mono<T>() {
|
||||
@Override
|
||||
public void subscribe(CoreSubscriber<? super T> 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);
|
||||
}
|
||||
|
|
|
@ -7,14 +7,35 @@ package io.opentelemetry.javaagent.instrumentation.azurecore.v1_36;
|
|||
|
||||
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 @@ class AzureSdkTest {
|
|||
.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<HttpPipelinePolicy> 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<Response<Void>> testMethod();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
Loading…
Reference in New Issue