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

add SdkTracerProvider.setScopeConfigurator() and support #7021

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,12 @@

/** {@link ExtendedSdkTracer} is SDK implementation of {@link ExtendedTracer}. */
final class ExtendedSdkTracer extends SdkTracer implements ExtendedTracer {
// TODO: add dedicated API for updating scope config.
@SuppressWarnings("FieldCanBeFinal") // For now, allow updating reflectively.
private boolean tracerEnabled;
Copy link
Member

Choose a reason for hiding this comment

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

Needed to make sure the updates are visible to ExtendedTracer#isEnabled(). Updated SdkTracer.tracerEnabled to be protected, and removed this field, such that ExtendedSdkTracer#isEnabled() resolves using the value from SdkTracer.tracerEnabled.


ExtendedSdkTracer(
TracerSharedState sharedState,
InstrumentationScopeInfo instrumentationScopeInfo,
TracerConfig tracerConfig) {
super(sharedState, instrumentationScopeInfo, tracerConfig);
this.tracerEnabled = tracerConfig.isEnabled();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ class SdkTracer implements Tracer {

private final TracerSharedState sharedState;
private final InstrumentationScopeInfo instrumentationScopeInfo;

// TODO: add dedicated API for updating scope config.
@SuppressWarnings("FieldCanBeFinal") // For now, allow updating reflectively.
private boolean tracerEnabled;
// deliberately not volatile because of performance concerns
// - which means its eventually consistent
protected boolean tracerEnabled;

SdkTracer(
TracerSharedState sharedState,
Expand Down Expand Up @@ -79,4 +78,13 @@ public SpanBuilder spanBuilder(String spanName) {
InstrumentationScopeInfo getInstrumentationScopeInfo() {
return instrumentationScopeInfo;
}

// Visible for testing
boolean isEnabled() {
return tracerEnabled;
}

void updateTracerConfig(TracerConfig tracerConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

Removed a comment on this method which implied it may one day be public, since this class is package private and will stay that way.

this.tracerEnabled = tracerConfig.isEnabled();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.opentelemetry.sdk.internal.ComponentRegistry;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.internal.SdkTracerProviderUtil;
import io.opentelemetry.sdk.trace.internal.TracerConfig;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.Closeable;
Expand All @@ -30,7 +31,9 @@ public final class SdkTracerProvider implements TracerProvider, Closeable {
static final String DEFAULT_TRACER_NAME = "";
private final TracerSharedState sharedState;
private final ComponentRegistry<SdkTracer> tracerSdkComponentRegistry;
private final ScopeConfigurator<TracerConfig> tracerConfigurator;
// deliberately not volatile because of performance concerns
// - which means its eventually consistent
private ScopeConfigurator<TracerConfig> tracerConfigurator;

/**
* Returns a new {@link SdkTracerProviderBuilder} for {@link SdkTracerProvider}.
Expand Down Expand Up @@ -100,6 +103,25 @@ public Sampler getSampler() {
return sharedState.getSampler();
}

/**
* Updates the tracer configurator, which computes {@link TracerConfig} for each {@link
* InstrumentationScopeInfo}.
*
* <p>This method is experimental so not public. You may reflectively call it using {@link
* SdkTracerProviderUtil#setTracerConfigurator(SdkTracerProvider, ScopeConfigurator)}.
*
* @see TracerConfig#configuratorBuilder()
*/
void setTracerConfigurator(ScopeConfigurator<TracerConfig> tracerConfigurator) {
Copy link
Member

Choose a reason for hiding this comment

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

Updated naming to mirror SdkTracerProviderBuilder, added javadoc pointing to the SdkTracerProviderUtil method for updatingl

this.tracerConfigurator = tracerConfigurator;
this.tracerSdkComponentRegistry
.getComponents()
.forEach(
sdkTracer ->
sdkTracer.updateTracerConfig(
getTracerConfig(sdkTracer.getInstrumentationScopeInfo())));
}

/**
* Attempts to stop all the activity for {@link Tracer}s created by this provider. Calls {@link
* SpanProcessor#shutdown()} for all registered {@link SpanProcessor}s.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
Expand All @@ -24,6 +25,21 @@ public final class SdkTracerProviderUtil {

private SdkTracerProviderUtil() {}

/** Reflectively set the {@link ScopeConfigurator} to the {@link SdkTracerProvider}. */
public static void setTracerConfigurator(
SdkTracerProvider sdkTracerProvider, ScopeConfigurator<TracerConfig> scopeConfigurator) {
try {
Method method =
SdkTracerProvider.class.getDeclaredMethod(
"setScopeConfigurator", ScopeConfigurator.class);
method.setAccessible(true);
method.invoke(sdkTracerProvider, scopeConfigurator);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
throw new IllegalStateException(
"Error calling setTracerConfigurator on SdkTracerProvider", e);
}
}

/** Reflectively set the {@link ScopeConfigurator} to the {@link SdkTracerProviderBuilder}. */
public static void setTracerConfigurator(
SdkTracerProviderBuilder sdkTracerProviderBuilder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
import io.opentelemetry.sdk.common.Clock;
import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.internal.SdkTracerProviderUtil;
import io.opentelemetry.sdk.trace.internal.TracerConfig;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.util.function.Supplier;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -184,6 +187,35 @@ void propagatesInstrumentationScopeInfoToTracer() {
assertThat(((SdkTracer) tracer).getInstrumentationScopeInfo()).isEqualTo(expected);
}

@Test
void propagatesEnablementToTracerDirectly() {
propagatesEnablementToTracer(true);
}

@Test
void propagatesEnablementToTracerByUtil() {
propagatesEnablementToTracer(false);
}

void propagatesEnablementToTracer(boolean directly) {
SdkTracer tracer = (SdkTracer) tracerFactory.get("test");
boolean isEnabled = tracer.isEnabled();
ScopeConfigurator<TracerConfig> flipConfigurator =
new ScopeConfigurator<TracerConfig>() {
@Override
public TracerConfig apply(InstrumentationScopeInfo scopeInfo) {
return isEnabled ? TracerConfig.disabled() : TracerConfig.enabled();
}
};
// all in the same thread, so should see enablement change immediately
if (directly) {
tracerFactory.setTracerConfigurator(flipConfigurator);
} else {
SdkTracerProviderUtil.setTracerConfigurator(tracerFactory, flipConfigurator);
}
assertThat(tracer.isEnabled()).isEqualTo(!isEnabled);
}

@Test
void build_SpanLimits() {
SpanLimits initialSpanLimits = SpanLimits.builder().build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.internal.TracerConfig;
import java.util.Collection;
import java.util.concurrent.atomic.AtomicLong;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -50,6 +51,14 @@ void getInstrumentationScopeInfo() {
assertThat(tracer.getInstrumentationScopeInfo()).isEqualTo(instrumentationScopeInfo);
}

@Test
void updateEnabled() {
tracer.updateTracerConfig(TracerConfig.disabled());
assertThat(tracer.isEnabled()).isFalse();
tracer.updateTracerConfig(TracerConfig.enabled());
assertThat(tracer.isEnabled()).isTrue();
}

@Test
void propagatesInstrumentationScopeInfoToSpan() {
ReadableSpan readableSpan = (ReadableSpan) tracer.spanBuilder("spanName").startSpan();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import io.opentelemetry.api.incubator.trace.ExtendedTracer;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanId;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.context.Scope;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
Expand All @@ -42,9 +41,13 @@ void disableScopes() throws InterruptedException {
.addSpanProcessor(SimpleSpanProcessor.create(exporter))
.build();

Tracer tracerA = tracerProvider.get("tracerA");
Tracer tracerB = tracerProvider.get("tracerB");
Tracer tracerC = tracerProvider.get("tracerC");
ExtendedSdkTracer tracerA = (ExtendedSdkTracer) tracerProvider.get("tracerA");
ExtendedSdkTracer tracerB = (ExtendedSdkTracer) tracerProvider.get("tracerB");
ExtendedSdkTracer tracerC = (ExtendedSdkTracer) tracerProvider.get("tracerC");

assertThat(tracerA.isEnabled()).isTrue();
assertThat(tracerB.isEnabled()).isFalse();
assertThat(tracerC.isEnabled()).isTrue();

Span parent;
Span child;
Expand Down Expand Up @@ -158,4 +161,67 @@ private static Stream<Arguments> tracerConfiguratorArgs() {
Arguments.of(enableStartsWithD, scopeDog, enabled()),
Arguments.of(enableStartsWithD, scopeDuck, enabled()));
}

@Test
void setScopeConfigurator() {
Copy link
Member

Choose a reason for hiding this comment

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

New test to ensure changes reach ExtendedTracer#isEnabled() and have the desired affect.

// 1. Initially, configure all tracers to be enabled except tracerB
InMemorySpanExporter exporter = InMemorySpanExporter.create();
SdkTracerProvider tracerProvider =
SdkTracerProvider.builder()
.addTracerConfiguratorCondition(nameEquals("tracerB"), disabled())
.addSpanProcessor(SimpleSpanProcessor.create(exporter))
.build();

ExtendedSdkTracer tracerA = (ExtendedSdkTracer) tracerProvider.get("tracerA");
ExtendedSdkTracer tracerB = (ExtendedSdkTracer) tracerProvider.get("tracerB");
ExtendedSdkTracer tracerC = (ExtendedSdkTracer) tracerProvider.get("tracerC");

// verify isEnabled()
assertThat(tracerA.isEnabled()).isTrue();
assertThat(tracerB.isEnabled()).isFalse();
assertThat(tracerC.isEnabled()).isTrue();

// verify spans are emitted as expected
tracerA.spanBuilder("spanA").startSpan().end();
tracerB.spanBuilder("spanB").startSpan().end();
tracerC.spanBuilder("spanC").startSpan().end();
assertThat(exporter.getFinishedSpanItems())
.satisfiesExactlyInAnyOrder(
span -> assertThat(span).hasName("spanA"), span -> assertThat(span).hasName("spanC"));
exporter.reset();

// 2. Update config to disable all tracers
tracerProvider.setTracerConfigurator(
ScopeConfigurator.<TracerConfig>builder().setDefault(TracerConfig.disabled()).build());

// verify isEnabled()
assertThat(tracerA.isEnabled()).isFalse();
assertThat(tracerB.isEnabled()).isFalse();
assertThat(tracerC.isEnabled()).isFalse();

// verify spans are emitted as expected
tracerA.spanBuilder("spanA").startSpan().end();
tracerB.spanBuilder("spanB").startSpan().end();
tracerC.spanBuilder("spanC").startSpan().end();
assertThat(exporter.getFinishedSpanItems()).isEmpty();

// 3. Update config to restore original
tracerProvider.setTracerConfigurator(
ScopeConfigurator.<TracerConfig>builder()
.addCondition(nameEquals("tracerB"), disabled())
.build());

// verify isEnabled()
assertThat(tracerA.isEnabled()).isTrue();
assertThat(tracerB.isEnabled()).isFalse();
assertThat(tracerC.isEnabled()).isTrue();

// verify spans are emitted as expected
tracerA.spanBuilder("spanA").startSpan().end();
tracerB.spanBuilder("spanB").startSpan().end();
tracerC.spanBuilder("spanC").startSpan().end();
assertThat(exporter.getFinishedSpanItems())
.satisfiesExactly(
span -> assertThat(span).hasName("spanA"), span -> assertThat(span).hasName("spanC"));
}
}
Loading