diff --git a/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientInstrumentationModule.java b/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientInstrumentationModule.java index c08468509c..762c684347 100644 --- a/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientInstrumentationModule.java +++ b/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientInstrumentationModule.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.googlehttpclient; import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; -import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.spanFromContext; import static io.opentelemetry.javaagent.instrumentation.googlehttpclient.GoogleHttpClientTracer.tracer; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -19,10 +18,8 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; import com.google.auto.service.AutoService; -import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.instrumentation.api.ContextStore; import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext; import io.opentelemetry.javaagent.tooling.InstrumentationModule; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; @@ -85,21 +82,20 @@ public class GoogleHttpClientInstrumentationModule extends InstrumentationModule @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - ContextStore contextStore = - InstrumentationContext.get(HttpRequest.class, Context.class); - context = contextStore.get(request); - - if (context == null) { - Context parentContext = currentContext(); - if (tracer().shouldStartSpan(parentContext)) { - context = tracer().startSpan(parentContext, request, request.getHeaders()); - contextStore.put(request, context); - scope = context.makeCurrent(); - } - } else { + context = InstrumentationContext.get(HttpRequest.class, Context.class).get(request); + if (context != null) { // span was created by GoogleHttpClientAsyncAdvice instrumentation below + // (executeAsync ends up calling execute from a separate thread) + // so make it current and end it in method exit scope = context.makeCurrent(); + return; } + Context parentContext = currentContext(); + if (!tracer().shouldStartSpan(parentContext)) { + return; + } + context = tracer().startSpan(parentContext, request); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @@ -113,16 +109,7 @@ public class GoogleHttpClientInstrumentationModule extends InstrumentationModule } scope.close(); - if (throwable == null) { - tracer().end(context, response); - } else { - tracer().endExceptionally(context, response, throwable); - } - // If HttpRequest.setThrowExceptionOnExecuteError is set to false, there are no exceptions - // for a failed request. Thus, check the response code - if (response != null && !response.isSuccessStatusCode()) { - spanFromContext(context).setStatus(StatusCode.ERROR); - } + tracer().endMaybeExceptionally(context, response, throwable); } } @@ -138,15 +125,10 @@ public class GoogleHttpClientInstrumentationModule extends InstrumentationModule return; } - context = tracer().startSpan(parentContext, request, request.getHeaders()); - - // propagating the context manually here so this instrumentation will work with and without - // the executors instrumentation - ContextStore contextStore = - InstrumentationContext.get(HttpRequest.class, Context.class); - contextStore.put(request, context); - + context = tracer().startSpan(parentContext, request); scope = context.makeCurrent(); + + InstrumentationContext.get(HttpRequest.class, Context.class).put(request, context); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientTracer.java b/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientTracer.java index f5bb9030bd..21d21ba1cd 100644 --- a/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientTracer.java +++ b/instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/GoogleHttpClientTracer.java @@ -5,10 +5,13 @@ package io.opentelemetry.javaagent.instrumentation.googlehttpclient; +import static io.opentelemetry.javaagent.instrumentation.googlehttpclient.HeadersInjectAdapter.SETTER; + import com.google.api.client.http.HttpHeaders; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; -import io.opentelemetry.context.propagation.TextMapPropagator.Setter; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer; import java.net.URI; import java.net.URISyntaxException; @@ -21,6 +24,10 @@ public class GoogleHttpClientTracer return TRACER; } + public Context startSpan(Context parentContext, HttpRequest request) { + return startSpan(parentContext, request, request.getHeaders()); + } + @Override protected String getInstrumentationName() { return "io.opentelemetry.javaagent.google-http-client"; @@ -56,8 +63,8 @@ public class GoogleHttpClientTracer } @Override - protected Setter getSetter() { - return HeadersInjectAdapter.SETTER; + protected TextMapPropagator.Setter getSetter() { + return SETTER; } private static String header(HttpHeaders headers, String name) {