From a7996784b6e43e5de0b2de390625a61243149b9c Mon Sep 17 00:00:00 2001 From: Eduard Schander <66794307+EddeCCC@users.noreply.github.com> Date: Thu, 7 Mar 2024 11:32:47 +0100 Subject: [PATCH] replace raw type for AttributeKey (#55) --- .../sdk/trace/OcelotSpanUtils.java | 70 +++++++------------ .../sdk/trace/OcelotSpanUtilsTest.java | 7 +- .../exporters/ExporterIntMockMvcTestBase.java | 19 +++-- .../tracing/JaegerGrpcExporterIntTest.java | 4 +- .../tracing/JaegerHttpExporterIntTest.java | 6 +- .../tracing/OtlpGrpcTraceExporterIntTest.java | 6 +- .../tracing/OtlpHttpTraceExporterIntTest.java | 6 +- .../server/rest/TraceControllerIntTest.java | 6 +- .../OpenTelemetryProtoConverterTest.java | 10 +-- ...0.48.0.json => ot-trace-prod-v0.48.0.json} | 0 10 files changed, 56 insertions(+), 78 deletions(-) rename src/test/resources/{ot-trace-array-v0.48.0.json => ot-trace-prod-v0.48.0.json} (100%) diff --git a/src/main/java/io/opentelemetry/sdk/trace/OcelotSpanUtils.java b/src/main/java/io/opentelemetry/sdk/trace/OcelotSpanUtils.java index 6b65739..7796384 100644 --- 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 attributesList, Map 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 attributeKey = AttributeKey.stringKey(key); + builder.put(attributeKey, value.getStringValue()); + } + case BOOL_VALUE -> { + AttributeKey attributeKey = AttributeKey.booleanKey(key); + builder.put(attributeKey, value.getBoolValue()); + } + case INT_VALUE -> { + AttributeKey attributeKey = AttributeKey.longKey(key); + builder.put(attributeKey, value.getIntValue()); + } + case DOUBLE_VALUE -> { + AttributeKey attributeKey = AttributeKey.doubleKey(key); + builder.put(attributeKey, value.getDoubleValue()); + } + case ARRAY_VALUE -> { + AttributeKey> attributeKey = AttributeKey.stringArrayKey(key); + List stringValues = value.getArrayValue().getValuesList().stream() + .map(OcelotSpanUtils::getValueAsString) + .toList(); + builder.put(attributeKey, stringValues); } } } @@ -253,23 +269,6 @@ public static Attributes toAttributes(List attributesList, Map 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 issue - * - * @param arrayValue the array containing any values - * - * @return the merged string of all values - */ - private static String mergeArray(ArrayValue arrayValue) { - List 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 index f878bab..f1c0be1 100644 --- 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 keyValues = Collections.singletonList(kvArray); + AttributeKey> 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 index 63def14..742c8f3 100644 --- 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. *
- * Currently, OTel is not able to process arrayValue objects in Attributes. - * Instead, all values will be merged to one string. - * See issue - * + * 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/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerGrpcExporterIntTest.java b/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerGrpcExporterIntTest.java index 4cef4ff..1a96f69 100644 --- a/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerGrpcExporterIntTest.java +++ b/src/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/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerHttpExporterIntTest.java b/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerHttpExporterIntTest.java index 940ad7b..fcb8658 100644 --- a/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/JaegerHttpExporterIntTest.java +++ b/src/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 { @@ -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/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpGrpcTraceExporterIntTest.java b/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpGrpcTraceExporterIntTest.java index e10158f..1fb91d2 100644 --- a/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpGrpcTraceExporterIntTest.java +++ b/src/test/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/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpHttpTraceExporterIntTest.java b/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpHttpTraceExporterIntTest.java index 89ca186..d043956 100644 --- a/src/test/java/rocks/inspectit/oce/eum/server/exporters/tracing/OtlpHttpTraceExporterIntTest.java +++ b/src/test/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 index 0ee3e30..37fa0dc 100644 --- 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 result = restTemplate.postForEntity("/spans", json, Void.class); diff --git a/src/test/java/rocks/inspectit/oce/eum/server/tracing/opentelemtry/OpenTelemetryProtoConverterTest.java b/src/test/java/rocks/inspectit/oce/eum/server/tracing/opentelemtry/OpenTelemetryProtoConverterTest.java index 2cdfb3f..821d043 100644 --- a/src/test/java/rocks/inspectit/oce/eum/server/tracing/opentelemtry/OpenTelemetryProtoConverterTest.java +++ b/src/test/java/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 result = converter.convert(request); List 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/src/test/resources/ot-trace-array-v0.48.0.json b/src/test/resources/ot-trace-prod-v0.48.0.json similarity index 100% rename from src/test/resources/ot-trace-array-v0.48.0.json rename to src/test/resources/ot-trace-prod-v0.48.0.json