Add library instrumentation for Apache HTTPClient 4.3 (#3623)

* Add apache httpclient 4.3 library instrumentation.

* Fixup

* Mostly done

* Finish

* Finish

* Update instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java

Co-authored-by: Lauri Tulmin <tulmin@gmail.com>

* Cleanup

Co-authored-by: Lauri Tulmin <tulmin@gmail.com>

instrumentation/apache-httpclient/apache-httpclient-4.3/library/build.gradle.kts

@@ -0,0 +1,14 @@
+plugins {
+  id("otel.library-instrumentation")
+  id("otel.nullaway-conventions")
+}
+
+dependencies {
+  library("org.apache.httpcomponents:httpclient:4.3")
+
+  implementation("org.slf4j:slf4j-api")
+
+  testImplementation(project(":instrumentation:apache-httpclient:apache-httpclient-4.3:testing"))
+
+  latestDepTestLibrary("org.apache.httpcomponents:httpclient:4.+")
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientHttpAttributesExtractor.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
+import org.apache.http.HttpResponse;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+final class ApacheHttpClientHttpAttributesExtractor
+    extends HttpAttributesExtractor<ApacheHttpClientRequest, HttpResponse> {
+
+  @Override
+  protected String method(ApacheHttpClientRequest request) {
+    return request.getMethod();
+  }
+
+  @Override
+  @Nullable
+  protected String url(ApacheHttpClientRequest request) {
+    return request.getUrl();
+  }
+
+  @Override
+  @Nullable
+  protected String target(ApacheHttpClientRequest request) {
+    return request.getTarget();
+  }
+
+  @Override
+  @Nullable
+  protected String host(ApacheHttpClientRequest request) {
+    return request.getHeader("Host");
+  }
+
+  @Override
+  @Nullable
+  protected String scheme(ApacheHttpClientRequest request) {
+    return request.getScheme();
+  }
+
+  @Override
+  @Nullable
+  protected String userAgent(ApacheHttpClientRequest request) {
+    return request.getHeader("User-Agent");
+  }
+
+  @Override
+  @Nullable
+  protected Long requestContentLength(
+      ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return null;
+  }
+
+  @Override
+  @Nullable
+  protected Long requestContentLengthUncompressed(
+      ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return null;
+  }
+
+  @Override
+  protected Integer statusCode(ApacheHttpClientRequest request, HttpResponse response) {
+    return response.getStatusLine().getStatusCode();
+  }
+
+  @Override
+  @Nullable
+  protected String flavor(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return request.getFlavor();
+  }
+
+  @Override
+  @Nullable
+  protected Long responseContentLength(ApacheHttpClientRequest request, HttpResponse response) {
+    return null;
+  }
+
+  @Override
+  @Nullable
+  protected Long responseContentLengthUncompressed(
+      ApacheHttpClientRequest request, HttpResponse response) {
+    return null;
+  }
+
+  @Override
+  @Nullable
+  protected String serverName(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return null;
+  }
+
+  @Override
+  @Nullable
+  protected String route(ApacheHttpClientRequest request) {
+    return null;
+  }
+
+  @Override
+  @Nullable
+  protected String clientIp(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return null;
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientNetAttributesExtractor.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetAttributesExtractor;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import org.apache.http.HttpResponse;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+final class ApacheHttpClientNetAttributesExtractor
+    extends NetAttributesExtractor<ApacheHttpClientRequest, HttpResponse> {
+
+  @Override
+  public String transport(ApacheHttpClientRequest request) {
+    return SemanticAttributes.NetTransportValues.IP_TCP;
+  }
+
+  @Override
+  @Nullable
+  public String peerName(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return request.getPeerName();
+  }
+
+  @Override
+  @Nullable
+  public Integer peerPort(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return request.getPeerPort();
+  }
+
+  @Override
+  @Nullable
+  public String peerIp(ApacheHttpClientRequest request, @Nullable HttpResponse response) {
+    return null;
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java

@@ -0,0 +1,135 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import java.net.URI;
+import java.net.URISyntaxException;
+import org.apache.http.Header;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.ProtocolVersion;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class ApacheHttpClientRequest {
+
+  private static final Logger logger = LoggerFactory.getLogger(ApacheHttpClientRequest.class);
+
+  @Nullable private final URI uri;
+
+  private final HttpRequest delegate;
+
+  ApacheHttpClientRequest(@Nullable HttpHost httpHost, HttpRequest httpRequest) {
+    URI calculatedUri = null;
+    if (httpRequest instanceof HttpUriRequest) {
+      calculatedUri = ((HttpUriRequest) httpRequest).getURI();
+    }
+    if (calculatedUri == null && httpHost != null) {
+      try {
+        calculatedUri = new URI(httpHost.toURI() + httpRequest.getRequestLine().getUri());
+      } catch (URISyntaxException e) {
+        // Ignore
+      }
+    }
+    uri = calculatedUri;
+    delegate = httpRequest;
+  }
+
+  /** Returns the actual {@link HttpRequest} being executed by the client. */
+  public HttpRequest getDelegate() {
+    return delegate;
+  }
+
+  @Nullable
+  String getHeader(String name) {
+    Header header = delegate.getFirstHeader(name);
+    return header != null ? header.getValue() : null;
+  }
+
+  void setHeader(String name, String value) {
+    delegate.setHeader(name, value);
+  }
+
+  String getMethod() {
+    return delegate.getRequestLine().getMethod();
+  }
+
+  @Nullable
+  String getUrl() {
+    return uri != null ? uri.toString() : null;
+  }
+
+  @Nullable
+  String getTarget() {
+    if (uri == null) {
+      return null;
+    }
+    String pathString = uri.getPath();
+    String queryString = uri.getQuery();
+    if (pathString != null && queryString != null) {
+      return pathString + "?" + queryString;
+    } else if (queryString != null) {
+      return "?" + queryString;
+    } else {
+      return pathString;
+    }
+  }
+
+  @Nullable
+  String getScheme() {
+    return uri != null ? uri.getScheme() : null;
+  }
+
+  @Nullable
+  String getFlavor() {
+    ProtocolVersion protocolVersion = delegate.getProtocolVersion();
+    String protocol = protocolVersion.getProtocol();
+    if (!protocol.equals("HTTP")) {
+      return null;
+    }
+    int major = protocolVersion.getMajor();
+    int minor = protocolVersion.getMinor();
+    if (major == 1 && minor == 0) {
+      return SemanticAttributes.HttpFlavorValues.HTTP_1_0;
+    }
+    if (major == 1 && minor == 1) {
+      return SemanticAttributes.HttpFlavorValues.HTTP_1_1;
+    }
+    if (major == 2 && minor == 0) {
+      return SemanticAttributes.HttpFlavorValues.HTTP_2_0;
+    }
+    logger.debug("unexpected http protocol version: {}", protocolVersion);
+    return null;
+  }
+
+  @Nullable
+  String getPeerName() {
+    return uri != null ? uri.getHost() : null;
+  }
+
+  @Nullable
+  Integer getPeerPort() {
+    if (uri == null) {
+      return null;
+    }
+    int port = uri.getPort();
+    if (port != -1) {
+      return port;
+    }
+    switch (uri.getScheme()) {
+      case "http":
+        return 80;
+      case "https":
+        return 443;
+      default:
+        logger.debug("no default port mapping for scheme: {}", uri.getScheme());
+        return null;
+    }
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTracing.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.context.propagation.ContextPropagators;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import org.apache.http.HttpResponse;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+
+/** Entrypoint for tracing Apache HTTP Client. */
+public final class ApacheHttpClientTracing {
+
+  /**
+   * Returns a new {@link ApacheHttpClientTracing} configured with the given {@link OpenTelemetry}.
+   */
+  public static ApacheHttpClientTracing create(OpenTelemetry openTelemetry) {
+    return newBuilder(openTelemetry).build();
+  }
+
+  /**
+   * Returns a new {@link ApacheHttpClientTracingBuilder} configured with the given {@link
+   * OpenTelemetry}.
+   */
+  public static ApacheHttpClientTracingBuilder newBuilder(OpenTelemetry openTelemetry) {
+    return new ApacheHttpClientTracingBuilder(openTelemetry);
+  }
+
+  private final Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter;
+  private final ContextPropagators propagators;
+
+  ApacheHttpClientTracing(
+      Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter,
+      ContextPropagators propagators) {
+    this.instrumenter = instrumenter;
+    this.propagators = propagators;
+  }
+
+  /** Returns a new {@link CloseableHttpClient} with tracing configured. */
+  public CloseableHttpClient newHttpClient() {
+    return newHttpClientBuilder().build();
+  }
+
+  /** Returns a new {@link HttpClientBuilder} to create a client with tracing configured. */
+  public HttpClientBuilder newHttpClientBuilder() {
+    return new TracingHttpClientBuilder(instrumenter, propagators);
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTracingBuilder.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.http.HttpResponse;
+
+/** A builder for {@link ApacheHttpClientTracing}. */
+public final class ApacheHttpClientTracingBuilder {
+
+  private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-4.3";
+
+  private final OpenTelemetry openTelemetry;
+
+  private final List<AttributesExtractor<? super ApacheHttpClientRequest, ? super HttpResponse>>
+      additionalExtractors = new ArrayList<>();
+
+  ApacheHttpClientTracingBuilder(OpenTelemetry openTelemetry) {
+    this.openTelemetry = openTelemetry;
+  }
+
+  /**
+   * Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented
+   * items. The {@link AttributesExtractor} will be executed after all default extractors.
+   */
+  public ApacheHttpClientTracingBuilder addAttributeExtractor(
+      AttributesExtractor<? super ApacheHttpClientRequest, ? super HttpResponse>
+          attributesExtractor) {
+    additionalExtractors.add(attributesExtractor);
+    return this;
+  }
+
+  /**
+   * Returns a new {@link ApacheHttpClientTracing} configured with this {@link
+   * ApacheHttpClientTracingBuilder}.
+   */
+  public ApacheHttpClientTracing build() {
+    HttpAttributesExtractor<ApacheHttpClientRequest, HttpResponse> httpAttributesExtractor =
+        new ApacheHttpClientHttpAttributesExtractor();
+    SpanNameExtractor<? super ApacheHttpClientRequest> spanNameExtractor =
+        HttpSpanNameExtractor.create(httpAttributesExtractor);
+    SpanStatusExtractor<? super ApacheHttpClientRequest, ? super HttpResponse> spanStatusExtractor =
+        HttpSpanStatusExtractor.create(httpAttributesExtractor);
+    ApacheHttpClientNetAttributesExtractor netAttributesExtractor =
+        new ApacheHttpClientNetAttributesExtractor();
+    Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter =
+        Instrumenter.<ApacheHttpClientRequest, HttpResponse>newBuilder(
+                openTelemetry, INSTRUMENTATION_NAME, spanNameExtractor)
+            .setSpanStatusExtractor(spanStatusExtractor)
+            .addAttributesExtractor(httpAttributesExtractor)
+            .addAttributesExtractor(netAttributesExtractor)
+            // We manually inject because we need to inject internal requests for redirects.
+            .newInstrumenter(SpanKindExtractor.alwaysClient());
+
+    return new ApacheHttpClientTracing(instrumenter, openTelemetry.getPropagators());
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/HttpHeaderSetter.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.context.propagation.TextMapSetter;
+import org.apache.http.client.methods.HttpRequestWrapper;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+enum HttpHeaderSetter implements TextMapSetter<HttpRequestWrapper> {
+  INSTANCE;
+
+  @Override
+  public void set(@Nullable HttpRequestWrapper carrier, String key, String value) {
+    if (carrier != null) {
+      carrier.setHeader(key, value);
+    }
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/TracingHttpClientBuilder.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.context.propagation.ContextPropagators;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import org.apache.http.HttpResponse;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.execchain.ClientExecChain;
+
+final class TracingHttpClientBuilder extends HttpClientBuilder {
+
+  private final Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter;
+  private final ContextPropagators propagators;
+
+  TracingHttpClientBuilder(
+      Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter,
+      ContextPropagators propagators) {
+    this.instrumenter = instrumenter;
+    this.propagators = propagators;
+  }
+
+  @Override
+  protected ClientExecChain decorateProtocolExec(ClientExecChain protocolExec) {
+    return new TracingProtocolExec(instrumenter, propagators, protocolExec);
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/TracingProtocolExec.java

@@ -0,0 +1,175 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
+import io.opentelemetry.context.propagation.ContextPropagators;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import java.io.IOException;
+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;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+final class TracingProtocolExec implements ClientExecChain {
+
+  private static final String REQUEST_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;
+  private final ClientExecChain exec;
+
+  TracingProtocolExec(
+      Instrumenter<ApacheHttpClientRequest, HttpResponse> instrumenter,
+      ContextPropagators propagators,
+      ClientExecChain exec) {
+    this.instrumenter = instrumenter;
+    this.propagators = propagators;
+    this.exec = exec;
+  }
+
+  @Override
+  public CloseableHttpResponse execute(
+      HttpRoute route,
+      HttpRequestWrapper request,
+      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);
+    }
+
+    HttpHost host = null;
+    if (route.getTargetHost() != null) {
+      host = route.getTargetHost();
+    } else if (httpContext.getTargetHost() != null) {
+      host = httpContext.getTargetHost();
+    }
+    if (host != null) {
+      if ((host.getSchemeName().equals("https") && host.getPort() == 443)
+          || (host.getSchemeName().equals("http") && host.getPort() == 80)) {
+        // port seems to be added to the host by route planning for standard ports even if not
+        // specified in the URL. There doesn't seem to be a way to differentiate between explicit
+        // and implicit port, but ignore in both cases to match the more common case.
+        host = new HttpHost(host.getHostName(), -1, host.getSchemeName());
+      }
+    }
+    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);
+
+    propagators.getTextMapPropagator().inject(context, request, HttpHeaderSetter.INSTANCE);
+
+    return execute(route, request, instrumenterRequest, httpContext, httpExecutionAware, context);
+  }
+
+  private CloseableHttpResponse execute(
+      HttpRoute route,
+      HttpRequestWrapper request,
+      ApacheHttpClientRequest instrumenterRequest,
+      HttpClientContext httpContext,
+      HttpExecutionAware httpExecutionAware,
+      Context context)
+      throws IOException, HttpException {
+    CloseableHttpResponse response = null;
+    Throwable error = null;
+    try (Scope ignored = context.makeCurrent()) {
+      response = exec.execute(route, request, httpContext, httpExecutionAware);
+      return response;
+    } catch (Throwable e) {
+      error = e;
+      throw e;
+    } finally {
+      if (!pendingRedirect(context, httpContext, request, instrumenterRequest, response)) {
+        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;
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestContextTest.groovy

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import org.apache.http.client.config.RequestConfig
+import org.apache.http.impl.client.CloseableHttpClient
+
+class ApacheClientHostRequestContextTest extends AbstractApacheClientHostRequestContextTest implements LibraryTestTrait {
+  @Override
+  protected CloseableHttpClient createClient() {
+    def builder = ApacheHttpClientTracing.create(openTelemetry).newHttpClientBuilder()
+    builder.defaultRequestConfig = RequestConfig.custom()
+      .setMaxRedirects(maxRedirects())
+      .setConnectTimeout(CONNECT_TIMEOUT_MS)
+      .build()
+    return builder.build()
+  }
+
+  // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
+  @Override
+  boolean testWithClientParent() {
+    false
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientHostRequestTest.groovy

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import org.apache.http.client.config.RequestConfig
+import org.apache.http.impl.client.CloseableHttpClient
+
+class ApacheClientHostRequestTest extends AbstractApacheClientHostRequestTest implements LibraryTestTrait {
+  @Override
+  protected CloseableHttpClient createClient() {
+    def builder = ApacheHttpClientTracing.create(openTelemetry).newHttpClientBuilder()
+    builder.defaultRequestConfig = RequestConfig.custom()
+      .setMaxRedirects(maxRedirects())
+      .setConnectTimeout(CONNECT_TIMEOUT_MS)
+      .build()
+    return builder.build()
+  }
+
+  // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
+  @Override
+  boolean testWithClientParent() {
+    false
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestContextTest.groovy

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import org.apache.http.client.config.RequestConfig
+import org.apache.http.impl.client.CloseableHttpClient
+
+class ApacheClientUriRequestContextTest extends AbstractApacheClientUriRequestContextTest implements LibraryTestTrait {
+  @Override
+  protected CloseableHttpClient createClient() {
+    def builder = ApacheHttpClientTracing.create(openTelemetry).newHttpClientBuilder()
+    builder.defaultRequestConfig = RequestConfig.custom()
+      .setMaxRedirects(maxRedirects())
+      .setConnectTimeout(CONNECT_TIMEOUT_MS)
+      .build()
+    return builder.build()
+  }
+
+  // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
+  @Override
+  boolean testWithClientParent() {
+    false
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/library/src/test/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheClientUriRequestTest.groovy

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import org.apache.http.client.config.RequestConfig
+import org.apache.http.impl.client.CloseableHttpClient
+
+class ApacheClientUriRequestTest extends AbstractApacheClientUriRequestTest implements LibraryTestTrait {
+  @Override
+  protected CloseableHttpClient createClient() {
+    def builder = ApacheHttpClientTracing.create(openTelemetry).newHttpClientBuilder()
+    builder.defaultRequestConfig = RequestConfig.custom()
+      .setMaxRedirects(maxRedirects())
+      .setConnectTimeout(CONNECT_TIMEOUT_MS)
+      .build()
+    return builder.build()
+  }
+
+  // library instrumentation doesn't have a good way of suppressing nested CLIENT spans yet
+  @Override
+  boolean testWithClientParent() {
+    false
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/testing/build.gradle.kts

@@ -0,0 +1,13 @@
+plugins {
+  id("otel.java-conventions")
+}
+
+dependencies {
+  api(project(":testing-common"))
+
+  api("org.apache.httpcomponents:httpclient:4.3")
+
+  implementation("org.codehaus.groovy:groovy-all")
+  implementation("io.opentelemetry:opentelemetry-api")
+  implementation("org.spockframework:spock-core")
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/testing/src/main/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTest.groovy

@@ -0,0 +1,174 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import io.opentelemetry.api.common.AttributeKey
+import io.opentelemetry.instrumentation.test.base.HttpClientTest
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
+import java.util.function.Consumer
+import org.apache.http.HttpHost
+import org.apache.http.HttpRequest
+import org.apache.http.HttpResponse
+import org.apache.http.impl.client.CloseableHttpClient
+import org.apache.http.message.BasicHeader
+import org.apache.http.message.BasicHttpRequest
+import org.apache.http.protocol.BasicHttpContext
+import spock.lang.Shared
+
+abstract class ApacheHttpClientTest<T extends HttpRequest> extends HttpClientTest<T> {
+
+  abstract protected CloseableHttpClient createClient()
+
+  @Override
+  Integer responseCodeOnRedirectError() {
+    return 302
+  }
+
+  @Shared
+  CloseableHttpClient client = createClient()
+
+  @Override
+  boolean testCausality() {
+    false
+  }
+
+  @Override
+  T buildRequest(String method, URI uri, Map<String, String> headers) {
+    def request = createRequest(method, uri)
+    headers.entrySet().each {
+      request.setHeader(new BasicHeader(it.key, it.value))
+    }
+    return request
+  }
+
+  @Override
+  Set<AttributeKey<?>> httpAttributes(URI uri) {
+    Set<AttributeKey<?>> extra = [
+      SemanticAttributes.HTTP_SCHEME,
+      SemanticAttributes.HTTP_TARGET
+    ]
+    super.httpAttributes(uri) + extra
+  }
+
+  // compilation fails with @Override annotation on this method (groovy quirk?)
+  int sendRequest(T request, String method, URI uri, Map<String, String> headers) {
+    def response = executeRequest(request, uri)
+    response.entity?.content?.close() // Make sure the connection is closed.
+    return response.statusLine.statusCode
+  }
+
+  // compilation fails with @Override annotation on this method (groovy quirk?)
+  void sendRequestWithCallback(T request, String method, URI uri, Map<String, String> headers, RequestResult requestResult) {
+    try {
+      executeRequestWithCallback(request, uri) {
+        it.entity?.content?.close() // Make sure the connection is closed.
+        requestResult.complete(it.statusLine.statusCode)
+      }
+    } catch (Throwable throwable) {
+      requestResult.complete(throwable)
+    }
+  }
+
+  abstract T createRequest(String method, URI uri)
+
+  abstract HttpResponse executeRequest(T request, URI uri)
+
+  abstract void executeRequestWithCallback(T request, URI uri, Consumer<HttpResponse> callback)
+
+  static String fullPathFromURI(URI uri) {
+    StringBuilder builder = new StringBuilder()
+    if (uri.getPath() != null) {
+      builder.append(uri.getPath())
+    }
+
+    if (uri.getQuery() != null) {
+      builder.append('?')
+      builder.append(uri.getQuery())
+    }
+
+    if (uri.getFragment() != null) {
+      builder.append('#')
+      builder.append(uri.getFragment())
+    }
+    return builder.toString()
+  }
+}
+
+abstract class AbstractApacheClientHostRequestTest extends ApacheHttpClientTest<BasicHttpRequest> {
+  @Override
+  BasicHttpRequest createRequest(String method, URI uri) {
+    return new BasicHttpRequest(method, fullPathFromURI(uri))
+  }
+
+  @Override
+  HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
+    return client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request)
+  }
+
+  @Override
+  void executeRequestWithCallback(BasicHttpRequest request, URI uri, Consumer<HttpResponse> callback) {
+    client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request) {
+      callback.accept(it)
+    }
+  }
+}
+
+abstract class AbstractApacheClientHostRequestContextTest extends ApacheHttpClientTest<BasicHttpRequest> {
+  @Override
+  BasicHttpRequest createRequest(String method, URI uri) {
+    return new BasicHttpRequest(method, fullPathFromURI(uri))
+  }
+
+  @Override
+  HttpResponse executeRequest(BasicHttpRequest request, URI uri) {
+    return client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request, new BasicHttpContext())
+  }
+
+  @Override
+  void executeRequestWithCallback(BasicHttpRequest request, URI uri, Consumer<HttpResponse> callback) {
+    client.execute(new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme()), request, {
+      callback.accept(it)
+    }, new BasicHttpContext())
+  }
+}
+
+abstract class AbstractApacheClientUriRequestTest extends ApacheHttpClientTest<HttpUriRequest> {
+  @Override
+  HttpUriRequest createRequest(String method, URI uri) {
+    return new HttpUriRequest(method, uri)
+  }
+
+  @Override
+  HttpResponse executeRequest(HttpUriRequest request, URI uri) {
+    return client.execute(request)
+  }
+
+  @Override
+  void executeRequestWithCallback(HttpUriRequest request, URI uri, Consumer<HttpResponse> callback) {
+    client.execute(request) {
+      callback.accept(it)
+    }
+  }
+}
+
+abstract class AbstractApacheClientUriRequestContextTest extends ApacheHttpClientTest<HttpUriRequest> {
+  @Override
+  HttpUriRequest createRequest(String method, URI uri) {
+    return new HttpUriRequest(method, uri)
+  }
+
+  @Override
+  HttpResponse executeRequest(HttpUriRequest request, URI uri) {
+    return client.execute(request, new BasicHttpContext())
+  }
+
+  @Override
+  void executeRequestWithCallback(HttpUriRequest request, URI uri, Consumer<HttpResponse> callback) {
+    client.execute(request, {
+      callback.accept(it)
+    }, new BasicHttpContext())
+  }
+}

instrumentation/apache-httpclient/apache-httpclient-4.3/testing/src/main/groovy/io/opentelemetry/instrumentation/apachehttpclient/v4_3/HttpUriRequest.groovy

@@ -0,0 +1,23 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.apachehttpclient.v4_3
+
+import org.apache.http.client.methods.HttpRequestBase
+
+class HttpUriRequest extends HttpRequestBase {
+
+  private final String methodName
+
+  HttpUriRequest(final String methodName, final URI uri) {
+    this.methodName = methodName
+    setURI(uri)
+  }
+
+  @Override
+  String getMethod() {
+    return methodName
+  }
+}

settings.gradle.kts

@@ -90,6 +90,8 @@ include(":instrumentation:apache-dubbo-2.7:testing")
 include(":instrumentation:apache-httpasyncclient-4.1:javaagent")
 include(":instrumentation:apache-httpclient:apache-httpclient-2.0:javaagent")
 include(":instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent")
+include(":instrumentation:apache-httpclient:apache-httpclient-4.3:library")
+include(":instrumentation:apache-httpclient:apache-httpclient-4.3:testing")
 include(":instrumentation:apache-httpclient:apache-httpclient-5.0:javaagent")
 include(":instrumentation:armeria-1.3:javaagent")
 include(":instrumentation:armeria-1.3:library")