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

rfc(feature): Continue trace over process boundaries #71

Merged
merged 11 commits into from
Feb 8, 2023

Conversation

antonpirker
Copy link
Member

@antonpirker antonpirker commented Feb 1, 2023

To make it possible to continue a trace over process boundaries (think of one program starting another one) For this to work we will use environment variables as the carrier.

Rendered RFC

@antonpirker antonpirker force-pushed the rfc/continue-trace-over-process-boundaries branch from 627e00f to 688a80b Compare February 1, 2023 11:28
- `SENTRY_TRACING_BAGGAGE` (similar to `baggage` HTTP header)
- `SENTRY_TRACING_SENTRY_TRACE` (similar to `sentry-trace` HTTP header)

The environment variables contain the same strings that the respecitve HTTP headers would contain.
Copy link

Choose a reason for hiding this comment

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

We should also define what happens when multiple mechanisms are enabled, e.g. when SENTRY_TRACING_USE_ENVIRONMENT is set to true, but also we have incoming HTTP/other requests with different sentry-trace values.
It raises some questions: for example, should the trace-id from SENTRY_TRACING_SENTRY_TRACE "override" all other trace-ids, received via HTTP? Probably not.

Or do we want to limit this RFC to purely non-server environments?

Copy link
Member

Choose a reason for hiding this comment

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

There can only ever be once trace so when an incoming HTTP request is coming in, it starts a new trace in a new hub, shadowing the process one. We do not have a trace to trace relationship so they will be disconnected which is reasonable.

@evanpurkhiser
Copy link
Member

This would allow errors to be associated to trace IDs as well correct? That's probably the more immediate case for the Cron monitoring feature, is being able to trace errors in sentry to specific monitor checkin failures.

Copy link
Member

@mitsuhiko mitsuhiko left a comment

Choose a reason for hiding this comment

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

I believe a non covered omission here is if we want a flag on a transaction that says "do not emit me" but do consider me sampled (to avoid charging customers unnecessary transactions). I don't think it should be part of the RFC, but it should me mentioned in a section that refers to out of scope discussions.

text/0071-continue-trace-over-process-boundaries.md Outdated Show resolved Hide resolved
- `SENTRY_TRACING_BAGGAGE` (similar to `baggage` HTTP header)
- `SENTRY_TRACING_SENTRY_TRACE` (similar to `sentry-trace` HTTP header)

The environment variables contain the same strings that the respecitve HTTP headers would contain.
Copy link
Member

Choose a reason for hiding this comment

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

There can only ever be once trace so when an incoming HTTP request is coming in, it starts a new trace in a new hub, shadowing the process one. We do not have a trace to trace relationship so they will be disconnected which is reasonable.

@antonpirker
Copy link
Member Author

This would allow errors to be associated to trace IDs as well correct? That's probably the more immediate case for the Cron monitoring feature, is being able to trace errors in sentry to specific monitor checkin failures.

Yes, this is possible @evanpurkhiser

@antonpirker antonpirker marked this pull request as ready for review February 8, 2023 17:16
@antonpirker antonpirker merged commit b1ce44d into main Feb 8, 2023
@antonpirker antonpirker deleted the rfc/continue-trace-over-process-boundaries branch February 8, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants