Skip to content

fix: don't adopt a foreign global OTel span as a Judgment parent - #769

Open
arthi-arumugam-git wants to merge 2 commits into
JudgmentLabs:mainfrom
arthi-arumugam-git:fix/no-foreign-global-parent
Open

fix: don't adopt a foreign global OTel span as a Judgment parent#769
arthi-arumugam-git wants to merge 2 commits into
JudgmentLabs:mainfrom
arthi-arumugam-git:fix/no-foreign-global-parent

Conversation

@arthi-arumugam-git

@arthi-arumugam-git arthi-arumugam-git commented Jul 27, 2026

Copy link
Copy Markdown

Picks up the regression from #749 that #751 and #752 were both aimed at. #752 has been open since 6 July, so this is an independent take on it with a failing test.

The problem

#749 made get_current_context() read the global OTel context whenever this provider owns the global provider. That folded two different jobs into one method:

  • write: publish the active Judgment span into the global context, so third-party instrumentation calling trace.get_current_span() can see it
  • read, decide what a newly started Judgment span should be parented to

Only the write direction actually needs to be global. Reading parenting from the global context means whatever span a host application or unrelated instrumentation happens to have left current becomes the parent of the next Judgment span, which silently reroots it onto a foreign trace. Nothing errors, and the trace just shows up in the wrong tree with the wrong trace_id.

The change

Split the two directions.

  • get_current_context() always reads Judgment's private runtime context. Parenting never consults the global context.
  • get_global_context() added for the cases that genuinely want the global view.
  • attach_context() attaches to the private context and, when we own the global provider, mirrors the same context into the global one. It returns both tokens in a _ContextToken so detach_context() can unwind both, global first.

The write path is unchanged in effect, so the interop #749 was after still works.

Tests

TestForeignGlobalParent adds two.

test_foreign_global_span_is_not_adopted_as_parent fails on main:

> assert span.parent is None
E assert SpanContext(trace_id=0x56a27dc4..., span_id=0x2832975e..., ...) is None

A foreign SDK span is made current in the global context, a Judgment root span is started, and on main that root comes back with a parent and the foreign trace's trace_id.

test_judgment_children_still_parent_to_judgment_spans is the guard in the other direction, with the same foreign span current, a nested Judgment span still parents to its Judgment parent and stays on the same trace.

The three existing TestGlobalContextBridge tests pass unchanged, including test_active_span_visible_to_external_when_global, which is the behaviour #749 was protecting. Full file is 19 passed.

Happy to adjust naming or fold this into #752 instead if you'd rather keep that one, I mostly wanted the failing test to exist somewhere.


Open in Devin Review

JudgmentLabs#749 made get_current_context() read the global OTel context whenever this
provider owns the global provider. That conflated two directions:

  - write: publish the active Judgment span to the global context so
    third-party instrumentation reading trace.get_current_span() sees it
  - read: decide what a new Judgment span should be parented to

Only the write direction needs to be global. Reading parenting from the
global context means any span a host application or unrelated instrumentation
left current becomes the parent of the next Judgment span, which silently
reroots it onto a foreign trace.

Split them. get_current_context() now always reads Judgment's private runtime
context. attach_context() writes to that context and, when we own the global
provider, mirrors into the global context as well, returning both tokens so
detach unwinds both.

Adds a test that fails on main: with a foreign span current in the global
context, a Judgment root span comes back with a parent and a trace_id
belonging to the foreign trace.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@arthi-arumugam-git

Copy link
Copy Markdown
Author

Gentle ping: this has been unchanged and ready for review for 8 days. Anything needed from my side to move it forward?

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.

1 participant