Next Python SDK major - #5005
Draft
sentrivana wants to merge 281 commits into
Draft
sentrivana wants to merge 281 commits into
sentrivana wants to merge 281 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5005 +/- ##
===========================================
+ Coverage 70.55% 83.76% +13.21%
===========================================
Files 180 180
Lines 18077 18080 +3
Branches 3008 3009 +1
===========================================
+ Hits 12754 15145 +2391
+ Misses 4432 1943 -2489
- Partials 891 992 +101
|
Closed
Contributor
Codecov Results 📊✅ 64061 passed | ❌ 2 failed | ⏭️ 2899 skipped | Total: 66962 | Pass Rate: 95.67% | Execution Time: 186m 38s 📊 Comparison with Base Branch
➕ New Tests (2)View new tests
❌ Failed Tests
|
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Anthropic
Documentation 📚
Internal Changes 🔧
Other
🤖 This preview updates automatically when you update the PR. |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
### Description The LaunchDarkly client is a [singleton](https://launchdarkly.com/docs/sdk/server-side/python#initialize-the-client). As such, it doesn't make sense to provide a way to patch a specific client only. This allows us to get rid of the integation `__init__`, which is always good: integrations are meant to be setup via `setup_once`. #### Issues Closes #7040 #### Reminders - Please add tests to validate your changes, and lint your code using `uv run ruff`. - 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)
- drop old chalice from ci - bump min version in `integrations/__init__.py`
We're now enforcing Tornado > 6, so compat code for older versions is not needed anymore
Comment on lines
1182
to
1208
| span = sentry_sdk.traces.start_span( | ||
| name=f"invoke_agent {run_name}" if run_name else "invoke_agent", | ||
| attributes={ | ||
| "sentry.op": OP.GEN_AI_INVOKE_AGENT, | ||
| "sentry.origin": LangchainIntegration.origin, | ||
| SPANDATA.GEN_AI_OPERATION_NAME: "invoke_agent", | ||
| SPANDATA.GEN_AI_RESPONSE_STREAMING: True, | ||
| }, | ||
| ) | ||
|
|
||
| if run_name: | ||
| span.set_data(SPANDATA.GEN_AI_FUNCTION_ID, run_name) | ||
| if run_name: | ||
| span.set_attribute(SPANDATA.GEN_AI_FUNCTION_ID, run_name) | ||
|
|
||
| _set_tools_on_span(span, tools) | ||
|
|
||
| input = args[0].get("input") if len(args) >= 1 else None | ||
| if input is not None and record_inputs: | ||
| normalized_messages = normalize_message_roles([input]) | ||
|
|
||
| scope = sentry_sdk.get_current_scope() | ||
| messages_data = ( | ||
| truncate_and_annotate_messages(normalized_messages, span, scope) | ||
| if should_truncate_gen_ai_input(client.options) | ||
| else normalized_messages | ||
| set_data_normalized( | ||
| span, | ||
| SPANDATA.GEN_AI_REQUEST_MESSAGES, | ||
| normalized_messages, | ||
| unpack=False, | ||
| ) | ||
| if messages_data is not None: | ||
| set_data_normalized( | ||
| span, | ||
| SPANDATA.GEN_AI_REQUEST_MESSAGES, | ||
| messages_data, | ||
| unpack=False, | ||
| ) | ||
|
|
||
| # Run the agent | ||
| result = f(self, *args, **kwargs) |
Contributor
There was a problem hiding this comment.
AgentExecutor.stream leaves invoke_agent span active if setup raises
Wrap result = f(self, *args, **kwargs) in try/except and call span.__exit__(*exc_info) on failure so a setup error does not leave the active span on the scope (see google_genai stream wrappers).
Evidence
new_stream()callssentry_sdk.traces.start_span(...), andSpan._start()immediately sets the span as the current scope span whenactive=True.result = f(self, *args, **kwargs)is unguarded; onlynew_iterator/new_iterator_asynccallspan.__exit__.- If
AgentExecutor.streamraises before returning an iterator, the span is never finished and remains the active span for later work. google_genaistream wrappers catch exceptions around the underlying call and finish the span; this path does not.
Identified by Warden · code-review · GUS-JBC
Comment on lines
777
to
800
| @@ -1169,44 +794,7 @@ def capture_event( | |||
|
|
|||
| envelope = Envelope(headers=headers) | |||
|
|
|||
| if is_transaction and isinstance(profile, Profile): | |||
| envelope.add_profile(profile.to_json(event_opt, self.options)) | |||
|
|
|||
| if is_transaction and not span_recorder_has_gen_ai_span: | |||
| envelope.add_transaction(event_opt) | |||
| elif is_transaction: | |||
| split_spans = _split_gen_ai_spans(event_opt) | |||
| if split_spans is None or not split_spans[1]: | |||
| envelope.add_transaction(event_opt) | |||
| else: | |||
| non_gen_ai_spans, gen_ai_spans = split_spans | |||
|
|
|||
| event_opt["spans"] = non_gen_ai_spans | |||
| envelope.add_transaction(event_opt) | |||
|
|
|||
| converted_gen_ai_spans = [ | |||
| _serialized_v1_span_to_serialized_v2_span(span, event_opt) | |||
| for span in gen_ai_spans | |||
| if isinstance(span, dict) | |||
| ] | |||
|
|
|||
| envelope.add_item( | |||
| Item( | |||
| type=SpanBatcher.TYPE, | |||
| content_type=SpanBatcher.CONTENT_TYPE, | |||
| headers={ | |||
| "item_count": len(converted_gen_ai_spans), | |||
| }, | |||
| payload=PayloadRef( | |||
| json={ | |||
| "version": 2, | |||
| "items": converted_gen_ai_spans, | |||
| }, | |||
| ), | |||
| ) | |||
| ) | |||
|
|
|||
| elif is_checkin: | |||
| if is_checkin: | |||
| envelope.add_checkin(event_opt) | |||
| else: | |||
| envelope.add_event(event_opt) | |||
Contributor
There was a problem hiding this comment.
Transaction events now sent as error events
After removing transaction handling, events with type "transaction" still pass through and are sampled as errors then envelope.add_event()'d; drop or reject them explicitly instead of mis-categorizing.
Evidence
- Previously
is_transactionskipped_should_sample_errorand usedenvelope.add_transaction(event_opt). - That branch is gone; only
is_checkinis special-cased, sotype == "transaction"falls into theelsepath. envelope.add_event()always sets item type to"event", so a transaction payload is emitted as an error event.- Call sites still exist that pass
{"type": "transaction"}intocapture_event(e.g.tests/test_transport.py), andEventtyping still allows"transaction".
Identified by Warden · code-review · 3PJ-F9H
Comment on lines
1279
to
1281
| ca_certs: "Optional[str]" = None, | ||
| propagate_traces: bool = True, | ||
| traces_sample_rate: "Optional[float]" = None, | ||
| trace_lifecycle: "Optional[Literal['static', 'stream']]" = None, | ||
| traces_sampler: "Optional[TracesSampler]" = None, |
Contributor
There was a problem hiding this comment.
trace_lifecycle removed without migration note
Removing trace_lifecycle will hard-fail existing init(trace_lifecycle=...) callers, and MIGRATION_GUIDE.md does not document the removal or that stream mode is now the only behavior.
Evidence
- The hunk deletes
trace_lifecycle: Optional[Literal['static', 'stream']]from the public constructor signature. DEFAULT_OPTIONSis generated fromClientConstructor.__init__, so the option disappears from accepted config keys._get_options()rejects unknown keys withTypeError, so old configs break at init time.- MIGRATION_GUIDE.md has no entry for
trace_lifecycle, while related removals likestream_gen_ai_spansare explicitly listed.
Identified by Warden · code-review · KAM-DSE
Comment on lines
+179
to
+186
| streaming_span = sentry_sdk.traces.start_span( | ||
| name=span.name, | ||
| parent_span=span, | ||
| attributes={ | ||
| "sentry.op": OP.HTTP_CLIENT_STREAM, | ||
| "sentry.origin": Boto3Integration.origin, | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
Boto3 streaming span remains active and hijacks subsequent span parenting
Create the streaming span with active=False (or explicitly scope it only around body reads). The current implementation leaves it as the active span for the entire lifetime of the returned StreamingBody, causing unrelated spans and trace propagation to use the HTTP streaming span as their parent and allowing delayed cleanup to overwrite the current span stack.
Evidence
_sentry_after_call()ends the request span at line 173, then startsstreaming_spanwith the defaultactive=Trueat lines 179-186.Span._start()replacesscope.spanwith the streaming span, so spans created while the body remains open are parented tohttp.client.streamrather than the caller's span.streaming_span.end()runs later fromread()orclose()and restores the span saved at creation; if cleanup occurs while another span is active,_end()unconditionally overwrites that current scope entry, corrupting the span stack.- Existing boto3 streaming tests assert emitted span metadata and parent IDs but do not verify the current span or nested spans while the body is open.
Identified by Warden · find-bugs · L6S-EH8
Boost the likelihood that agents learn the correct API when exploring our SDK code.
…voke Agent spans (#7520) Remove the available tools from Invoke Agent spans because it is ambiguous for an agent that makes multiple model calls in the course of its execution. Each model call can have different available tools.
…t spans (#7517) Remove the available tools from Invoke Agent spans because it is ambiguous for an agent that makes multiple model calls in the course of its execution. Each model call can have different available tools.
There's a `data_collection` version of the removed tests. Closes https://linear.app/getsentry/issue/PY-2810/remove-send-default-pii-from-ariadne
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
### Description
- drop `send_default_pii` support from aws-lambda integration.
- explicitly use `data_collection: {}` in `sentry_init(...)` to fallback
to default config.
#### Issues
Resolves #7585
Also added some empty lines to tests to make them easier to parse Closes https://linear.app/getsentry/issue/PY-2811/remove-send-default-pii-from-celery
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
The parameter allowed users to opt out of collecting GenAI attributes while keeping `send_default_pii=True`. It is ignored when data collection is used, and the major release drops `send_default_pii` in favor of data collection.
…ent spans (#7521) Remove the response streaming flag from Invoke Agent spans because it is ambiguous for an agent that makes multiple model calls in the course of its execution. Each model call may be synchronous or streamed, independently.
There is no need for agents to be under a common parent in span-first, since agents that hand off to each other share the same trace ID in the streaming trace lifecycle (sibling agent invocation spans).
### Description - remove `send_default_pii` branching in `tests_s3.py`. - Remove `send_default_pii` fallback from `tracing_utils.get_url_attributes()` #### Issues Resolves #7658
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
We're preparing our next major on this branch.
The project is tracked in Linear. If you don't have access, we'll try to tag issues belonging to the project with the
SDK3.0 label on GitHub so that you can follow along.Notable changes
Context
You might have read this announcement about us discontinuing work on a 3.0. This is referring to the work done on the
potel-basebranch, which included two types of changes: a huge refactor of our tracing code on the one hand, and various unrelated changes, improvements and fixes on the other. We're dropping the huge refactor part, and only porting the rest, to a new branch and eventually a new 3.0 release.