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

Do not attach empty sentry-trace and baggage headers #2385

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Fix `Gpu.vendorId` should be a String ([#2343](https://github.com/getsentry/sentry-java/pull/2343))
- Do not attach empty `sentry-trace` and `baggage` headers ([#2385](https://github.com/getsentry/sentry-java/pull/2385))

### Features

Expand Down
1 change: 1 addition & 0 deletions buildSrc/src/main/java/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ object Config {
val SENTRY_SPRING_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring"
val SENTRY_SPRING_BOOT_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring-boot"
val SENTRY_SPRING_BOOT_JAKARTA_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring-boot.jakarta"
val SENTRY_OPENTELEMETRY_AGENT_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.opentelemetry.agent"
val group = "io.sentry"
val description = "SDK for sentry.io"
val versionNameProp = "versionName"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SentryOkHttpInterceptor(
var code: Int? = null
try {
val requestBuilder = request.newBuilder()
if (span != null &&
if (span != null && !span.isNoOp &&
PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url.toString())
) {
span.toSentryTrace().let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH

var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList()

if (PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url)) {
if (!span.isNoOp && PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url)) {
val sentryTraceHeader = span.toSentryTrace()
val baggageHeader = span.toBaggageHeader(request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value })
cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,23 @@ class SentryApolloInterceptor(
chain.proceedAsync(request, dispatcher, callBack)
} else {
val span = startChild(request, activeSpan)
val sentryTraceHeader = span.toSentryTrace()

// we have no access to URI, no way to verify tracing origins
val requestHeaderBuilder = request.requestHeaders.toBuilder()
requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value)
span.toBaggageHeader(listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)))?.let {
requestHeaderBuilder.addHeader(it.name, it.value)
val requestWithHeader = if (span.isNoOp) {
request
} else {
val sentryTraceHeader = span.toSentryTrace()

// we have no access to URI, no way to verify tracing origins
val requestHeaderBuilder = request.requestHeaders.toBuilder()
requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value)
span.toBaggageHeader(listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)))
?.let {
requestHeaderBuilder.addHeader(it.name, it.value)
}
val headers = requestHeaderBuilder.build()
request.toBuilder().requestHeaders(headers).build()
}
val headers = requestHeaderBuilder.build()
val requestWithHeader = request.toBuilder().requestHeaders(headers).build()

span.setData("operationId", requestWithHeader.operation.operationId())
span.setData("variables", requestWithHeader.operation.variables().valueMap().toString())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public SentryJdbcEventListener() {
@Override
public void onBeforeAnyExecute(final @NotNull StatementInformation statementInformation) {
final ISpan parent = hub.getSpan();
if (parent != null) {
if (parent != null && !parent.isNoOp()) {
final ISpan span = parent.startChild("db.query", statementInformation.getSql());
CURRENT_SPAN.set(span);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O

final RequestWrapper requestWrapper = new RequestWrapper(request);

if (PropagationTargetsUtils.contain(hub.getOptions().getTracePropagationTargets(), url)) {
if (!span.isNoOp()
&& PropagationTargetsUtils.contain(hub.getOptions().getTracePropagationTargets(), url)) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
final @Nullable Collection<String> requestBaggageHeader =
request.headers().get(BaggageHeader.BAGGAGE_HEADER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ tasks {
attributes.put("Can-Retransform-Classes", "true")
attributes.put("Implementation-Vendor", "Sentry")
attributes.put("Implementation-Version", "sentry-${project.version}-otel-${Config.Libs.otelJavaagentVersion}")
attributes.put("Sentry-Version-Name", project.version)
attributes.put("Sentry-Opentelemetry-SDK-Name", Config.Sentry.SENTRY_OPENTELEMETRY_AGENT_SDK_NAME)
attributes.put("Sentry-Opentelemetry-Version-Name", Config.Libs.otelVersion)
attributes.put("Sentry-Opentelemetry-Javaagent-Version-Name", Config.Libs.otelJavaagentVersion)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,87 @@
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder;
import io.sentry.Instrumenter;
import io.sentry.Sentry;
import io.sentry.SentryOptions;
import io.sentry.protocol.SdkVersion;
import java.io.IOException;
import java.net.URL;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.jar.Attributes;
import java.util.jar.Manifest;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public final class SentryAutoConfigurationCustomizerProvider
implements AutoConfigurationCustomizerProvider {

@Override
public void customize(AutoConfigurationCustomizer autoConfiguration) {
final @Nullable String sentryPropertiesFile = System.getenv("SENTRY_PROPERTIES_FILE");
final @Nullable String sentryDsn = System.getenv("SENTRY_DSN");

if (sentryPropertiesFile != null || sentryDsn != null) {
Sentry.init(
options -> {
options.setEnableExternalConfiguration(true);
options.setInstrumenter(Instrumenter.OTEL);
final @Nullable SdkVersion sdkVersion = createSdkVersion(options);
if (sdkVersion != null) {
options.setSdkVersion(sdkVersion);
}
});
}

autoConfiguration
.addTracerProviderCustomizer(this::configureSdkTracerProvider)
.addPropertiesSupplier(this::getDefaultProperties);
}

private @Nullable SdkVersion createSdkVersion(final @NotNull SentryOptions sentryOptions) {
SdkVersion sdkVersion = sentryOptions.getSdkVersion();

try {
final @NotNull Enumeration<URL> resources =
ClassLoader.getSystemClassLoader().getResources("META-INF/MANIFEST.MF");
while (resources.hasMoreElements()) {
try {
final @NotNull Manifest manifest = new Manifest(resources.nextElement().openStream());
final @Nullable Attributes mainAttributes = manifest.getMainAttributes();
if (mainAttributes != null) {
final @Nullable String name = mainAttributes.getValue("Sentry-Opentelemetry-SDK-Name");
final @Nullable String version = mainAttributes.getValue("Sentry-Version-Name");

if (name != null && version != null) {
sdkVersion = SdkVersion.updateSdkVersion(sdkVersion, name, version);
sdkVersion.addPackage("maven:io.sentry:sentry-opentelemetry-agent", version);
final @Nullable String otelVersion =
mainAttributes.getValue("Sentry-Opentelemetry-Version-Name");
if (otelVersion != null) {
sdkVersion.addPackage("maven:io.opentelemetry:opentelemetry-sdk", otelVersion);
}
final @Nullable String otelJavaagentVersion =
mainAttributes.getValue("Sentry-Opentelemetry-Javaagent-Version-Name");
if (otelJavaagentVersion != null) {
sdkVersion.addPackage(
"maven:io.opentelemetry.javaagent:opentelemetry-javaagent",
otelJavaagentVersion);
}
}
}
} catch (Exception e) {
// ignore
}
}
} catch (IOException e) {
// ignore
}

return sdkVersion;
}

private SdkTracerProviderBuilder configureSdkTracerProvider(
SdkTracerProviderBuilder tracerProvider, ConfigProperties config) {
return tracerProvider.addSpanProcessor(new SentrySpanProcessor());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public <C> void inject(final Context context, final C carrier, final TextMapSett
return;
}
final @Nullable ISpan sentrySpan = spanStorage.get(otelSpanContext.getSpanId());
if (sentrySpan == null) {
if (sentrySpan == null || sentrySpan.isNoOp()) {
return;
}

Expand Down Expand Up @@ -75,10 +75,8 @@ public <C> Context extract(
Context modifiedContext = context.with(SentryOtelKeys.SENTRY_TRACE_KEY, sentryTraceHeader);

final @Nullable String baggageString = getter.get(carrier, BaggageHeader.BAGGAGE_HEADER);
if (baggageString != null) {
Baggage baggage = Baggage.fromHeader(baggageString);
modifiedContext = modifiedContext.with(SentryOtelKeys.SENTRY_BAGGAGE_KEY, baggage);
}
Baggage baggage = Baggage.fromHeader(baggageString);
modifiedContext = modifiedContext.with(SentryOtelKeys.SENTRY_BAGGAGE_KEY, baggage);

Span wrappedSpan = Span.wrap(otelSpanContext);
modifiedContext = modifiedContext.with(wrappedSpan);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public SentrySpanAdvice(final @NotNull IHub hub) {
public Object invoke(final @NotNull MethodInvocation invocation) throws Throwable {
final ISpan activeSpan = hub.getSpan();

if (activeSpan == null) {
if (activeSpan == null || activeSpan.isNoOp()) {
// there is no active transaction, we do not start new span
return invocation.proceed();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
request.getMethod() != null ? request.getMethod().name() : "unknown";
span.setDescription(methodName + " " + request.getURI());

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();

if (PropagationTargetsUtils.contain(
if (!span.isNoOp() && PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.getURI())) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
@Nullable
BaggageHeader baggage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) {
final ISpan span = activeSpan.startChild("http.client");
span.setDescription(request.method().name() + " " + request.url());

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();

final ClientRequest.Builder requestBuilder = ClientRequest.from(request);

if (PropagationTargetsUtils.contain(
if (!span.isNoOp() && PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.url())) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());

final @Nullable BaggageHeader baggageHeader =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public SentrySpanAdvice(final @NotNull IHub hub) {
public Object invoke(final @NotNull MethodInvocation invocation) throws Throwable {
final ISpan activeSpan = hub.getSpan();

if (activeSpan == null) {
if (activeSpan == null || activeSpan.isNoOp()) {
// there is no active transaction, we do not start new span
return invocation.proceed();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
request.getMethod() != null ? request.getMethod().name() : "unknown";
span.setDescription(methodName + " " + request.getURI());

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();

if (PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.getURI())) {
if (!span.isNoOp()
&& PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.getURI())) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
@Nullable
BaggageHeader baggage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,13 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) {
final ISpan span = activeSpan.startChild("http.client");
span.setDescription(request.method().name() + " " + request.url());

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();

final ClientRequest.Builder requestBuilder = ClientRequest.from(request);

if (PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.url())) {
if (!span.isNoOp()
&& PropagationTargetsUtils.contain(
hub.getOptions().getTracePropagationTargets(), request.url())) {

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());

final @Nullable BaggageHeader baggageHeader =
Expand Down
5 changes: 5 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ public abstract interface class io/sentry/ISpan {
public abstract fun getTag (Ljava/lang/String;)Ljava/lang/String;
public abstract fun getThrowable ()Ljava/lang/Throwable;
public abstract fun isFinished ()Z
public abstract fun isNoOp ()Z
public abstract fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public abstract fun setDescription (Ljava/lang/String;)V
public abstract fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down Expand Up @@ -747,6 +748,7 @@ public final class io/sentry/NoOpSpan : io/sentry/ISpan {
public fun getTag (Ljava/lang/String;)Ljava/lang/String;
public fun getThrowable ()Ljava/lang/Throwable;
public fun isFinished ()Z
public fun isNoOp ()Z
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
public fun setDescription (Ljava/lang/String;)V
public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down Expand Up @@ -783,6 +785,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
public fun getThrowable ()Ljava/lang/Throwable;
public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource;
public fun isFinished ()Z
public fun isNoOp ()Z
public fun isProfileSampled ()Ljava/lang/Boolean;
public fun isSampled ()Ljava/lang/Boolean;
public fun scheduleFinish ()V
Expand Down Expand Up @@ -1587,6 +1590,7 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction {
public fun getTimestamp ()Ljava/lang/Double;
public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource;
public fun isFinished ()Z
public fun isNoOp ()Z
public fun isProfileSampled ()Ljava/lang/Boolean;
public fun isSampled ()Ljava/lang/Boolean;
public fun scheduleFinish ()V
Expand Down Expand Up @@ -1698,6 +1702,7 @@ public final class io/sentry/Span : io/sentry/ISpan {
public fun getTimestamp ()Ljava/lang/Double;
public fun getTraceId ()Lio/sentry/protocol/SentryId;
public fun isFinished ()Z
public fun isNoOp ()Z
public fun isProfileSampled ()Ljava/lang/Boolean;
public fun isSampled ()Ljava/lang/Boolean;
public fun setData (Ljava/lang/String;Ljava/lang/Object;)V
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/Baggage.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public final class Baggage {
final @NotNull ILogger logger;

@NotNull
public static Baggage fromHeader(final String headerValue) {
public static Baggage fromHeader(final @Nullable String headerValue) {
return Baggage.fromHeader(
headerValue, false, HubAdapter.getInstance().getOptions().getLogger());
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ public void flush(long timeoutMillis) {
SentryLevel.WARNING, "Instance is disabled and this 'traceHeaders' call is a no-op.");
} else {
final ISpan span = stack.peek().getScope().getSpan();
if (span != null) {
if (span != null && !span.isNoOp()) {
traceHeader = span.toSentryTrace();
}
}
Expand Down
8 changes: 8 additions & 0 deletions sentry/src/main/java/io/sentry/ISpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,12 @@ ISpan startChild(
* @param unit the unit the value is measured in
*/
void setMeasurement(@NotNull String name, @NotNull Number value, @NotNull MeasurementUnit unit);

/**
* Whether this span instance is a NOOP that doesn't collect information
*
* @return true if NOOP
*/
@ApiStatus.Internal
boolean isNoOp();
}
Loading