replace raw type for AttributeKey (#55)

inspectIT · Mar 7, 2024 · a799678 · a799678
1 parent fd9de43
commit a799678
Show file tree

Hide file tree

Showing 10 changed files with 56 additions and 78 deletions.
diff --git a/src/main/java/io/opentelemetry/sdk/trace/OcelotSpanUtils.java b/src/main/java/io/opentelemetry/sdk/trace/OcelotSpanUtils.java
@@ -10,7 +10,6 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.ContextKey;
 import io.opentelemetry.proto.common.v1.AnyValue;
-import io.opentelemetry.proto.common.v1.ArrayValue;
 import io.opentelemetry.proto.common.v1.KeyValue;
 import io.opentelemetry.proto.trace.v1.Span;
 import io.opentelemetry.proto.trace.v1.Status;
@@ -232,15 +231,32 @@ public static Attributes toAttributes(List<KeyValue> attributesList, Map<String,
                 // skip invalid data
                 if (attribute == null) continue;
 
-                AttributeKey attributeKey = toAttributeKey(attribute);
-                if (attributeKey != null) {
-                    AnyValue value = attribute.getValue();
-                    switch (attribute.getValue().getValueCase()) {
-                        case STRING_VALUE -> builder.put(attributeKey, value.getStringValue());
-                        case BOOL_VALUE -> builder.put(attributeKey, value.getBoolValue());
-                        case INT_VALUE -> builder.put(attributeKey, value.getIntValue());
-                        case DOUBLE_VALUE -> builder.put(attributeKey, value.getDoubleValue());
-                        case ARRAY_VALUE -> builder.put(attributeKey, mergeArray(value.getArrayValue()));
+                String key = attribute.getKey();
+                AnyValue value = attribute.getValue();
+                AnyValue.ValueCase valueCase = value.getValueCase();
+                switch (valueCase) {
+                    case STRING_VALUE ->  {
+                        AttributeKey<String> attributeKey = AttributeKey.stringKey(key);
+                        builder.put(attributeKey, value.getStringValue());
+                    }
+                    case BOOL_VALUE -> {
+                        AttributeKey<Boolean> attributeKey = AttributeKey.booleanKey(key);
+                        builder.put(attributeKey, value.getBoolValue());
+                    }
+                    case INT_VALUE -> {
+                        AttributeKey<Long> attributeKey = AttributeKey.longKey(key);
+                        builder.put(attributeKey, value.getIntValue());
+                    }
+                    case DOUBLE_VALUE -> {
+                        AttributeKey<Double> attributeKey = AttributeKey.doubleKey(key);
+                        builder.put(attributeKey, value.getDoubleValue());
+                    }
+                    case ARRAY_VALUE -> {
+                        AttributeKey<List<String>> attributeKey = AttributeKey.stringArrayKey(key);
+                        List<String> stringValues = value.getArrayValue().getValuesList().stream()
+                                .map(OcelotSpanUtils::getValueAsString)
+                                .toList();
+                        builder.put(attributeKey, stringValues);
                     }
                 }
             }
@@ -253,23 +269,6 @@ public static Attributes toAttributes(List<KeyValue> attributesList, Map<String,
         return builder.build();
     }
 
-    /**
-     * @return Returns a {@link AttributeKey} which represents the given {@link KeyValue}.
-     */
-    private static AttributeKey<?> toAttributeKey(KeyValue attribute) {
-        String key = attribute.getKey();
-        AnyValue.ValueCase valueCase = attribute.getValue().getValueCase();
-        return switch (valueCase) {
-            case STRING_VALUE -> AttributeKey.stringKey(key);
-            case BOOL_VALUE -> AttributeKey.booleanKey(key);
-            case INT_VALUE -> AttributeKey.longKey(key);
-            case DOUBLE_VALUE -> AttributeKey.doubleKey(key);
-            // Currently, OTel is not able to process arrayValue in attributes
-            case ARRAY_VALUE -> AttributeKey.stringKey(key);
-            default -> null;
-        };
-    }
-
     /**
      * @return Returns a {@link SpanKind} representing the given {@link Span.SpanKind} instance.
      */
@@ -285,23 +284,6 @@ private static SpanKind toSpanKind(Span.SpanKind spanKind) {
         };
     }
 
-    /**
-     * Merges all values of an array. The values will always be converted to strings.
-     * Currently, OTel is not able to process arrayValue objects in Attributes.
-     * See <a href="https://github.com/open-telemetry/opentelemetry-java/issues/6243">issue</a>
-     *
-     * @param arrayValue the array containing any values
-     *
-     * @return the merged string of all values
-     */
-    private static String mergeArray(ArrayValue arrayValue) {
-        List<AnyValue> values = arrayValue.getValuesList();
-        String mergedString = values.stream()
-                .map(OcelotSpanUtils::getValueAsString)
-                .collect(Collectors.joining(", "));
-        return mergedString;
-    }
-
     /**
      * @return the {@link AnyValue} as string.
      */

diff --git a/src/test/java/io/opentelemetry/sdk/trace/OcelotSpanUtilsTest.java b/src/test/java/io/opentelemetry/sdk/trace/OcelotSpanUtilsTest.java
@@ -1,5 +1,6 @@
 package io.opentelemetry.sdk.trace;
 
+import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.StatusCode;
@@ -11,7 +12,6 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -139,7 +139,7 @@ public void verifyValidAttributes() {
         }
 
         @Test
-        public void verifyMergedArrayValue() {
+        public void verifyArrayValue() {
             KeyValue kvArray = KeyValue.newBuilder()
                     .setKey("browser.brands")
                     .setValue(AnyValue.newBuilder().setArrayValue(
@@ -158,8 +158,9 @@ public void verifyMergedArrayValue() {
                     .build();
             List<KeyValue> keyValues = Collections.singletonList(kvArray);
 
+            AttributeKey<List<String>> arrayKey = AttributeKey.stringArrayKey("browser.brands");
             Attributes expected = Attributes.builder()
-                    .put("browser.brands", "Chrome, Firefox, 100")
+                    .put(arrayKey, List.of("Chrome", "Firefox", "100"))
                     .build();
 
             Attributes attributes = OcelotSpanUtils.toAttributes(keyValues);

diff --git a/src/test/java/rocks/inspectit/oce/eum/server/exporters/ExporterIntMockMvcTestBase.java b/src/test/java/rocks/inspectit/oce/eum/server/exporters/ExporterIntMockMvcTestBase.java
@@ -28,8 +28,8 @@ public class ExporterIntMockMvcTestBase {
     @Autowired
     protected MockMvc mockMvc;
 
-    @Value("classpath:ot-trace-array-v0.48.0.json")
-    private Resource resourceSpans;
+    @Value("classpath:ot-trace-prod-v0.48.0.json")
+    private Resource prodSpans;
 
     public static final String SERVICE_NAME = "E2E-test";
 
@@ -39,7 +39,9 @@ public class ExporterIntMockMvcTestBase {
 
     static String SUT_URL = "http://test.com/login";
 
-    // Trace-Id used in the resource spans
+    /**
+     * Trace-Id used in {@link #prodSpans}
+     */
     protected static String RESOURCE_TRACE_ID = "a4a68b53c52438381b6cb304410ff0be";
 
     protected static String FAKE_BEACON_KEY_NAME = "does_not_exist";
@@ -97,16 +99,13 @@ protected void postSpan(String traceId) throws Exception {
     }
 
     /**
-     * Posts a {@code Span} to {@link rocks.inspectit.oce.eum.server.rest.TraceController#spans(String)}.
+     * Posts {@code Span}s to {@link rocks.inspectit.oce.eum.server.rest.TraceController#spans(String)}.
      * The span data will be read from a file.
      * <br>
-     * Currently, OTel is not able to process arrayValue objects in Attributes.
-     * Instead, all values will be merged to one string.
-     * See <a href="https://github.com/open-telemetry/opentelemetry-java/issues/6243">issue</a>
-     *
+     * The span data originates from production and should always pass valid
      */
-    protected void postResourceSpans() throws Exception {
-        try (Reader reader = new InputStreamReader(resourceSpans.getInputStream())) {
+    protected void postProdSpans() throws Exception {
+        try (Reader reader = new InputStreamReader(prodSpans.getInputStream())) {
             String json = CharStreams.toString(reader);
 
             mockMvc.perform(post("/spans").contentType(MediaType.APPLICATION_JSON).content(json))

diff --git a/...test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerGrpcExporterIntTest.java b/...test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerGrpcExporterIntTest.java
@@ -31,8 +31,8 @@ void verifyTraceSentGrpcWithJaeger() throws Exception {
     }
 
     @Test
-    void verifyTraceWithArrayValueSentGrpcWithJaeger() throws Exception {
-        postResourceSpans();
+    void verifyProdTraceSentGrpcWithJaeger() throws Exception {
+        postProdSpans();
         awaitSpansExported(RESOURCE_TRACE_ID);
     }
 }
diff --git a/...test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerHttpExporterIntTest.java b/...test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerHttpExporterIntTest.java
@@ -12,7 +12,7 @@
 
 @DirtiesContext
 @ContextConfiguration(initializers = JaegerHttpExporterIntTest.EnvInitializer.class)
-class JaegerHttpExporterIntTest extends ExporterIntTestBaseWithOtelCollector {
+public class JaegerHttpExporterIntTest extends ExporterIntTestBaseWithOtelCollector {
 
     static class EnvInitializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {
 
@@ -31,8 +31,8 @@ void verifyTraceSentHttpWithJaeger() throws Exception {
     }
 
     @Test
-    void verifyTraceWithArrayValueSentGrpcWithJaeger() throws Exception {
-        postResourceSpans();
+    void verifyProdTraceSentGrpcWithJaeger() throws Exception {
+        postProdSpans();
         awaitSpansExported(RESOURCE_TRACE_ID);
     }
 }
diff --git a/...t/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpGrpcTraceExporterIntTest.java b/...t/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpGrpcTraceExporterIntTest.java
@@ -10,8 +10,6 @@
 import rocks.inspectit.oce.eum.server.configuration.model.exporters.TransportProtocol;
 import rocks.inspectit.oce.eum.server.exporters.ExporterIntTestBaseWithOtelCollector;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 @DirtiesContext
 @ContextConfiguration(initializers = OtlpGrpcTraceExporterIntTest.EnvInitializer.class)
 public class OtlpGrpcTraceExporterIntTest extends ExporterIntTestBaseWithOtelCollector {
@@ -33,8 +31,8 @@ void verifyTraceSentGrpcWithOtlp() throws Exception {
     }
 
     @Test
-    void verifyTraceWithArrayValueSentGrpcWithOtlp() throws Exception {
-        postResourceSpans();
+    void verifyProdTraceSentGrpcWithOtlp() throws Exception {
+        postProdSpans();
         awaitSpansExported(RESOURCE_TRACE_ID);
     }
 }
diff --git a/...t/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpHttpTraceExporterIntTest.java b/...t/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpHttpTraceExporterIntTest.java
@@ -10,8 +10,6 @@
 import rocks.inspectit.oce.eum.server.configuration.model.exporters.TransportProtocol;
 import rocks.inspectit.oce.eum.server.exporters.ExporterIntTestBaseWithOtelCollector;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 @DirtiesContext
 @ContextConfiguration(initializers = OtlpHttpTraceExporterIntTest.EnvInitializer.class)
 public class OtlpHttpTraceExporterIntTest extends ExporterIntTestBaseWithOtelCollector {
@@ -33,8 +31,8 @@ void verifyTraceSentHttpWithOtlp() throws Exception {
     }
 
     @Test
-    void verifyTraceWithArrayValueSentHttpWithOtlp() throws Exception {
-        postResourceSpans();
+    void verifyProdTraceSentHttpWithOtlp() throws Exception {
+        postProdSpans();
         awaitSpansExported(RESOURCE_TRACE_ID);
     }
 }
diff --git a/src/test/java/rocks/inspectit/oce/eum/server/rest/TraceControllerIntTest.java b/src/test/java/rocks/inspectit/oce/eum/server/rest/TraceControllerIntTest.java
@@ -36,8 +36,8 @@ public class TraceControllerIntTest {
     @Value("classpath:ot-trace-large-v0.48.0.json")
     private Resource resourceSpan;
 
-    @Value("classpath:ot-trace-array-v0.48.0.json")
-    private Resource resourceSpans;
+    @Value("classpath:ot-trace-prod-v0.48.0.json")
+    private Resource prodResourceSpans;
 
     @MockBean
     SpanExporter spanExporter;
@@ -88,7 +88,7 @@ public void verifyLargeTrace() throws Exception {
 
     @Test
     public void verifyTraceWithMultipleResourceSpans() throws Exception {
-        try (Reader reader = new InputStreamReader(resourceSpans.getInputStream())) {
+        try (Reader reader = new InputStreamReader(prodResourceSpans.getInputStream())) {
             String json = CharStreams.toString(reader);
 
             ResponseEntity<Void> result = restTemplate.postForEntity("/spans", json, Void.class);

diff --git a/.../rocks/inspectit/oce/eum/server/tracing/opentelemtry/OpenTelemetryProtoConverterTest.java b/.../rocks/inspectit/oce/eum/server/tracing/opentelemtry/OpenTelemetryProtoConverterTest.java
@@ -41,7 +41,7 @@ class OpenTelemetryProtoConverterTest {
 
     public static final String TRACE_REQUEST_FILE_LARGE = "/ot-trace-large-v0.48.0.json";
 
-    public static final String TRACE_REQUEST_FILE_ARRAY = "/ot-trace-array-v0.48.0.json";
+    public static final String TRACE_REQUEST_FILE_PROD = "/ot-trace-prod-v0.48.0.json";
 
     @InjectMocks
     private OpenTelemetryProtoConverter converter;
@@ -57,8 +57,8 @@ private ExportTraceServiceRequest getLargeTestRequest() throws Exception {
         return getTestRequest(TRACE_REQUEST_FILE_LARGE);
     }
 
-    private ExportTraceServiceRequest getArrayTestRequest() throws Exception {
-        return getTestRequest(TRACE_REQUEST_FILE_ARRAY);
+    private ExportTraceServiceRequest getProdTestRequest() throws Exception {
+        return getTestRequest(TRACE_REQUEST_FILE_PROD);
     }
 
     private ExportTraceServiceRequest getTestRequest(String file) throws Exception {
@@ -153,7 +153,7 @@ public void convertLargeRequest() throws Exception {
 
         @Test
         void convertArrayRequest() throws Exception {
-            ExportTraceServiceRequest request = getArrayTestRequest();
+            ExportTraceServiceRequest request = getProdTestRequest();
 
             Collection<SpanData> result = converter.convert(request);
             List<SpanData> resultList = result.stream().toList();
@@ -196,7 +196,7 @@ class AnonymizeIpAddress {
         private HttpServletRequest mockRequest;
 
         @BeforeEach
-        private void beforeEach() {
+        public void beforeEach() {
             converter.requestSupplier = () -> mockRequest;
         }
 

diff --git a/...est/resources/ot-trace-array-v0.48.0.json → ...test/resources/ot-trace-prod-v0.48.0.json b/...est/resources/ot-trace-array-v0.48.0.json → ...test/resources/ot-trace-prod-v0.48.0.json