Use builder-pattern to build a DefaultOtelResourceMapper

openzipkin-contrib · Sep 26, 2024 · efdbae7 · efdbae7
1 parent 686a72d
commit efdbae7
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 38 deletions.
diff --git a/collector-http/src/main/java/zipkin2/collector/otel/http/DefaultOtelResourceMapper.java b/collector-http/src/main/java/zipkin2/collector/otel/http/DefaultOtelResourceMapper.java
@@ -12,16 +12,36 @@
  * Default implementation of {@link OtelResourceMapper} that simply maps resource attributes except for {@link ServiceAttributes#SERVICE_NAME} to tags with optional prefix.
  */
 public class DefaultOtelResourceMapper implements OtelResourceMapper {
-  private final String resourceAttributePrefix;
 
-  private static final String DEFAULT_PREFIX = "";
+  public static DefaultOtelResourceMapper create() {
+    return newBuilder().build();
+  }
+
+  public static Builder newBuilder() {
+    return new Builder();
+  }
+
+  public static final class Builder {
+    private String resourceAttributePrefix = "";
+
+    /** The prefix for tags mapped from resource attributes. Defaults to the empty string. */
+    public Builder resourceAttributePrefix(String resourceAttributePrefix) {
+      if (resourceAttributePrefix == null) {
+        throw new NullPointerException("resourceAttributePrefix == null");
+      }
+      this.resourceAttributePrefix = resourceAttributePrefix;
+      return this;
+    }
 
-  public DefaultOtelResourceMapper(String resourceAttributePrefix) {
-    this.resourceAttributePrefix = resourceAttributePrefix;
+    public DefaultOtelResourceMapper build() {
+      return new DefaultOtelResourceMapper(this);
+    }
   }
 
-  public DefaultOtelResourceMapper() {
-    this(DEFAULT_PREFIX);
+  private final String resourceAttributePrefix;
+
+  private DefaultOtelResourceMapper(Builder builder) {
+    this.resourceAttributePrefix = builder.resourceAttributePrefix;
   }
 
   public String getResourceAttributePrefix() {

diff --git a/collector-http/src/main/java/zipkin2/collector/otel/http/OpenTelemetryHttpCollector.java b/collector-http/src/main/java/zipkin2/collector/otel/http/OpenTelemetryHttpCollector.java
@@ -92,7 +92,7 @@ public OpenTelemetryHttpCollector build() {
   OpenTelemetryHttpCollector(Builder builder) {
     collector = builder.delegate.build();
     metrics = builder.metrics;
-    otelResourceMapper = builder.otelResourceMapper == null ? new DefaultOtelResourceMapper() : builder.otelResourceMapper;
+    otelResourceMapper = builder.otelResourceMapper == null ? DefaultOtelResourceMapper.create() : builder.otelResourceMapper;
   }
 
   @Override

diff --git a/collector-http/src/main/java/zipkin2/collector/otel/http/SpanTranslator.java b/collector-http/src/main/java/zipkin2/collector/otel/http/SpanTranslator.java
@@ -53,7 +53,7 @@ final class SpanTranslator {
   }
 
   SpanTranslator() {
-    this(new DefaultOtelResourceMapper());
+    this(DefaultOtelResourceMapper.create());
   }
 
 

diff --git a/collector-http/src/test/java/zipkin2/collector/otel/http/SpanTranslatorTest.java b/collector-http/src/test/java/zipkin2/collector/otel/http/SpanTranslatorTest.java
@@ -63,7 +63,7 @@ void translate_invalidTraceId() {
     ExportTraceServiceRequest data = requestBuilderWithSpanCustomizer(span -> span
         .setTraceId(ByteString.fromHex("00000000000000000000000000000000")))
         .build();
-    assertThatThrownBy(() ->   spanTranslator.translate(data))
+    assertThatThrownBy(() -> spanTranslator.translate(data))
         .isInstanceOf(IllegalArgumentException.class);
   }
 
@@ -72,7 +72,7 @@ void translate_invalidSpanId() {
     ExportTraceServiceRequest data = requestBuilderWithSpanCustomizer(span -> span
         .setSpanId(ByteString.fromHex("0000000000000000")))
         .build();
-    assertThatThrownBy(() ->   spanTranslator.translate(data))
+    assertThatThrownBy(() -> spanTranslator.translate(data))
         .isInstanceOf(IllegalArgumentException.class);
   }
 
@@ -462,14 +462,12 @@ void translate_WithResourceAttributes() {
         Function.identity(), span -> span
             .setKind(SpanKind.SPAN_KIND_SERVER)
             .addAttributes(stringAttribute("foo", "bar1"))
-            .addAttributes(stringAttribute("foo", "bar2"))
             .setStatus(Status.newBuilder().setCode(Status.StatusCode.STATUS_CODE_UNSET).build()))
         .build();
 
     Span expectedSpan =
         zipkinSpanBuilder(Span.Kind.SERVER)
-            .putTag("foo", "bar2")
-            .putTag(SpanTranslator.OTEL_DROPPED_ATTRIBUTES_COUNT, "1")
+            .putTag("foo", "bar1")
             .putTag("java.version", "21.0.4")
             .putTag("os.name", "Linux")
             .putTag("os.arch", "amd64")
@@ -482,30 +480,30 @@ void translate_WithResourceAttributes() {
 
   @Test
   void translate_WithResourceAttributes_prefixed() {
-    SpanTranslator spanTranslator = new SpanTranslator(new DefaultOtelResourceMapper("otel.resources."));
+    SpanTranslator spanTranslator = new SpanTranslator(DefaultOtelResourceMapper.newBuilder()
+        .resourceAttributePrefix("otel.resources.")
+        .build());
     ExportTraceServiceRequest data = requestBuilder(resource -> resource
-                    .addAttributes(stringAttribute("java.version", "21.0.4"))
-                    .addAttributes(stringAttribute("os.name", "Linux"))
-                    .addAttributes(stringAttribute("os.arch", "amd64"))
-                    .addAttributes(stringAttribute("hostname", "localhost")),
-            Function.identity(), span -> span
-                    .setKind(SpanKind.SPAN_KIND_SERVER)
-                    .addAttributes(stringAttribute("foo", "bar1"))
-                    .addAttributes(stringAttribute("foo", "bar2"))
-                    .setStatus(Status.newBuilder().setCode(Status.StatusCode.STATUS_CODE_UNSET).build()))
-            .build();
+            .addAttributes(stringAttribute("java.version", "21.0.4"))
+            .addAttributes(stringAttribute("os.name", "Linux"))
+            .addAttributes(stringAttribute("os.arch", "amd64"))
+            .addAttributes(stringAttribute("hostname", "localhost")),
+        Function.identity(), span -> span
+            .setKind(SpanKind.SPAN_KIND_SERVER)
+            .addAttributes(stringAttribute("foo", "bar1"))
+            .setStatus(Status.newBuilder().setCode(Status.StatusCode.STATUS_CODE_UNSET).build()))
+        .build();
 
     Span expectedSpan =
-            zipkinSpanBuilder(Span.Kind.SERVER)
-                    .putTag("foo", "bar2")
-                    .putTag(SpanTranslator.OTEL_DROPPED_ATTRIBUTES_COUNT, "1")
-                    .putTag("otel.resources.java.version", "21.0.4")
-                    .putTag("otel.resources.os.name", "Linux")
-                    .putTag("otel.resources.os.arch", "amd64")
-                    .putTag("otel.resources.hostname", "localhost")
-                    .build();
+        zipkinSpanBuilder(Span.Kind.SERVER)
+            .putTag("foo", "bar1")
+            .putTag("otel.resources.java.version", "21.0.4")
+            .putTag("otel.resources.os.name", "Linux")
+            .putTag("otel.resources.os.arch", "amd64")
+            .putTag("otel.resources.hostname", "localhost")
+            .build();
 
     assertThat(spanTranslator.translate(data))
-            .containsExactly(expectedSpan);
+        .containsExactly(expectedSpan);
   }
 }
diff --git a/module/src/main/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorModule.java b/module/src/main/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorModule.java
@@ -42,6 +42,10 @@ ArmeriaServerConfigurator otelHttpCollectorConfigurator(
   @ConditionalOnMissingBean(OtelResourceMapper.class)
   @Bean
   OtelResourceMapper otelResourceMapper(ZipkinOpenTelemetryHttpCollectorProperties properties) {
-    return new DefaultOtelResourceMapper(properties.resourceAttributePrefix());
+    DefaultOtelResourceMapper.Builder builder = DefaultOtelResourceMapper.newBuilder();
+    if (properties.getResourceAttributePrefix() != null) {
+      builder.resourceAttributePrefix(properties.getResourceAttributePrefix());
+    }
+    return builder.build();
   }
 }
diff --git a/module/src/main/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorProperties.java b/module/src/main/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorProperties.java
@@ -10,7 +10,7 @@
 public class ZipkinOpenTelemetryHttpCollectorProperties {
   private String resourceAttributePrefix;
 
-  public String resourceAttributePrefix() {
+  public String getResourceAttributePrefix() {
     return resourceAttributePrefix;
   }
 

diff --git a/module/src/test/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorModuleTest.java b/module/src/test/java/zipkin/module/otel/ZipkinOpenTelemetryHttpCollectorModuleTest.java
@@ -29,9 +29,15 @@ class ZipkinOpenTelemetryHttpCollectorModuleTest {
   void httpCollector_enabledByDefault() {
     contextRunner.withUserConfiguration(ZipkinOpenTelemetryHttpCollectorModule.class)
         .withUserConfiguration(InMemoryConfiguration.class)
-        .run(context -> assertThat(context).hasSingleBean(OpenTelemetryHttpCollector.class)
-            .hasSingleBean(DefaultOtelResourceMapper.class)
-            .hasSingleBean(ArmeriaServerConfigurator.class));
+        .run(context -> {
+          assertThat(context).hasSingleBean(OpenTelemetryHttpCollector.class)
+              .hasSingleBean(DefaultOtelResourceMapper.class)
+              .hasSingleBean(ArmeriaServerConfigurator.class);
+          OpenTelemetryHttpCollector collector = context.getBean(OpenTelemetryHttpCollector.class);
+          OtelResourceMapper otelResourceMapper = collector.getOtelResourceMapper();
+          assertThat(otelResourceMapper).isInstanceOf(DefaultOtelResourceMapper.class);
+          assertThat(((DefaultOtelResourceMapper) otelResourceMapper).getResourceAttributePrefix()).isEqualTo("");
+        });
   }
 
   @Test