Skip to content

Fix tracing scope being lost after it closes (LogContextScopeManager) #19008

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Fix tracing scope being lost after it closes (LogContextScopeManager) #19008

wants to merge 4 commits into from
+168 −3

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Oct 1, 2025 •
edited
Loading

Fix tracing scope being lost after it closes in our custom LogContextScopeManager

This test is originally from #18849 spawning from #18804 (comment)

Todo

  • Remove asdf debug logs
  • Figure out how best to clear the logcontext after the span is finished. Do we even care about that?

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

@MadLittleMods MadLittleMods changed the title Fix tracing scope being lost after it closes Fix tracing scope being lost after it closes (LogContextScopeManager) Oct 1, 2025
@MadLittleMods MadLittleMods mentioned this pull request Oct 1, 2025
Merged
3 tasks
MadLittleMods
MadLittleMods commented Oct 2, 2025
View reviewed changes
@MadLittleMods MadLittleMods added the A-Tracing OpenTracing (maybe OpenTelemetry in the future) label Oct 2, 2025
Base automatically changed from madlittlemods/revert-opentracing-context-vars-scope-manager to develop October 2, 2025 16:27
@MadLittleMods
Fix logcontext problems in `test_run_in_background_active_scope_still…
70d0fd3 
…_available`

This test is still expected to fail with our current `LogContextScopeManager`
implementation but there were some flaws in how we were handling
the logcontext before (like pumping the reactor in a non-sentinel logcontext)

This test is originally from
#18849
spawning from #18804 (comment)
@MadLittleMods
Onyl clear logcontext if spans should finish
4e06a50
@MadLittleMods MadLittleMods force-pushed the madlittlemods/log-context-tracing-scope-still-available-after-close branch from 658ca51 to 4e06a50 Compare October 2, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Tracing OpenTracing (maybe OpenTelemetry in the future)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@MadLittleMods