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

POTEL 11 - Move sampling logic into OTel Sampler #3462

Merged
merged 65 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
305baf5
replace hub with scopes
adinauer Mar 27, 2024
95f5e1b
Add Scopes
adinauer Apr 2, 2024
27f2398
Introduce `IScopes` interface.
adinauer Apr 2, 2024
ce3c14f
Replace `IHub` with `IScopes` in core
adinauer Apr 2, 2024
ce615f4
Replace `IHub` with `IScopes` in android core
adinauer Apr 2, 2024
22ddc00
Replace `IHub` with `IScopes` in android integrations
adinauer Apr 2, 2024
305c217
Replace `IHub` with `IScopes` in apollo integrations
adinauer Apr 2, 2024
da927bc
Replace `IHub` with `IScopes` in okhttp integration
adinauer Apr 2, 2024
8279276
Replace `IHub` with `IScopes` in graphql integration
adinauer Apr 2, 2024
9bfc086
Replace `IHub` with `IScopes` in logging integrations
adinauer Apr 2, 2024
b998e50
Replace `IHub` with `IScopes` in more integrations
adinauer Apr 2, 2024
739827a
Replace `IHub` with `IScopes` in OTel integration
adinauer Apr 2, 2024
69f2d63
Replace `IHub` with `IScopes` in Spring 5 / Spring Boot 2 integrations
adinauer Apr 2, 2024
792d482
Replace `IHub` with `IScopes` in Spring 6 / Spring Boot 3 integrations
adinauer Apr 2, 2024
9bcbce6
Replace `IHub` with `IScopes` in samples
adinauer Apr 2, 2024
3f25a4b
Merge branch 'feat/hsm-13-replacements-in-samples' into feat/hubs-sco…
adinauer Apr 2, 2024
d6fb40a
gitscopes -> github
adinauer Apr 2, 2024
7752bcc
Replace ThreadLocal with ScopesStorage
adinauer Apr 4, 2024
1e329c5
Move client and throwable to span map to scope
adinauer Apr 4, 2024
b0d89ae
Add global scope
adinauer Apr 4, 2024
cdd414a
use global scope in Scopes
adinauer Apr 4, 2024
98da9ff
Implement pushScope popScope and withScope for Scopes
adinauer Apr 4, 2024
2d26033
Add pushIsolationScope; add fork methods to ISCope
adinauer Apr 12, 2024
bbb6700
Use separate scopes for current, isolation and global scope; rename m…
adinauer Apr 12, 2024
c714b21
Allow controlling which scope configureScope uses
adinauer Apr 12, 2024
a474402
Combine scopes
adinauer Apr 12, 2024
ae93e33
Use new API for CRONS integrations
adinauer Apr 12, 2024
b01298b
Add lifecycle helper
adinauer Apr 12, 2024
b64e688
Change spring integrations to use new API
adinauer Apr 12, 2024
d06fc50
Use new API in servlet integrations
adinauer Apr 12, 2024
f0af5c3
Use new API for kotlin coroutines and wrapers for Supplier/Callable
adinauer Apr 12, 2024
2f02001
Discussion TODOs
adinauer Apr 12, 2024
bf4a7bf
Fix breadcrumb ordering
adinauer Apr 15, 2024
62cb91a
Mark TODOS with [HSM]
adinauer Apr 15, 2024
b1630ea
Add getGlobalScope and forkedRootScopes to IScopes
adinauer Apr 16, 2024
136b9ce
Fix EventProcessor ordering on scopes
adinauer Apr 16, 2024
94d54ef
Reuse code in Scopes
adinauer Apr 16, 2024
017599d
No longer replace global scope
adinauer Apr 16, 2024
f4c2b3c
Replace hub occurrences in comments, var names etc.
adinauer Apr 16, 2024
61c9d4a
Implement ScopesTest
adinauer Apr 18, 2024
04f3892
Implement CombinedScopeViewTest
adinauer Apr 18, 2024
840c194
Fix combined contexts
adinauer Apr 19, 2024
ab1c3a6
Use combined scopes for cross platform
adinauer Apr 19, 2024
23506c5
Changes according to reviews of previous PRs
adinauer Apr 23, 2024
c9b6f8b
more
adinauer Apr 23, 2024
696a809
even more
adinauer Apr 23, 2024
847200d
isEnabled checks client instead of having a property on Scopes
adinauer Apr 24, 2024
8e86d3b
Use SentryOptions.empty
adinauer Apr 25, 2024
06db228
Remove Hub
adinauer Apr 25, 2024
37ab4d0
Use OpenTelemetry for Performance and Scopes propagation
adinauer Apr 30, 2024
faef2f8
Promote certain span attributes
adinauer May 2, 2024
c57b2d3
Merge branch '8.x.x' into feat/potel-1-context-forking-and-basics
adinauer May 3, 2024
5b15128
Merge branch '8.x.x' into feat/potel-1-context-forking-and-basics
adinauer May 7, 2024
9d48538
Merge branch 'feat/potel-1-context-forking-and-basics' into feat/pote…
adinauer May 7, 2024
9b900c7
Use OTel in Sentry API
adinauer May 13, 2024
b3919bd
Deduplicate SpanInfo extraction
adinauer May 15, 2024
36ed84a
Forward Sentry API to Sentry through OTel
adinauer May 22, 2024
8f56e3f
Use OTel status for Sentry span API
adinauer May 23, 2024
81982bc
POTel Tracing
adinauer May 28, 2024
fba451c
fix root span detection (remote flag), and scope closing
adinauer May 29, 2024
c787e91
Inherit OTel span IDs when sending to sentry
adinauer May 31, 2024
8c25d81
Fix tracing; parse incoming baggage; add baggage to outgoing
adinauer Jun 3, 2024
ffe1bef
Cleanup
adinauer Jun 5, 2024
a31fc08
Move sampling logic to OTel Sampler
adinauer Jun 6, 2024
ee540fd
Merge branch '8.x.x' into feat/potel-11-otel-sampler
adinauer Jun 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ private SdkTracerProviderBuilder configureSdkTracerProvider(
// TODO [POTEL] configurable or separate packages for old vs new way
// return tracerProvider.addSpanProcessor(new SentrySpanProcessor());
return tracerProvider
.setSampler(new SentrySampler())
.addSpanProcessor(new PotelSentrySpanProcessor())
.addSpanProcessor(BatchSpanProcessor.builder(new SentrySpanExporter()).build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ public final class io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor
public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
}

public final class io/sentry/opentelemetry/OtelSamplingUtil {
public fun <init> ()V
public static fun extractSamplingDecision (Lio/opentelemetry/api/common/Attributes;)Lio/sentry/TracesSamplingDecision;
public static fun extractSamplingDecisionOrDefault (Lio/opentelemetry/api/common/Attributes;)Lio/sentry/TracesSamplingDecision;
}

public final class io/sentry/opentelemetry/OtelSpanInfo {
public fun <init> (Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/util/Map;)V
Expand Down Expand Up @@ -36,6 +42,20 @@ public final class io/sentry/opentelemetry/SentryPropagator : io/opentelemetry/c
public fun inject (Lio/opentelemetry/context/Context;Ljava/lang/Object;Lio/opentelemetry/context/propagation/TextMapSetter;)V
}

public final class io/sentry/opentelemetry/SentrySampler : io/opentelemetry/sdk/trace/samplers/Sampler {
public fun <init> ()V
public fun <init> (Lio/sentry/IScopes;)V
public fun getDescription ()Ljava/lang/String;
public fun shouldSample (Lio/opentelemetry/context/Context;Ljava/lang/String;Ljava/lang/String;Lio/opentelemetry/api/trace/SpanKind;Lio/opentelemetry/api/common/Attributes;Ljava/util/List;)Lio/opentelemetry/sdk/trace/samplers/SamplingResult;
}

public final class io/sentry/opentelemetry/SentrySamplingResult : io/opentelemetry/sdk/trace/samplers/SamplingResult {
public fun <init> (Lio/sentry/TracesSamplingDecision;)V
public fun getAttributes ()Lio/opentelemetry/api/common/Attributes;
public fun getDecision ()Lio/opentelemetry/sdk/trace/samplers/SamplingDecision;
public fun getSentryDecision ()Lio/sentry/TracesSamplingDecision;
}

public final class io/sentry/opentelemetry/SentrySpanExporter : io/opentelemetry/sdk/trace/export/SpanExporter {
public fun <init> ()V
public fun <init> (Lio/sentry/IScopes;)V
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package io.sentry.opentelemetry;

import io.opentelemetry.api.common.Attributes;
import io.sentry.TracesSamplingDecision;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
public final class OtelSamplingUtil {

public static @Nullable TracesSamplingDecision extractSamplingDecisionOrDefault(
adinauer marked this conversation as resolved.
Show resolved Hide resolved
final @NotNull Attributes attributes) {
final @Nullable TracesSamplingDecision decision = extractSamplingDecision(attributes);
if (decision != null) {
return decision;
} else {
return new TracesSamplingDecision(false);
}
}

public static @Nullable TracesSamplingDecision extractSamplingDecision(
final @NotNull Attributes attributes) {
final @Nullable Boolean sampled = attributes.get(InternalSemanticAttributes.SAMPLED);
if (sampled != null) {
final @Nullable Double sampleRate = attributes.get(InternalSemanticAttributes.SAMPLE_RATE);
final @Nullable Boolean profileSampled =
attributes.get(InternalSemanticAttributes.PROFILE_SAMPLED);
final @Nullable Double profileSampleRate =
attributes.get(InternalSemanticAttributes.PROFILE_SAMPLE_RATE);

return new TracesSamplingDecision(
sampled, sampleRate, profileSampled == null ? false : profileSampled, profileSampleRate);
} else {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,14 @@
import io.sentry.Baggage;
import io.sentry.IScopes;
import io.sentry.PropagationContext;
import io.sentry.SamplingContext;
import io.sentry.ScopesAdapter;
import io.sentry.Sentry;
import io.sentry.SentryDate;
import io.sentry.SentryLevel;
import io.sentry.SentryLongDate;
import io.sentry.SentryTraceHeader;
import io.sentry.SpanId;
import io.sentry.TracesSampler;
import io.sentry.TracesSamplingDecision;
import io.sentry.TransactionContext;
import io.sentry.protocol.SentryId;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -30,15 +27,12 @@ public final class PotelSentrySpanProcessor implements SpanProcessor {
private final @NotNull SentryWeakSpanStorage spanStorage = SentryWeakSpanStorage.getInstance();
private final @NotNull IScopes scopes;

private final @NotNull TracesSampler tracesSampler;

public PotelSentrySpanProcessor() {
this(ScopesAdapter.getInstance());
}

PotelSentrySpanProcessor(final @NotNull IScopes scopes) {
this.scopes = scopes;
this.tracesSampler = new TracesSampler(scopes.getOptions());
}

@Override
Expand All @@ -55,10 +49,26 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri

final @Nullable OtelSpanWrapper sentryParentSpan =
spanStorage.getSentrySpan(otelSpan.getParentSpanContext());
@Nullable TracesSamplingDecision samplingDecision = null;
@NotNull
TracesSamplingDecision samplingDecision =
OtelSamplingUtil.extractSamplingDecisionOrDefault(otelSpan.toSpanData().getAttributes());
@Nullable Baggage baggage = null;
otelSpan.setAttribute(IS_REMOTE_PARENT, otelSpan.getParentSpanContext().isRemote());
if (sentryParentSpan == null) {
final @NotNull String traceId = otelSpan.getSpanContext().getTraceId();
final @NotNull String spanId = otelSpan.getSpanContext().getSpanId();
final @NotNull SpanId sentrySpanId = new SpanId(spanId);
final @NotNull String parentSpanId = otelSpan.getParentSpanContext().getSpanId();
final @Nullable SpanId sentryParentSpanId =
io.opentelemetry.api.trace.SpanId.isValid(parentSpanId) ? new SpanId(parentSpanId) : null;

@Nullable
SentryTraceHeader sentryTraceHeader = parentContext.get(SentryOtelKeys.SENTRY_TRACE_KEY);
@Nullable Baggage baggageFromContext = parentContext.get(SentryOtelKeys.SENTRY_BAGGAGE_KEY);
if (sentryTraceHeader != null) {
baggage = baggageFromContext;
}

final @Nullable Boolean baggageMutable =
otelSpan.getAttribute(InternalSemanticAttributes.BAGGAGE_MUTABLE);
final @Nullable String baggageString =
Expand All @@ -69,77 +79,20 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri
baggage.freeze();
}
}
final @Nullable Boolean sampled = otelSpan.getAttribute(InternalSemanticAttributes.SAMPLED);
final @Nullable Double sampleRate =
otelSpan.getAttribute(InternalSemanticAttributes.SAMPLE_RATE);
final @Nullable Boolean profileSampled =
otelSpan.getAttribute(InternalSemanticAttributes.PROFILE_SAMPLED);
final @Nullable Double profileSampleRate =
otelSpan.getAttribute(InternalSemanticAttributes.PROFILE_SAMPLE_RATE);
if (sampled != null) {
// span created by Sentry API

final @NotNull String traceId = otelSpan.getSpanContext().getTraceId();
final @NotNull String spanId = otelSpan.getSpanContext().getSpanId();
// TODO [POTEL] parent span id could be invalid
final @NotNull String parentSpanId = otelSpan.getParentSpanContext().getSpanId();

final @NotNull PropagationContext propagationContext =
new PropagationContext(
new SentryId(traceId),
new SpanId(spanId),
new SpanId(parentSpanId),
baggage,
sampled);

scopes.configureScope(
scope -> {
scope.withPropagationContext(
oldPropagationContext -> {
scope.setPropagationContext(propagationContext);
});
});

// TODO [POTEL] can we use OTel Sampler to let OTel know our sampling decision
// Sentry not sampled vs OTel not sampled may mean different things for trace propagation
samplingDecision =
new TracesSamplingDecision(
sampled,
sampleRate,
profileSampled == null ? false : profileSampled,
profileSampleRate);
} else {
// span not created by Sentry API

final @NotNull String traceId = otelSpan.getSpanContext().getTraceId();
final @NotNull String spanId = otelSpan.getSpanContext().getSpanId();
final @NotNull SpanId sentrySpanId = new SpanId(spanId);

@Nullable
SentryTraceHeader sentryTraceHeader = parentContext.get(SentryOtelKeys.SENTRY_TRACE_KEY);
@Nullable Baggage baggageFromContext = parentContext.get(SentryOtelKeys.SENTRY_BAGGAGE_KEY);
if (sentryTraceHeader != null) {
baggage = baggageFromContext;
}

final @NotNull PropagationContext propagationContext =
sentryTraceHeader == null
? new PropagationContext(new SentryId(traceId), sentrySpanId, null, baggage, null)
: PropagationContext.fromHeaders(sentryTraceHeader, baggage, sentrySpanId);

scopes.configureScope(
scope -> {
scope.withPropagationContext(
oldPropagationContext -> {
scope.setPropagationContext(propagationContext);
});
});

final @NotNull TransactionContext transactionContext =
TransactionContext.fromPropagationContext(propagationContext);
samplingDecision = tracesSampler.sample(new SamplingContext(transactionContext, null));
}
// TODO [POTEL] what do we use as fallback here? could happen if misconfigured (i.e. sampler
// not in place)
final boolean sampled = samplingDecision != null ? samplingDecision.getSampled() : true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, should we use otelSpan.getSpanContext().isSampled() as fallback?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm not sure


final @NotNull PropagationContext propagationContext =
sentryTraceHeader == null
? new PropagationContext(
new SentryId(traceId), sentrySpanId, sentryParentSpanId, baggage, sampled)
: PropagationContext.fromHeaders(sentryTraceHeader, baggage, sentrySpanId);

updatePropagationContext(scopes, propagationContext);
}

final @NotNull SpanContext spanContext = otelSpan.getSpanContext();
final @NotNull SentryDate startTimestamp =
new SentryLongDate(otelSpan.toSpanData().getStartEpochNanos());
Expand All @@ -149,6 +102,17 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri
otelSpan, scopes, startTimestamp, samplingDecision, sentryParentSpan, baggage));
}

private static void updatePropagationContext(
IScopes scopes, PropagationContext propagationContext) {
scopes.configureScope(
scope -> {
scope.withPropagationContext(
oldPropagationContext -> {
scope.setPropagationContext(propagationContext);
});
});
}

@Override
public boolean isStartRequired() {
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package io.sentry.opentelemetry;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.sdk.trace.data.LinkData;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import io.sentry.Baggage;
import io.sentry.IScopes;
import io.sentry.PropagationContext;
import io.sentry.SamplingContext;
import io.sentry.ScopesAdapter;
import io.sentry.SentryTraceHeader;
import io.sentry.SpanId;
import io.sentry.TracesSampler;
import io.sentry.TracesSamplingDecision;
import io.sentry.TransactionContext;
import io.sentry.protocol.SentryId;
import java.util.List;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public final class SentrySampler implements Sampler {

private final @NotNull SentryWeakSpanStorage spanStorage = SentryWeakSpanStorage.getInstance();
private final @NotNull TracesSampler tracesSampler;

public SentrySampler(final @NotNull IScopes scopes) {
this.tracesSampler = new TracesSampler(scopes.getOptions());
}

public SentrySampler() {
this(ScopesAdapter.getInstance());
}

@Override
public SamplingResult shouldSample(
final @NotNull Context parentContext,
final @NotNull String traceId,
final @NotNull String name,
final @NotNull SpanKind spanKind,
final @NotNull Attributes attributes,
final @NotNull List<LinkData> parentLinks) {
// note: parentLinks seems to usually be empty
final @Nullable Span parentOtelSpan = Span.fromContextOrNull(parentContext);
final @Nullable OtelSpanWrapper parentSentrySpan =
parentOtelSpan != null ? spanStorage.getSentrySpan(parentOtelSpan.getSpanContext()) : null;

if (parentSentrySpan != null) {
return copyParentSentryDecision(parentSentrySpan);
} else {
final @Nullable TracesSamplingDecision samplingDecision =
OtelSamplingUtil.extractSamplingDecision(attributes);
if (samplingDecision != null) {
return new SentrySamplingResult(samplingDecision);
} else {
return handleRootOtelSpan(traceId, parentContext);
}
}
}

private @NotNull SentrySamplingResult handleRootOtelSpan(
adinauer marked this conversation as resolved.
Show resolved Hide resolved
final @NotNull String traceId, final @NotNull Context parentContext) {
@Nullable Baggage baggage = null;
@Nullable
SentryTraceHeader sentryTraceHeader = parentContext.get(SentryOtelKeys.SENTRY_TRACE_KEY);
@Nullable Baggage baggageFromContext = parentContext.get(SentryOtelKeys.SENTRY_BAGGAGE_KEY);
if (sentryTraceHeader != null) {
baggage = baggageFromContext;
}

// there's no way to get the span id here, so we just use a random id for sampling
SpanId randomSpanId = new SpanId();
final @NotNull PropagationContext propagationContext =
sentryTraceHeader == null
? new PropagationContext(new SentryId(traceId), randomSpanId, null, baggage, null)
: PropagationContext.fromHeaders(sentryTraceHeader, baggage, randomSpanId);

final @NotNull TransactionContext transactionContext =
TransactionContext.fromPropagationContext(propagationContext);
final @NotNull TracesSamplingDecision sentryDecision =
tracesSampler.sample(new SamplingContext(transactionContext, null));
return new SentrySamplingResult(sentryDecision);
}

private @NotNull SentrySamplingResult copyParentSentryDecision(
final @NotNull OtelSpanWrapper parentSentrySpan) {
final @Nullable TracesSamplingDecision parentSamplingDecision =
parentSentrySpan.getSamplingDecision();
if (parentSamplingDecision != null) {
return new SentrySamplingResult(parentSamplingDecision);
} else {
// this should never happen and only serve to calm the compiler
// TODO [POTEL] log
return new SentrySamplingResult(new TracesSamplingDecision(true));
}
}

@Override
public String getDescription() {
return "SentrySampler";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package io.sentry.opentelemetry;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import io.sentry.TracesSamplingDecision;
import org.jetbrains.annotations.NotNull;

public final class SentrySamplingResult implements SamplingResult {
private final TracesSamplingDecision sentryDecision;

public SentrySamplingResult(final @NotNull TracesSamplingDecision sentryDecision) {
this.sentryDecision = sentryDecision;
}

@Override
public SamplingDecision getDecision() {
if (sentryDecision.getSampled()) {
return SamplingDecision.RECORD_AND_SAMPLE;
} else {
return SamplingDecision.RECORD_ONLY;
}
}

@Override
public Attributes getAttributes() {
return Attributes.builder()
.put(InternalSemanticAttributes.SAMPLED, sentryDecision.getSampled())
.put(InternalSemanticAttributes.SAMPLE_RATE, sentryDecision.getSampleRate())
.put(InternalSemanticAttributes.PROFILE_SAMPLED, sentryDecision.getProfileSampled())
.put(InternalSemanticAttributes.PROFILE_SAMPLE_RATE, sentryDecision.getProfileSampleRate())
.build();
}

public TracesSamplingDecision getSentryDecision() {
return sentryDecision;
}
}
Loading