remove need for reflection

open-telemetry · Jun 27, 2024 · f1e8291 · f1e8291
1 parent 8afa75d
commit f1e8291
Show file tree

Hide file tree

Showing 17 changed files with 206 additions and 93 deletions.
diff --git a/.../instrumentation/api/incubator/builder/internal/DefaultHttpServerInstrumenterBuilder.java b/.../instrumentation/api/incubator/builder/internal/DefaultHttpServerInstrumenterBuilder.java
@@ -26,7 +26,6 @@
 import io.opentelemetry.instrumentation.api.semconv.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpSpanNameExtractorBuilder;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpSpanStatusExtractor;
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -212,17 +211,15 @@ public OpenTelemetry getOpenTelemetry() {
   }
 
   @CanIgnoreReturnValue
-  public static <REQUEST, RESPONSE>
-      DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE> unwrapAndConfigure(
-          CoreCommonConfig config, Object builder) {
-    DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE> defaultBuilder = unwrapBuilder(builder);
-    set(config::getKnownHttpRequestMethods, defaultBuilder::setKnownMethods);
-    set(config::getServerRequestHeaders, defaultBuilder::setCapturedRequestHeaders);
-    set(config::getServerResponseHeaders, defaultBuilder::setCapturedResponseHeaders);
+  public DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE> configure(
+      CoreCommonConfig config) {
+    set(config::getKnownHttpRequestMethods, this::setKnownMethods);
+    set(config::getServerRequestHeaders, this::setCapturedRequestHeaders);
+    set(config::getServerResponseHeaders, this::setCapturedResponseHeaders);
     set(
         config::shouldEmitExperimentalHttpServerTelemetry,
-        defaultBuilder::setEmitExperimentalHttpServerMetrics);
-    return defaultBuilder;
+        this::setEmitExperimentalHttpServerMetrics);
+    return this;
   }
 
   private static <T> void set(Supplier<T> supplier, Consumer<T> consumer) {
@@ -231,25 +228,4 @@ private static <T> void set(Supplier<T> supplier, Consumer<T> consumer) {
       consumer.accept(t);
     }
   }
-
-  /**
-   * This method is used to access the builder field of the builder object.
-   *
-   * <p>This approach allows us to re-use the existing builder classes from the library modules
-   */
-  @SuppressWarnings("unchecked")
-  private static <REQUEST, RESPONSE>
-      DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE> unwrapBuilder(Object builder) {
-    if (builder instanceof DefaultHttpServerInstrumenterBuilder<?, ?>) {
-      return (DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE>) builder;
-    }
-    Class<?> builderClass = builder.getClass();
-    try {
-      Field field = builderClass.getDeclaredField("serverBuilder");
-      field.setAccessible(true);
-      return (DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE>) field.get(builder);
-    } catch (Exception e) {
-      throw new IllegalStateException("Could not access serverBuilder field in " + builderClass, e);
-    }
-  }
 }
diff --git a/...opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTelemetryBuilder.java b/...opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientTelemetryBuilder.java
@@ -20,11 +20,10 @@
 public final class ApacheHttpClientTelemetryBuilder {
 
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-4.3";
-  private final DefaultHttpClientInstrumenterBuilder<ApacheHttpClientRequest, HttpResponse>
-      clientBuilder;
+  private final DefaultHttpClientInstrumenterBuilder<ApacheHttpClientRequest, HttpResponse> builder;
 
   ApacheHttpClientTelemetryBuilder(OpenTelemetry openTelemetry) {
-    clientBuilder =
+    builder =
         new DefaultHttpClientInstrumenterBuilder<>(
             INSTRUMENTATION_NAME, openTelemetry, ApacheHttpClientHttpAttributesGetter.INSTANCE);
   }
@@ -37,7 +36,7 @@ public final class ApacheHttpClientTelemetryBuilder {
   public ApacheHttpClientTelemetryBuilder addAttributeExtractor(
       AttributesExtractor<? super ApacheHttpClientRequest, ? super HttpResponse>
           attributesExtractor) {
-    clientBuilder.addAttributeExtractor(attributesExtractor);
+    builder.addAttributeExtractor(attributesExtractor);
     return this;
   }
 
@@ -48,7 +47,7 @@ public ApacheHttpClientTelemetryBuilder addAttributeExtractor(
    */
   @CanIgnoreReturnValue
   public ApacheHttpClientTelemetryBuilder setCapturedRequestHeaders(List<String> requestHeaders) {
-    clientBuilder.setCapturedRequestHeaders(requestHeaders);
+    builder.setCapturedRequestHeaders(requestHeaders);
     return this;
   }
 
@@ -59,7 +58,7 @@ public ApacheHttpClientTelemetryBuilder setCapturedRequestHeaders(List<String> r
    */
   @CanIgnoreReturnValue
   public ApacheHttpClientTelemetryBuilder setCapturedResponseHeaders(List<String> responseHeaders) {
-    clientBuilder.setCapturedResponseHeaders(responseHeaders);
+    builder.setCapturedResponseHeaders(responseHeaders);
     return this;
   }
 
@@ -78,7 +77,7 @@ public ApacheHttpClientTelemetryBuilder setCapturedResponseHeaders(List<String>
    */
   @CanIgnoreReturnValue
   public ApacheHttpClientTelemetryBuilder setKnownMethods(Set<String> knownMethods) {
-    clientBuilder.setKnownMethods(knownMethods);
+    builder.setKnownMethods(knownMethods);
     return this;
   }
 
@@ -91,7 +90,7 @@ public ApacheHttpClientTelemetryBuilder setKnownMethods(Set<String> knownMethods
   @CanIgnoreReturnValue
   public ApacheHttpClientTelemetryBuilder setEmitExperimentalHttpClientMetrics(
       boolean emitExperimentalHttpClientMetrics) {
-    clientBuilder.setEmitExperimentalHttpClientMetrics(emitExperimentalHttpClientMetrics);
+    builder.setEmitExperimentalHttpClientMetrics(emitExperimentalHttpClientMetrics);
     return this;
   }
 
@@ -102,7 +101,7 @@ public ApacheHttpClientTelemetryBuilder setSpanNameExtractor(
               SpanNameExtractor<ApacheHttpClientRequest>,
               ? extends SpanNameExtractor<? super ApacheHttpClientRequest>>
           spanNameExtractorTransformer) {
-    clientBuilder.setSpanNameExtractor(spanNameExtractorTransformer);
+    builder.setSpanNameExtractor(spanNameExtractorTransformer);
     return this;
   }
 
@@ -112,6 +111,6 @@ public ApacheHttpClientTelemetryBuilder setSpanNameExtractor(
    */
   public ApacheHttpClientTelemetry build() {
     return new ApacheHttpClientTelemetry(
-        clientBuilder.build(), clientBuilder.getOpenTelemetry().getPropagators());
+        builder.build(), builder.getOpenTelemetry().getPropagators());
   }
 }
diff --git a/...pentelemetry/instrumentation/apachehttpclient/v5_2/ApacheHttpClient5TelemetryBuilder.java b/...pentelemetry/instrumentation/apachehttpclient/v5_2/ApacheHttpClient5TelemetryBuilder.java
@@ -21,10 +21,10 @@ public final class ApacheHttpClient5TelemetryBuilder {
 
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-5.2";
   private final DefaultHttpClientInstrumenterBuilder<ApacheHttpClient5Request, HttpResponse>
-      clientBuilder;
+      builder;
 
   ApacheHttpClient5TelemetryBuilder(OpenTelemetry openTelemetry) {
-    clientBuilder =
+    builder =
         new DefaultHttpClientInstrumenterBuilder<>(
             INSTRUMENTATION_NAME, openTelemetry, ApacheHttpClient5HttpAttributesGetter.INSTANCE);
   }
@@ -37,7 +37,7 @@ public final class ApacheHttpClient5TelemetryBuilder {
   public ApacheHttpClient5TelemetryBuilder addAttributeExtractor(
       AttributesExtractor<? super ApacheHttpClient5Request, ? super HttpResponse>
           attributesExtractor) {
-    clientBuilder.addAttributeExtractor(attributesExtractor);
+    builder.addAttributeExtractor(attributesExtractor);
     return this;
   }
 
@@ -48,7 +48,7 @@ public ApacheHttpClient5TelemetryBuilder addAttributeExtractor(
    */
   @CanIgnoreReturnValue
   public ApacheHttpClient5TelemetryBuilder setCapturedRequestHeaders(List<String> requestHeaders) {
-    clientBuilder.setCapturedRequestHeaders(requestHeaders);
+    builder.setCapturedRequestHeaders(requestHeaders);
     return this;
   }
 
@@ -60,7 +60,7 @@ public ApacheHttpClient5TelemetryBuilder setCapturedRequestHeaders(List<String>
   @CanIgnoreReturnValue
   public ApacheHttpClient5TelemetryBuilder setCapturedResponseHeaders(
       List<String> responseHeaders) {
-    clientBuilder.setCapturedResponseHeaders(responseHeaders);
+    builder.setCapturedResponseHeaders(responseHeaders);
     return this;
   }
 
@@ -79,7 +79,7 @@ public ApacheHttpClient5TelemetryBuilder setCapturedResponseHeaders(
    */
   @CanIgnoreReturnValue
   public ApacheHttpClient5TelemetryBuilder setKnownMethods(Set<String> knownMethods) {
-    clientBuilder.setKnownMethods(knownMethods);
+    builder.setKnownMethods(knownMethods);
     return this;
   }
 
@@ -92,7 +92,7 @@ public ApacheHttpClient5TelemetryBuilder setKnownMethods(Set<String> knownMethod
   @CanIgnoreReturnValue
   public ApacheHttpClient5TelemetryBuilder setEmitExperimentalHttpClientMetrics(
       boolean emitExperimentalHttpClientMetrics) {
-    clientBuilder.setEmitExperimentalHttpClientMetrics(emitExperimentalHttpClientMetrics);
+    builder.setEmitExperimentalHttpClientMetrics(emitExperimentalHttpClientMetrics);
     return this;
   }
 
@@ -103,7 +103,7 @@ public ApacheHttpClient5TelemetryBuilder setSpanNameExtractor(
               SpanNameExtractor<ApacheHttpClient5Request>,
               ? extends SpanNameExtractor<? super ApacheHttpClient5Request>>
           spanNameExtractorTransformer) {
-    clientBuilder.setSpanNameExtractor(spanNameExtractorTransformer);
+    builder.setSpanNameExtractor(spanNameExtractorTransformer);
     return this;
   }
 
@@ -113,6 +113,6 @@ public ApacheHttpClient5TelemetryBuilder setSpanNameExtractor(
    */
   public ApacheHttpClient5Telemetry build() {
     return new ApacheHttpClient5Telemetry(
-        clientBuilder.build(), clientBuilder.getOpenTelemetry().getPropagators());
+        builder.build(), builder.getOpenTelemetry().getPropagators());
   }
 }
diff --git a/...instrumentation/spring/autoconfigure/instrumentation/web/RestClientBeanPostProcessor.java b/...instrumentation/spring/autoconfigure/instrumentation/web/RestClientBeanPostProcessor.java
@@ -56,7 +56,7 @@ private static RestClient addRestClientInterceptorIfNotPresent(
 
   static ClientHttpRequestInterceptor getInterceptor(
       OpenTelemetry openTelemetry, ConfigProperties config) {
-    return InstrumentationConfigUtil.configureBuilder(
+    return InstrumentationConfigUtil.configureClientBuilder(
             config, SpringWebTelemetry.builder(openTelemetry), WebTelemetryUtil.GET_BUILDER)
         .build()
         .newInterceptor();

diff --git a/...instrumentation/spring/autoconfigure/instrumentation/web/RestTemplateInstrumentation.java b/...instrumentation/spring/autoconfigure/instrumentation/web/RestTemplateInstrumentation.java
@@ -24,7 +24,7 @@ static RestTemplate addIfNotPresent(
       RestTemplate restTemplate, OpenTelemetry openTelemetry, ConfigProperties config) {
 
     ClientHttpRequestInterceptor instrumentationInterceptor =
-        InstrumentationConfigUtil.configureBuilder(
+        InstrumentationConfigUtil.configureClientBuilder(
                 config, SpringWebTelemetry.builder(openTelemetry), WebTelemetryUtil.GET_BUILDER)
             .build()
             .newInterceptor();

diff --git a/...trumentation/spring/autoconfigure/instrumentation/webflux/WebClientBeanPostProcessor.java b/...trumentation/spring/autoconfigure/instrumentation/webflux/WebClientBeanPostProcessor.java
@@ -8,6 +8,7 @@
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.spring.webflux.v5_3.SpringWebfluxTelemetry;
+import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.SpringWebfluxUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.beans.factory.config.BeanPostProcessor;
@@ -33,7 +34,10 @@ final class WebClientBeanPostProcessor implements BeanPostProcessor {
   static SpringWebfluxTelemetry getWebfluxTelemetry(
       OpenTelemetry openTelemetry, ConfigProperties config) {
     return InstrumentationConfigUtil.configureClientAndServerBuilder(
-            config, SpringWebfluxTelemetry.builder(openTelemetry))
+            config,
+            SpringWebfluxTelemetry.builder(openTelemetry),
+            SpringWebfluxUtil.GET_CLIENT_BUILDER,
+            SpringWebfluxUtil.GET_SERVER_BUILDER)
         .build();
   }
 

diff --git a/...g/autoconfigure/instrumentation/webmvc/SpringWebMvc5InstrumentationAutoConfiguration.java b/...g/autoconfigure/instrumentation/webmvc/SpringWebMvc5InstrumentationAutoConfiguration.java
@@ -9,6 +9,7 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.spring.webmvc.v5_3.SpringWebMvcTelemetry;
+import io.opentelemetry.instrumentation.spring.webmvc.v5_3.internal.SpringMvcUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import javax.servlet.Filter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
@@ -26,7 +27,7 @@ public class SpringWebMvc5InstrumentationAutoConfiguration {
   @Bean
   Filter otelWebMvcFilter(OpenTelemetry openTelemetry, ConfigProperties config) {
     return InstrumentationConfigUtil.configureServerBuilder(
-            config, SpringWebMvcTelemetry.builder(openTelemetry))
+            config, SpringWebMvcTelemetry.builder(openTelemetry), SpringMvcUtil.GET_BUILDER)
         .build()
         .createServletFilter();
   }

diff --git a/...g/autoconfigure/instrumentation/webmvc/SpringWebMvc6InstrumentationAutoConfiguration.java b/...g/autoconfigure/instrumentation/webmvc/SpringWebMvc6InstrumentationAutoConfiguration.java
@@ -9,6 +9,7 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.spring.webmvc.v6_0.SpringWebMvcTelemetry;
+import io.opentelemetry.instrumentation.spring.webmvc.v6_0.internal.SpringMvcUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import jakarta.servlet.Filter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
@@ -26,7 +27,7 @@ public class SpringWebMvc6InstrumentationAutoConfiguration {
   @Bean
   Filter otelWebMvcFilter(OpenTelemetry openTelemetry, ConfigProperties config) {
     return InstrumentationConfigUtil.configureServerBuilder(
-            config, SpringWebMvcTelemetry.builder(openTelemetry))
+            config, SpringWebMvcTelemetry.builder(openTelemetry), SpringMvcUtil.GET_BUILDER)
         .build()
         .createServletFilter();
   }

diff --git a/...y/instrumentation/spring/autoconfigure/internal/properties/InstrumentationConfigUtil.java b/...y/instrumentation/spring/autoconfigure/internal/properties/InstrumentationConfigUtil.java
@@ -20,30 +20,42 @@ public class InstrumentationConfigUtil {
   private InstrumentationConfigUtil() {}
 
   @CanIgnoreReturnValue
-  public static <T, REQUEST, RESPONSE> T configureClientAndServerBuilder(ConfigProperties config, T builder) {
-    DefaultHttpClientInstrumenterBuilder.unwrapAndConfigure(
-        new CoreCommonConfig(new ConfigPropertiesBridge(config)), builder);
-    DefaultHttpServerInstrumenterBuilder.unwrapAndConfigure(
-        new CoreCommonConfig(new ConfigPropertiesBridge(config)), builder);
+  public static <T, CLIENT_REQUEST, CLIENT_RESPONSE, SERVER_REQUEST, SERVER_RESPONSE>
+      T configureClientAndServerBuilder(
+          ConfigProperties config,
+          T builder,
+          Function<T, DefaultHttpClientInstrumenterBuilder<CLIENT_REQUEST, CLIENT_RESPONSE>>
+              getClientBuilder,
+          Function<T, DefaultHttpServerInstrumenterBuilder<SERVER_REQUEST, SERVER_RESPONSE>>
+              getServerBuilder) {
+    CoreCommonConfig commonConfig = getConfig(config);
+    getClientBuilder.apply(builder).configure(commonConfig);
+    getServerBuilder.apply(builder).configure(commonConfig);
     return builder;
   }
 
   @CanIgnoreReturnValue
-  public static <T> T configureClientBuilder(
+  public static <T, REQUEST, RESPONSE> T configureClientBuilder(
       ConfigProperties config,
       T builder,
       Function<T, DefaultHttpClientInstrumenterBuilder<REQUEST, RESPONSE>> getBuilder) {
-    getBuilder.apply(builder).configure(new CoreCommonConfig(new ConfigPropertiesBridge(config)));
+    getBuilder.apply(builder).configure(getConfig(config));
     return builder;
   }
 
   @CanIgnoreReturnValue
-  public static <T> T configureServerBuilder(ConfigProperties config, T builder) {
-    DefaultHttpServerInstrumenterBuilder.unwrapAndConfigure(
-        new CoreCommonConfig(new ConfigPropertiesBridge(config)), builder);
+  public static <T, REQUEST, RESPONSE> T configureServerBuilder(
+      ConfigProperties config,
+      T builder,
+      Function<T, DefaultHttpServerInstrumenterBuilder<REQUEST, RESPONSE>> getBuilder) {
+    getBuilder.apply(builder).configure(getConfig(config));
     return builder;
   }
 
+  private static CoreCommonConfig getConfig(ConfigProperties config) {
+    return new CoreCommonConfig(new ConfigPropertiesBridge(config));
+  }
+
   public static boolean isStatementSanitizationEnabled(ConfigProperties config, String key) {
     return config.getBoolean(
         key, config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true));