Apache http client 4.3 low level instrumentation (#10253)

This commit is contained in:
Lauri Tulmin 2024-02-14 09:26:01 +02:00 committed by GitHub
parent 4f0baff3f8
commit 32b3326190
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 24 additions and 87 deletions

View File

@ -9,30 +9,22 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.semconv.http.HttpClientRequestResendCount;
import java.io.IOException;
import javax.annotation.Nullable;
import org.apache.http.HttpException;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.ProtocolException;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpExecutionAware;
import org.apache.http.client.methods.HttpRequestWrapper;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.impl.client.DefaultRedirectStrategy;
import org.apache.http.impl.client.RedirectLocations;
import org.apache.http.impl.execchain.ClientExecChain;
final class TracingProtocolExec implements ClientExecChain {
private static final String REQUEST_CONTEXT_ATTRIBUTE_ID =
private static final String REQUEST_PARENT_CONTEXT_ATTRIBUTE_ID =
TracingProtocolExec.class.getName() + ".context";
private static final String REQUEST_WRAPPER_ATTRIBUTE_ID =
TracingProtocolExec.class.getName() + ".requestWrapper";
private static final String REDIRECT_COUNT_ATTRIBUTE_ID =
TracingProtocolExec.class.getName() + ".redirectCount";
private final Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter;
private final ContextPropagators propagators;
@ -54,14 +46,12 @@ final class TracingProtocolExec implements ClientExecChain {
HttpClientContext httpContext,
HttpExecutionAware httpExecutionAware)
throws IOException, HttpException {
Context context = httpContext.getAttribute(REQUEST_CONTEXT_ATTRIBUTE_ID, Context.class);
if (context != null) {
ApacheHttpClientRequest instrumenterRequest =
httpContext.getAttribute(REQUEST_WRAPPER_ATTRIBUTE_ID, ApacheHttpClientRequest.class);
// Request already had a context so a redirect. Don't create a new span just inject and
// execute.
propagators.getTextMapPropagator().inject(context, request, HttpHeaderSetter.INSTANCE);
return execute(route, request, instrumenterRequest, httpContext, httpExecutionAware, context);
Context parentContext =
httpContext.getAttribute(REQUEST_PARENT_CONTEXT_ATTRIBUTE_ID, Context.class);
if (parentContext == null) {
parentContext = HttpClientRequestResendCount.initialize(Context.current());
httpContext.setAttribute(REQUEST_PARENT_CONTEXT_ATTRIBUTE_ID, parentContext);
}
HttpHost host = null;
@ -81,16 +71,11 @@ final class TracingProtocolExec implements ClientExecChain {
}
ApacheHttpClientRequest instrumenterRequest = new ApacheHttpClientRequest(host, request);
Context parentContext = Context.current();
if (!instrumenter.shouldStart(parentContext, instrumenterRequest)) {
return exec.execute(route, request, httpContext, httpExecutionAware);
}
context = instrumenter.start(parentContext, instrumenterRequest);
httpContext.setAttribute(REQUEST_CONTEXT_ATTRIBUTE_ID, context);
httpContext.setAttribute(REQUEST_WRAPPER_ATTRIBUTE_ID, instrumenterRequest);
httpContext.setAttribute(REDIRECT_COUNT_ATTRIBUTE_ID, 0);
Context context = instrumenter.start(parentContext, instrumenterRequest);
propagators.getTextMapPropagator().inject(context, request, HttpHeaderSetter.INSTANCE);
return execute(route, request, instrumenterRequest, httpContext, httpExecutionAware, context);
@ -113,63 +98,7 @@ final class TracingProtocolExec implements ClientExecChain {
error = e;
throw e;
} finally {
if (!pendingRedirect(context, httpContext, request, instrumenterRequest, response)) {
instrumenter.end(context, instrumenterRequest, response, error);
}
instrumenter.end(context, instrumenterRequest, response, error);
}
}
private boolean pendingRedirect(
Context context,
HttpClientContext httpContext,
HttpRequestWrapper request,
ApacheHttpClientRequest instrumenterRequest,
@Nullable CloseableHttpResponse response) {
if (response == null) {
return false;
}
if (!httpContext.getRequestConfig().isRedirectsEnabled()) {
return false;
}
// TODO(anuraaga): Support redirect strategies other than the default. There is no way to get
// the user defined redirect strategy without some tricks, but it's very rare to override
// the strategy, usually it is either on or off as checked above. We can add support for this
// later if needed.
try {
if (!DefaultRedirectStrategy.INSTANCE.isRedirected(request, response, httpContext)) {
return false;
}
} catch (ProtocolException e) {
// DefaultRedirectStrategy.isRedirected cannot throw this so just return a default.
return false;
}
// Very hacky and a bit slow, but the only way to determine whether the client will fail with
// a circular redirect, which happens before exec decorators run.
RedirectLocations redirectLocations =
(RedirectLocations) httpContext.getAttribute(HttpClientContext.REDIRECT_LOCATIONS);
if (redirectLocations != null) {
RedirectLocations copy = new RedirectLocations();
copy.addAll(redirectLocations);
try {
DefaultRedirectStrategy.INSTANCE.getLocationURI(request, response, httpContext);
} catch (ProtocolException e) {
// We will not be returning to the Exec, finish the span.
instrumenter.end(context, instrumenterRequest, response, new ClientProtocolException(e));
return true;
} finally {
httpContext.setAttribute(HttpClientContext.REDIRECT_LOCATIONS, copy);
}
}
int redirectCount = httpContext.getAttribute(REDIRECT_COUNT_ATTRIBUTE_ID, Integer.class);
if (++redirectCount > httpContext.getRequestConfig().getMaxRedirects()) {
return false;
}
httpContext.setAttribute(REDIRECT_COUNT_ATTRIBUTE_ID, redirectCount);
return true;
}
}

View File

@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URI;
@ -54,8 +55,15 @@ public abstract class AbstractApacheHttpClientTest {
return client;
}
abstract static class ApacheHttpClientTest<T> extends AbstractHttpClientTest<T> {
@Override
protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
optionsBuilder.markAsLowLevelInstrumentation();
}
}
@Nested
class ApacheClientHostRequestTest extends AbstractHttpClientTest<BasicHttpRequest> {
class ApacheClientHostRequestTest extends ApacheHttpClientTest<BasicHttpRequest> {
@Override
public BasicHttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
@ -92,7 +100,7 @@ public abstract class AbstractApacheHttpClientTest {
}
@Nested
class ApacheClientHostRequestContextTest extends AbstractHttpClientTest<BasicHttpRequest> {
class ApacheClientHostRequestContextTest extends ApacheHttpClientTest<BasicHttpRequest> {
@Override
public BasicHttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
@ -133,7 +141,7 @@ public abstract class AbstractApacheHttpClientTest {
}
@Nested
class ApacheClientHostAbsoluteUriRequestTest extends AbstractHttpClientTest<BasicHttpRequest> {
class ApacheClientHostAbsoluteUriRequestTest extends ApacheHttpClientTest<BasicHttpRequest> {
@Override
public BasicHttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
@ -170,7 +178,7 @@ public abstract class AbstractApacheHttpClientTest {
@Nested
class ApacheClientHostAbsoluteUriRequestContextTest
extends AbstractHttpClientTest<BasicHttpRequest> {
extends ApacheHttpClientTest<BasicHttpRequest> {
@Override
public BasicHttpRequest buildRequest(String method, URI uri, Map<String, String> headers) {
@ -210,7 +218,7 @@ public abstract class AbstractApacheHttpClientTest {
}
@Nested
class ApacheClientUriRequestTest extends AbstractHttpClientTest<HttpUriRequest> {
class ApacheClientUriRequestTest extends ApacheHttpClientTest<HttpUriRequest> {
@Override
public HttpUriRequest buildRequest(String method, URI uri, Map<String, String> headers) {
@ -241,7 +249,7 @@ public abstract class AbstractApacheHttpClientTest {
}
@Nested
class ApacheClientUriRequestContextTest extends AbstractHttpClientTest<HttpUriRequest> {
class ApacheClientUriRequestContextTest extends ApacheHttpClientTest<HttpUriRequest> {
@Override
public HttpUriRequest buildRequest(String method, URI uri, Map<String, String> headers) {