-
Notifications
You must be signed in to change notification settings - Fork 904
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
common: Add a debug mode for tracing #7850
Merged
rustyrussell
merged 7 commits into
ElementsProject:master
from
cdecker:202447-trace-debug
Nov 23, 2024
Merged
common: Add a debug mode for tracing #7850
rustyrussell
merged 7 commits into
ElementsProject:master
from
cdecker:202447-trace-debug
Nov 23, 2024
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
cdecker
force-pushed
the
202447-trace-debug
branch
2 times, most recently
from
November 22, 2024 12:33
d4ce0f9
to
c3e2e98
Compare
cdecker
changed the title
Draft: common: Add a debug mode for tracing
common: Add a debug mode for tracing
Nov 22, 2024
After adding the DB query instrumentation we ran into a couple of issues, with spans not being resumed correctly, and it was rather hard to identify the problem. This adds debug statements so we can trace the tracing (traception if you will). Changelog-None
This was a bit harder to identify: during an `io_loop` run we suspend the current span before handing over to `io_loop`, and later when a callback is called we resume the span again. Depending on how we return from the `io_loop` instance that is used to drive the startup, we either have resumed the last span, or we don't. Since we start a span before `io_loop` and want it to be emitted afterwards, we need to take care of the case where we returned from a callback that did not resume, and therefore the current context is empty. Making `trace_span_resume` idempotent means we can just resume it manually. Ideally we'd push the suspend / resume logic down into `io_loop` itself, and then we'd have just one place. Maybe suspend and resume callbacks that can be configured in `io_loop`?
We have the space in memory set aside anyway, so let's just copy the `trace_id` into the span itself, rather than resolving the `root` at time of emission.
Trace spans form a tree, but we don't actually check that the structure doesn't break. Breakage can for example come if we use the same key accidentally, making a new span its own ancestor.
It turns out that under some circumstances we end up clearing the pointee of `current` but not the pointer. Thus when we select the next slot we can end up reusing the same slot, making it its own parent. We forcefull break these cycles by enforcing that `current` should never be returned and be set as its own parent. Changelog-None
cdecker
force-pushed
the
202447-trace-debug
branch
from
November 22, 2024 13:34
c3e2e98
to
795e4b2
Compare
Just added a couple of compile-time guards and sprinkled the invariant checking in a couple of places (disabled if compile time guard is unset).
cdecker
force-pushed
the
202447-trace-debug
branch
from
November 22, 2024 13:59
795e4b2
to
cc7a826
Compare
endothermicdev
approved these changes
Nov 23, 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.
ACK cc7a826
rustyrussell
approved these changes
Nov 23, 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.
Ack cc7a826
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.
Replaces an earlier attempt in #7847
Closes #7843