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.

Sign up for GitHub

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

Consistent application of exporter customizers when otel.{signal}.exporter=none #7017

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Consistent application of exporter customizers when otel.{signal}.exporter=none #7017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion sdk-extensions/autoconfigure/build.gradle.kts
View file
Open in desktop
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")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was interfering with the changes I was making to FullConfigTest. Setting span limits is well tested elsewhere, and we should limit the integration-style FullConfigTest to concepts that really require everything to be stood up in an end-to-end fashion.

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading