-
Notifications
You must be signed in to change notification settings - Fork 11
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: More granular instrumentation and remove long running spans #811
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Defaults are: ``` sampler: Box::new(Sampler::ParentBased(Box::new(Sampler::AlwaysOn))), id_generator: Box::<RandomIdGenerator>::default(), ```
… handling, and high level route handling This fixes the trace hierarchy which used to be disjoint because the parent span/trace id was missing and a new one was being generated in the axum layers
4 tasks
Techassi
reviewed
Jun 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some questions and nitpicks.
…ss tracing context
…that can easily be triggered by clients
…ifferent. Add links between traces
…ers are about to shut down
… are defined in opentelemetry, not opentelemetry_otlp. Even though we happen to be using otlp providers.
Techassi
requested changes
Jun 21, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some small remarks.
Co-authored-by: Techassi <git@techassi.dev>
Co-authored-by: Techassi <git@techassi.dev>
…Tracing::init method
Techassi
approved these changes
Jun 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Full chain
Note
All spans are in the same trace when the client has not sent a trace ID in the request.
Linked traces
When a client sends trace information, we can pick that up and use the trace context from there. Notice the traceparent send is the same one as what is received. The webhook is using the clients trace context from thereon.
Note
Unfortunately trace ids cannot be updated on spans, so once we are able to extract the trace-id from the headers of a client request, we can only use it from that point forward.
But, we are able to add links to spans to make life easier.
Client makes a request, and injects the trace-id into headers.
A link appears on the span where the parent context was updated.
Clicking on the link will take you to the trace with the spans that existed before the trace-id was extracted from the headers.
This trace also has a span with a link back to the other.
Similarly in Jaeger
Definition of Done Checklist
Reviewer