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

Draft: suppress instrumentation flag #3443

Closed
wants to merge 2 commits into from
+73 −11
Closed

Draft: suppress instrumentation flag #3443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87abd3b
draft: suppress_instrumentation
lmolkova Jun 30, 2021
ccb64a7
up
lmolkova Jun 30, 2021
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
27 changes: 16 additions & 11 deletions ...rumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.TextMapGetter;
import io.opentelemetry.context.propagation.TextMapPropagator;
Expand Down Expand Up @@ -50,6 +51,7 @@ public abstract class BaseTracer {

private final Tracer tracer;
private final ContextPropagators propagators;
public static final ContextKey<Boolean> SUPPRESS_INSTRUMENTATION_KEY = ContextKey.named("otel.suppress_instrumentation");

/**
* Instead of using this always pass an OpenTelemetry instance; javaagent tracers should
Expand Down Expand Up @@ -101,17 +103,20 @@ protected String getVersion() {
* @see #withServerSpan(Context, Span)
*/
public final boolean shouldStartSpan(Context context, SpanKind proposedKind) {
boolean suppressed = false;
switch (proposedKind) {
case CLIENT:
suppressed = ClientSpan.exists(context);
break;
case SERVER:
case CONSUMER:
suppressed = ServerSpan.exists(context) || ConsumerSpan.exists(context);
break;
default:
break;
Boolean suppresedContext = context.get(SUPPRESS_INSTRUMENTATION_KEY);
boolean suppressed = suppresedContext != null && suppresedContext.booleanValue();
if (!suppressed) {
switch (proposedKind) {
case CLIENT:
suppressed = ClientSpan.exists(context);
break;
case SERVER:
case CONSUMER:
suppressed = ServerSpan.exists(context) || ConsumerSpan.exists(context);
break;
default:
break;
}
}
if (suppressed) {
supportability.recordSuppressedSpan(proposedKind, getInstrumentationName());
Expand Down
28 changes: 28 additions & 0 deletions ...ion-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/BaseTracerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@ package io.opentelemetry.instrumentation.api.tracer
import io.opentelemetry.api.trace.Span
import io.opentelemetry.api.trace.SpanKind
import io.opentelemetry.context.Context
import io.opentelemetry.context.ContextKey
import spock.lang.Shared
import spock.lang.Specification

import static org.junit.Assume.assumeTrue

// TODO add tests for BaseTracer
class BaseTracerTest extends Specification {

@Shared
def tracer = newTracer()

Expand All @@ -21,6 +25,10 @@ class BaseTracerTest extends Specification {
@Shared
def root = Context.root()

@Shared
def ContextKey<Boolean> suppressContextKey = ContextKey.named("otel.suppress_instrumentation")
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used somewhere?

Copy link
Contributor Author

@lmolkova lmolkova Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

nope, this is a draft PR, I'm not even sure I want to proceed with it, let me close it so I don't confuse anyone.



@Shared
def existingSpan = Span.getInvalid()

Expand Down Expand Up @@ -59,6 +67,26 @@ class BaseTracerTest extends Specification {
SpanKind.CLIENT | tracer.withServerSpan(root, existingSpan) | true
}

def "test shouldStartSpan is false when instrumentation is suppressed"() {
when:
boolean result = tracer.shouldStartSpan(context, kind)

then:
result == expected

where:
kind | context | expected
SpanKind.CLIENT | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, false) | true
SpanKind.SERVER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, false) | true
SpanKind.INTERNAL | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, false) | true
SpanKind.CONSUMER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, false) | true
SpanKind.PRODUCER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, false) | true
SpanKind.CLIENT | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true) | false
SpanKind.SERVER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true) | false
SpanKind.INTERNAL | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true) | false
SpanKind.CONSUMER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true) | false
SpanKind.PRODUCER | root.with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true) | false
}

class SomeInnerClass implements Runnable {
void run() {
Expand Down
29 changes: 29 additions & 0 deletions ...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

package io.opentelemetry.instrumentation.test.base

import io.opentelemetry.context.ContextKey
import io.opentelemetry.context.Scope
import io.opentelemetry.instrumentation.api.tracer.BaseTracer

import static io.opentelemetry.api.trace.SpanKind.CLIENT
import static io.opentelemetry.api.trace.SpanKind.SERVER
import static io.opentelemetry.api.trace.StatusCode.ERROR
Expand Down Expand Up @@ -389,6 +393,31 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
method << BODY_METHODS
}

def "should suppress nested spans if instrumentation is suppressed (#method)"() {
when:
def uri = resolveAddress("/success")
def responseCode = runUnderParentClientSpan {
Scope s = Context.current().with(BaseTracer.SUPPRESS_INSTRUMENTATION_KEY, true).makeCurrent();
try {
doRequest(method, uri)
} finally {
s.close();
}
}

then:
responseCode == 200
// there should be 2 separate traces since the nested CLIENT span is suppressed
// (and the span context propagation along with it)
assertTraces(1) {
trace(0, 1) {
serverSpan(it, 0)
}
}

where:
method << BODY_METHODS
}

//FIXME: add tests for POST with large/chunked data

Expand Down