Skip to content

Commit

Permalink
Consistent application of exporter customizers when otel.{signal}.exp…
Browse files Browse the repository at this point in the history 
…orter=none (#7017)
  • Loading branch information
@jack-berg
jack-berg authored Jan 16, 2025
1 parent 97410cb commit a1c0d0b
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 269 deletions.
1 change: 0 additions & 1 deletion sdk-extensions/autoconfigure/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ testing {
environment("OTEL_PROPAGATORS", "tracecontext,baggage,b3,b3multi,jaeger,ottrace,test")
environment("OTEL_EXPORTER_OTLP_HEADERS", "cat=meow,dog=bark")
environment("OTEL_EXPORTER_OTLP_TIMEOUT", "5000")
environment("OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "2")
environment("OTEL_TEST_CONFIGURED", "true")
environment("OTEL_TEST_WRAPPED", "1")
}
Expand Down
8 changes: 1 addition & 7 deletions ...gure/src/main/java/io/opentelemetry/sdk/autoconfigure/LogRecordExporterConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,7 @@ static Map<String, LogRecordExporter> configureLogRecordExporters(
throw new ConfigurationException(
"otel.logs.exporter contains " + EXPORTER_NONE + " along with other exporters");
}
LogRecordExporter noop = LogRecordExporter.composite();
LogRecordExporter customized = logRecordExporterCustomizer.apply(noop, config);
if (customized == noop) {
return Collections.emptyMap();
}
closeables.add(customized);
return Collections.singletonMap(EXPORTER_NONE, customized);
return Collections.emptyMap();
}

if (exporterNames.isEmpty()) {
Expand Down
8 changes: 1 addition & 7 deletions ...configure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,7 @@ static Map<String, SpanExporter> configureSpanExporters(
throw new ConfigurationException(
"otel.traces.exporter contains " + EXPORTER_NONE + " along with other exporters");
}
SpanExporter noop = SpanExporter.composite();
SpanExporter customized = spanExporterCustomizer.apply(noop, config);
if (customized == noop) {
return Collections.emptyMap();
}
closeables.add(customized);
return Collections.singletonMap(EXPORTER_NONE, customized);
return Collections.emptyMap();
}

if (exporterNames.isEmpty()) {
Expand Down
84 changes: 0 additions & 84 deletions .../src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -54,14 +52,10 @@
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter;
import io.opentelemetry.sdk.trace.IdGenerator;
import io.opentelemetry.sdk.trace.ReadWriteSpan;
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
import io.opentelemetry.sdk.trace.SpanProcessor;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.IOException;
import java.math.BigDecimal;
Expand Down Expand Up @@ -98,8 +92,6 @@ class AutoConfiguredOpenTelemetrySdkTest {
@Mock private TextMapGetter<Map<String, String>> getter;
@Mock private Sampler sampler1;
@Mock private Sampler sampler2;
@Mock private SpanExporter spanExporter1;
@Mock private SpanExporter spanExporter2;
@Mock private MetricReader metricReader;
@Mock private LogRecordProcessor logRecordProcessor;

Expand Down Expand Up @@ -247,76 +239,6 @@ void builder_addSamplerCustomizer() {
.isEqualTo(sampler2);
}

@Test
void builder_addSpanExporterCustomizer() {
Mockito.lenient().when(spanExporter2.shutdown()).thenReturn(CompletableResultCode.ofSuccess());

SdkTracerProvider sdkTracerProvider =
builder
.addSpanExporterCustomizer(
(previous, config) -> {
assertThat(previous).isSameAs(SpanExporter.composite());
return spanExporter1;
})
.addSpanExporterCustomizer(
(previous, config) -> {
assertThat(previous).isSameAs(spanExporter1);
return spanExporter2;
})
.build()
.getOpenTelemetrySdk()
.getSdkTracerProvider();

assertThat(sdkTracerProvider)
.extracting("sharedState")
.extracting("activeSpanProcessor")
.extracting("worker")
.extracting("spanExporter")
.isEqualTo(spanExporter2);
}

@Test
void builder_addSpanProcessorCustomizer() {
SpanProcessor mockProcessor1 = Mockito.mock(SpanProcessor.class);
SpanProcessor mockProcessor2 = Mockito.mock(SpanProcessor.class);
when(mockProcessor2.isStartRequired()).thenReturn(true);
when(mockProcessor2.isEndRequired()).thenReturn(true);
Mockito.lenient().doReturn(CompletableResultCode.ofSuccess()).when(mockProcessor2).shutdown();
Mockito.lenient().when(spanExporter1.shutdown()).thenReturn(CompletableResultCode.ofSuccess());

SdkTracerProvider sdkTracerProvider =
builder
.addSpanExporterCustomizer((prev, config) -> spanExporter1)
.addSpanProcessorCustomizer(
(previous, config) -> {
assertThat(previous).isNotSameAs(mockProcessor2);
return mockProcessor1;
})
.addSpanProcessorCustomizer(
(previous, config) -> {
assertThat(previous).isSameAs(mockProcessor1);
return mockProcessor2;
})
.build()
.getOpenTelemetrySdk()
.getSdkTracerProvider();

assertThat(sdkTracerProvider)
.extracting("sharedState")
.extracting("activeSpanProcessor")
.isSameAs(mockProcessor2);

Span span = sdkTracerProvider.get("dummy-scope").spanBuilder("dummy-span").startSpan();

verify(mockProcessor2).onStart(any(), same((ReadWriteSpan) span));

span.end();
verify(mockProcessor2).onEnd(same((ReadableSpan) span));

verifyNoInteractions(mockProcessor1);
verifyNoInteractions(spanExporter1);
}

@Test
void builder_addAutoConfigurationCustomizerProviderUsingComponentLoader() {
AutoConfigurationCustomizerProvider customizerProvider =
Expand Down Expand Up @@ -420,8 +342,6 @@ void builder_addMeterProviderCustomizer() {
verify(metricReader).forceFlush();
}

// TODO: add test for addMetricExporterCustomizer once OTLP export is enabled by default

@Test
void builder_addLoggerProviderCustomizer() {
Mockito.lenient()
Expand All @@ -442,10 +362,6 @@ void builder_addLoggerProviderCustomizer() {
verify(logRecordProcessor).forceFlush();
}

// TODO: add test for addLogRecordExporterCustomizer once OTLP export is enabled by default

// TODO: add test for addLogRecordProcessorCustomizer once OTLP export is enabled by default

@Test
void builder_setResultAsGlobalFalse() {
GlobalOpenTelemetry.set(OpenTelemetry.noop());
Expand Down
124 changes: 55 additions & 69 deletions ...oconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/FullConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
import org.junit.jupiter.api.extension.RegisterExtension;

@SuppressWarnings("InterruptedExceptionSwallowed")
class FullConfigTest {
public class FullConfigTest {

private static final BlockingQueue<ExportTraceServiceRequest> otlpTraceRequests =
new LinkedBlockingDeque<>();
Expand Down Expand Up @@ -193,7 +193,6 @@ void configures() throws Exception {
.spanBuilder("test")
.startSpan()
.setAttribute("cat", "meow")
.setAttribute("dog", "bark")
.end();

Meter meter = GlobalOpenTelemetry.get().getMeter("test");
Expand All @@ -209,7 +208,6 @@ void configures() throws Exception {

EventLogger eventLogger = GlobalEventLoggerProvider.get().eventLoggerBuilder("test").build();
eventLogger.builder("namespace.test-name").put("cow", "moo").emit();
;

openTelemetrySdk.getSdkTracerProvider().forceFlush().join(10, TimeUnit.SECONDS);
openTelemetrySdk.getSdkLoggerProvider().forceFlush().join(10, TimeUnit.SECONDS);
Expand All @@ -218,37 +216,19 @@ void configures() throws Exception {
await().untilAsserted(() -> assertThat(otlpTraceRequests).hasSize(1));

ExportTraceServiceRequest traceRequest = otlpTraceRequests.take();
assertThat(traceRequest.getResourceSpans(0).getResource().getAttributesList())
.contains(
KeyValue.newBuilder()
.setKey("service.name")
.setValue(AnyValue.newBuilder().setStringValue("test").build())
.build(),
KeyValue.newBuilder()
.setKey("cat")
.setValue(AnyValue.newBuilder().setStringValue("meow").build())
.build());
List<KeyValue> spanResourceAttributes =
traceRequest.getResourceSpans(0).getResource().getAttributesList();
assertHasKeyValue(spanResourceAttributes, "service.name", "test");
assertHasKeyValue(spanResourceAttributes, "cat", "meow");
io.opentelemetry.proto.trace.v1.Span span =
traceRequest.getResourceSpans(0).getScopeSpans(0).getSpans(0);
// Dog dropped by attribute limit.
assertThat(span.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("configured")
.setValue(AnyValue.newBuilder().setBoolValue(true).build())
.build(),
KeyValue.newBuilder()
.setKey("wrapped")
.setValue(AnyValue.newBuilder().setIntValue(1).build())
.build(),
KeyValue.newBuilder()
.setKey("cat")
.setValue(AnyValue.newBuilder().setStringValue("meow").build())
.build());
assertHasKeyValue(span.getAttributesList(), "configured", true);
assertHasKeyValue(span.getAttributesList(), "wrapped", 1);
assertHasKeyValue(span.getAttributesList(), "cat", "meow");
assertHasKeyValue(span.getAttributesList(), "extra-key", "extra-value");

// await on assertions since metrics may come in different order for BatchSpanProcessor,
// exporter, or the ones we
// created in the test.
// exporter, or the ones we created in the test.
await()
.untilAsserted(
() -> {
Expand All @@ -257,16 +237,10 @@ void configures() throws Exception {
assertThat(metricRequest.getResourceMetricsList())
.satisfiesExactly(
resourceMetrics -> {
assertThat(resourceMetrics.getResource().getAttributesList())
.contains(
KeyValue.newBuilder()
.setKey("service.name")
.setValue(AnyValue.newBuilder().setStringValue("test").build())
.build(),
KeyValue.newBuilder()
.setKey("cat")
.setValue(AnyValue.newBuilder().setStringValue("meow").build())
.build());
List<KeyValue> metricResourceAttributes =
resourceMetrics.getResource().getAttributesList();
assertHasKeyValue(metricResourceAttributes, "service.name", "test");
assertHasKeyValue(metricResourceAttributes, "cat", "meow");
assertThat(resourceMetrics.getScopeMetricsList())
.anySatisfy(
scopeMetrics -> {
Expand All @@ -277,18 +251,10 @@ void configures() throws Exception {
// SPI was loaded
assertThat(metric.getName()).isEqualTo("my-metric");
// TestMeterProviderConfigurer configures a view that
// only passes on attribute
// named allowed
// only passes an attribute named "allowed"
// configured-test
assertThat(getFirstDataPointLabels(metric))
.contains(
KeyValue.newBuilder()
.setKey("allowed")
.setValue(
AnyValue.newBuilder()
.setStringValue("bear")
.build())
.build());
assertHasKeyValue(
getFirstDataPointLabels(metric), "allowed", "bear");
});
})
// This verifies that AutoConfigureListener was invoked and the OTLP
Expand All @@ -312,21 +278,20 @@ void configures() throws Exception {

await().untilAsserted(() -> assertThat(otlpLogsRequests).hasSize(1));
ExportLogsServiceRequest logRequest = otlpLogsRequests.take();
assertThat(logRequest.getResourceLogs(0).getResource().getAttributesList())
.contains(
KeyValue.newBuilder()
.setKey("service.name")
.setValue(AnyValue.newBuilder().setStringValue("test").build())
.build(),
KeyValue.newBuilder()
.setKey("cat")
.setValue(AnyValue.newBuilder().setStringValue("meow").build())
.build());
List<KeyValue> logResourceAttributes =
logRequest.getResourceLogs(0).getResource().getAttributesList();
assertHasKeyValue(logResourceAttributes, "service.name", "test");
assertHasKeyValue(logResourceAttributes, "cat", "meow");

assertThat(logRequest.getResourceLogs(0).getScopeLogs(0).getLogRecordsList())
// LogRecordCustomizer customizes BatchLogProcessor to add an extra attribute on every log
// record
.allSatisfy(
logRecord ->
assertHasKeyValue(logRecord.getAttributesList(), "extra-key", "extra-value"))
.satisfiesExactlyInAnyOrder(
logRecord -> {
// LogRecordExporterCustomizer filters logs not whose level is less than Severity.INFO
// LogRecordCustomizer filters logs not whose level is less than Severity.INFO
assertThat(logRecord.getBody().getStringValue()).isEqualTo("info log message");
assertThat(logRecord.getSeverityNumberValue())
.isEqualTo(Severity.INFO.getSeverityNumber());
Expand All @@ -340,16 +305,37 @@ void configures() throws Exception {
.build());
assertThat(logRecord.getSeverityNumber())
.isEqualTo(SeverityNumber.SEVERITY_NUMBER_INFO);
assertThat(logRecord.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.name")
.setValue(
AnyValue.newBuilder().setStringValue("namespace.test-name").build())
.build());
assertHasKeyValue(logRecord.getAttributesList(), "event.name", "namespace.test-name");
});
}

private static void assertHasKeyValue(List<KeyValue> keyValues, String key, boolean value) {
assertThat(keyValues)
.contains(
KeyValue.newBuilder()
.setKey(key)
.setValue(AnyValue.newBuilder().setBoolValue(value))
.build());
}

private static void assertHasKeyValue(List<KeyValue> keyValues, String key, long value) {
assertThat(keyValues)
.contains(
KeyValue.newBuilder()
.setKey(key)
.setValue(AnyValue.newBuilder().setIntValue(value))
.build());
}

private static void assertHasKeyValue(List<KeyValue> keyValues, String key, String value) {
assertThat(keyValues)
.contains(
KeyValue.newBuilder()
.setKey(key)
.setValue(AnyValue.newBuilder().setStringValue(value))
.build());
}

private static List<KeyValue> getFirstDataPointLabels(Metric metric) {
switch (metric.getDataCase()) {
case GAUGE:
Expand Down
Loading

0 comments on commit a1c0d0b

Please sign in to comment.