Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace raw type for AttributeKey #55

Merged
merged 1 commit into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 26 additions & 44 deletions src/main/java/io/opentelemetry/sdk/trace/OcelotSpanUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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";
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ void verifyTraceSentGrpcWithJaeger() throws Exception {
}

@Test
void verifyTraceWithArrayValueSentGrpcWithJaeger() throws Exception {
postResourceSpans();
void verifyProdTraceSentGrpcWithJaeger() throws Exception {
postProdSpans();
awaitSpansExported(RESOURCE_TRACE_ID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand All @@ -31,8 +31,8 @@ void verifyTraceSentHttpWithJaeger() throws Exception {
}

@Test
void verifyTraceWithArrayValueSentGrpcWithJaeger() throws Exception {
postResourceSpans();
void verifyProdTraceSentGrpcWithJaeger() throws Exception {
postProdSpans();
awaitSpansExported(RESOURCE_TRACE_ID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -33,8 +31,8 @@ void verifyTraceSentGrpcWithOtlp() throws Exception {
}

@Test
void verifyTraceWithArrayValueSentGrpcWithOtlp() throws Exception {
postResourceSpans();
void verifyProdTraceSentGrpcWithOtlp() throws Exception {
postProdSpans();
awaitSpansExported(RESOURCE_TRACE_ID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -33,8 +31,8 @@ void verifyTraceSentHttpWithOtlp() throws Exception {
}

@Test
void verifyTraceWithArrayValueSentHttpWithOtlp() throws Exception {
postResourceSpans();
void verifyProdTraceSentHttpWithOtlp() throws Exception {
postProdSpans();
awaitSpansExported(RESOURCE_TRACE_ID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -196,7 +196,7 @@ class AnonymizeIpAddress {
private HttpServletRequest mockRequest;

@BeforeEach
private void beforeEach() {
public void beforeEach() {
converter.requestSupplier = () -> mockRequest;
}

Expand Down
Loading