Skip to content

Conversation

@constantinius
Copy link
Contributor

Description

chat and execute tool spans under an invoke agent span should have the agent name property set

Issues

Closes https://linear.app/getsentry/issue/TET-1293/make-sure-that-agent-name-is-set-on-all-of-its-gen-ai-children

@constantinius constantinius requested a review from a team as a code owner October 24, 2025 13:44
@linear
Copy link

linear bot commented Oct 24, 2025

@constantinius constantinius requested a review from a team October 24, 2025 13:45
cursor[bot]

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Oct 24, 2025

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
27744 2 27742 2207
View the top 2 failed test(s) by shortest run time
tests.tracing.test_decorator::test_trace_decorator_async_no_trx
Stack Traces | 0.038s run time
tests/tracing/test_decorator.py:81: in test_trace_decorator_async_no_trx
    fake_debug.assert_called_once_with(
.../hostedtoolcache/Python/3.14.0.../x64/lib/python3.14/unittest/mock.py:996: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'mock' to be called once. Called 5 times.
E   Calls: [call('[Monitor] health check negative, downsampling with a factor of %d', 10),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'time.sleep'),
E    call('[Monitor] health check negative, downsampling with a factor of %d', 10),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'time.sleep'),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'test_decorator.my_async_example_function')].
tests.tracing.test_decorator::test_trace_decorator_async_no_trx
Stack Traces | 0.046s run time
tests/tracing/test_decorator.py:81: in test_trace_decorator_async_no_trx
    fake_debug.assert_called_once_with(
.../hostedtoolcache/Python/3.14.0.../x64-freethreaded/lib/python3.14t/unittest/mock.py:996: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'mock' to be called once. Called 5 times.
E   Calls: [call('[Monitor] health check negative, downsampling with a factor of %d', 10),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'time.sleep'),
E    call('[Monitor] health check negative, downsampling with a factor of %d', 10),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'time.sleep'),
E    call('Cannot create a child span for %s. Please start a Sentry transaction before calling this function.', 'test_decorator.my_async_example_function')].

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

sentrivana added a commit that referenced this pull request Oct 29, 2025
### Description
Cherry-picked off of
#5030

#### Issues
Ref
https://linear.app/getsentry/issue/TET-1293/make-sure-that-agent-name-is-set-on-all-of-its-gen-ai-children

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)

---------

Co-authored-by: Fabian Schindler <[email protected]>
Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

I took the google-genai part out of this PR and merged it separately since it was straightforward: #5038

Regarding the Langchain part, we talked about this and I also thought about it a bit more. Documenting my reasoning and the proposed path forward for this sort of pattern here.

The problem: We need access to some package-internal object (in this case the agent) to be able to extract something from it in different places in the integration.

In this PR, we're opting for storing it on the current scope in the _contexts. I'm unhappy about this since we're misusing a field whose purpose is something entirely different (populating contexts on events) to store something integration specific.

Other solutions that have been proposed:

  • Store it on something coming from Langchain directly that gets passed around and is accessible everywhere we need it. This is the usual way of doing things, but we couldn't find a good candidate here.
  • Store it on the parent transaction and access it from there. The problem here is there might be different agents in a transaction.
  • Store it on a parent span and access it from here. But there is no good way of traversing the span tree.

TL;DR: recommended way forward:

My recommended solution would be to store this in a ContextVar on the top level of the integration. That way, we have the same execution context isolation that we have with scopes (they're also context vars), but we're not misusing _contexts, and we're keeping the change contained to the integration.

Alternatively, can this be filled in in relay? I.e., we set the agent name on the topmost span that makes sense, and relay copies it to all child spans where it makes sense.

shellmayr pushed a commit that referenced this pull request Oct 30, 2025
### Description
Cherry-picked off of
#5030

#### Issues
Ref
https://linear.app/getsentry/issue/TET-1293/make-sure-that-agent-name-is-set-on-all-of-its-gen-ai-children

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)

---------

Co-authored-by: Fabian Schindler <[email protected]>
@constantinius
Copy link
Contributor Author

I talked to the people at Relay and it seems to be feasible to put it in the place where the span-buffer is evaluated (not Relay itself).

So, I think this is a viable solution next to the ContextVar approach.

Copy link
Contributor Author

@constantinius constantinius left a comment

Choose a reason for hiding this comment

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

Alright, this is now implemented using ContextVars.

set_data_normalized(span, SPANDATA.GEN_AI_RESPONSE_TEXT, output)

_pop_agent()

Copy link

Choose a reason for hiding this comment

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

Bug: Agent Context Stack: Unbalanced Push/Pop

In _wrap_agent_executor_stream, _push_agent is called immediately at line 868, but _pop_agent is only called inside the iterator functions (lines 919 and 940) which execute lazily when the iterator is consumed. If the returned iterator is never consumed or only partially consumed, the agent name remains on the contextvar stack indefinitely, causing a memory leak and incorrect agent name propagation to unrelated spans in the same async context.

Fix in Cursor Fix in Web

if stack is None:
stack = []
stack.append(agent_name)
_agent_stack.set(stack)
Copy link

Choose a reason for hiding this comment

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

Bug: Mutable Objects Break Context Isolation

_push_agent mutates the list returned by _agent_stack.get() in place without copying it first. Since contextvars share mutable object references between parent and child contexts, modifications to the stack in a child context will incorrectly affect the parent context's stack, breaking context isolation and causing agent names to leak across async contexts.

Fix in Cursor Fix in Web

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.

3 participants