Skip to content

feat(langgraph): Gate prompt/response collection on data_collection option - #7175

Open
ericapisani wants to merge 4 commits into
masterfrom
py-2588-langgraph
Open

feat(langgraph): Gate prompt/response collection on data_collection option#7175
ericapisani wants to merge 4 commits into
masterfrom
py-2588-langgraph

Conversation

@ericapisani

@ericapisani ericapisani commented Aug 11, 2026

Copy link
Copy Markdown
Member

Modify the LangGraph integration to respect the data_collection config for controlling whether prompts, responses, tool calls, and available tools are captured in spans.

When data collection is enabled, the gen_ai.inputs flag controls request messages, tool calls, and available tools, while gen_ai.outputs controls the response text. Tool calls are gated on inputs because they are fed back to the model as input. Available tools are only gated once data collection is configured, since they were never gated on the legacy PII settings.

When data collection is not configured, falls back to legacy send_default_pii and include_prompts settings for compatibility.

Refs #6748
Refs #6748

…ption

Modify the LangGraph integration to respect the data_collection config for
controlling whether prompts, responses, tool calls, and available tools are
captured in spans.

When data collection is enabled, the gen_ai.inputs flag controls request
messages, tool calls, and available tools, while gen_ai.outputs controls the
response text. Tool calls are gated on inputs because they are fed back to
the model as input. Available tools are only gated once data collection is
configured, since they were never gated on the legacy PII settings.

When data collection is not configured, falls back to legacy send_default_pii
and include_prompts settings for compatibility.

Refs PY-2588
Refs #6748
@ericapisani
ericapisani requested a review from a team as a code owner August 11, 2026 20:24
@linear-code

linear-code Bot commented Aug 11, 2026

Copy link
Copy Markdown

PY-2588

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f54e39e. Configure here.

Comment thread sentry_sdk/integrations/langgraph.py Outdated
Rename the data collection tests and the invoke span helper so the names state what is being verified, and drop the now-redundant docstrings.
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

112702 passed | ❌ 1 failed | ⏭️ 6759 skipped | Total: 119462 | Pass Rate: 94.34% | Execution Time: 412m 32s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +240
Passed Tests 📈 +239
Failed Tests 📈 +1
Skipped Tests

➕ New Tests (1)

View new tests
  • test_continuous_profiler_auto_start_and_stop_sampled[experiment-thread]
    • File: tests.profiler.test_continuous_profiler
    • Status: ❌ Failing

❌ Failed Tests

test_continuous_profiler_auto_start_and_stop_sampled[experiment-thread]

File: tests.profiler.test_continuous_profiler
Suite: py3.7-common
Error: AssertionError: profiler should not be running assert '1e05d589796041b198d7e955af8c9bad' is None + where '1e05d589796041b198d7e955af8c9bad' = get_profiler_id()

Stack Trace
tests/profiler/test_continuous_profiler.py:812: in test_continuous_profiler_auto_start_and_stop_sampled
    assert get_profiler_id() is None, "profiler should not be running"
E   AssertionError: profiler should not be running
E   assert '1e05d589796041b198d7e955af8c9bad' is None
E    +  where '1e05d589796041b198d7e955af8c9bad' = get_profiler_id()

✅ Patch coverage is 98.00%. Project has 2488 uncovered lines.
❌ Project coverage is 90.19%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
sentry_sdk/integrations/langgraph.py 98.00% ⚠️ 1 Missing and 9 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    90.21%    90.19%    -0.02%
==========================================
  Files          193       193         —
  Lines        25361     25374       +13
  Branches      9302      9320       +18
==========================================
+ Hits         22880     22886        +6
- Misses        2481      2488        +7
- Partials      1436      1439        +3

Generated by Codecov Action

Comment thread tests/integrations/langgraph/test_langgraph.py
Comment thread sentry_sdk/integrations/langgraph.py
@alexander-alderman-webb

Copy link
Copy Markdown
Contributor

This integration sucks 😞 .
Could you take a look at the bot comments @ericapisani 🙏

@ericapisani

Copy link
Copy Markdown
Member Author

@alexander-alderman-webb Got it sorted - the changes I had introduced broke the message deltas. Have moved where the input gate is so it doesn't affect that and added regression tests for this scenario 🚀

Comment thread sentry_sdk/integrations/langgraph.py Outdated

@sentrivana sentrivana 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.

Looks good, left some suggestions

SPANDATA.GEN_AI_REQUEST_MESSAGES,
messages_data,
unpack=False,
if _should_record_inputs(integration):

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.

We could move this check above, to line 215, and combine it with the len(args) check so that we only do the input messages manipulation if necessary. Cheap way to save some cycles and remove one level of nesting.

SPANDATA.GEN_AI_REQUEST_MESSAGES,
messages_data,
unpack=False,
if _should_record_inputs(integration):

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.

Same here, we could combine this with the args check on L258.

SPANDATA.GEN_AI_REQUEST_MESSAGES,
messages_data,
unpack=False,
if _should_record_inputs(integration):

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.

And here

SPANDATA.GEN_AI_REQUEST_MESSAGES,
messages_data,
unpack=False,
if _should_record_inputs(integration):

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.

And here (goodness this integration is a mess)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

goodness this integration is a mess

Yeah, it needs a good overhaul 😭 😅

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