-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(core): Pass name
& attributes
to tracesSampler
#10426
Conversation
@mydea I will check this in a second. Looks promising. Thank you. However, I suspect, this will be only partial solution to the problem I am seeking to solve. The greater problem is that getNodeAutoInstrumentations({
'@opentelemetry/instrumentation-fastify': {
requestHook: () => {},
},
'@opentelemetry/instrumentation-http': {
requestHook: () => {},
},
}), ... and the data I need to make the sampling decision can only be obtained by referencing So I am struggling to understand what's the correct way to determine sampling when the data for sampling decision only becomes during the transaction and not at the start of the transaction. |
size-limit report 📦
|
Hmm, that's of course a problem - what span is the sampling based off on then, I wonder? sampling runs for the "root" span, so if that does not contain the info you care about, you're out of luck, I fear, with the current model... 🤔 |
Perhaps I am missing something obvious, but why can't sampling decision be made at the end of the request based on attributes contained within spans? |
0f8354b
to
3cec51d
Compare
That's sadly a fundamental change to how sampling works with Sentry - we always do head-based sampling, as of now. This limitation is unlikely to be changed in the near future, although we are always exploring ways to change this potentially. One of the reasons that this becomes tricky is that we want to ensure to inherit the sampling decision for child spans. So if we'd only sample a parent span when it is finished, we could not know and cascade this for child spans that may be finished before that 🤔 |
I realize this is not a quick fix, but could this be achieved by allowing clients to deploy a service that buffers traces before they are sent to Sentry, and allow to decide whether to keep/reject traces inside of that buffer once information is available about the entire trace? Presumably, I should be able to cook this up myself as long as Sentry allows to configure custom destination. |
@mydea Related – how do I know inside of request if the request was sampled or not? I need to generate and inject <meta content="${sentryTrace}" name="sentry-trace" /> I currently do this: import { context, trace } from '@opentelemetry/api';
const activeContext = context.active();
const activeSpan = trace.getSpan(activeContext);
if (!activeSpan) {
return {
sentryTrace: null,
spanId: null,
traceId: null,
};
}
const { spanId, traceId } = activeSpan.spanContext();
const sentryTrace = `${traceId}-${spanId}-0`; but need to figure out how to adjust |
Co-authored-by: Luca Forstner <[email protected]>
11e6edf
to
e352d7d
Compare
Updates
tracesSampler
to receivename
andattributes
instead of relying ontransactionContext
being passed.