Repository navigation
Next Python SDK major - #5005
sentrivana wants to merge 447 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
|
Coverage Results 📊✅ 57426 passed | ⏭️ 2943 skipped | Total: 60369 | Pass Rate: 95.12% | Execution Time: 151m 28s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 88.41% (2838 of 3210 changed executable lines covered; target 80%). Changed files with executable lines (143)
Generated by Coverage Action |
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. |
Fixes for things that the bots [surfaced](#5005) on the major branch: - some version checks were too late (after patching) - fix TrytondWSGI integration name/`_MIN_VERSIONS` entry mismatch Also, changed the warning of the `DidNotEnable` message from "X not installed" to "X not installed or incompatible".
Originally raised by a bot [here](#5005 (comment)): the `parse_version` function parses version strings as is (e.g. 3.1 becomes `(3, 1)`). We use these parsed version tuples in integrations to compare the installed version against the minimum (defined in `integrations/__init__.py`). The minimum versions are often three-part, e.g. `(3, 1, 0)`. This means that we can mistakenly consider a valid version to be below the minimum, because in pure tuple terms, `(3, 1) < (3, 1, 0)` is true. This can also happen in reverse (package version has three parts, while our min version boundary has two). In this PR, we make the internal version comparison work as expected regardless of mismatches in the length of the version strings/tuples.
…8005) We're not recording the correct reason if a event/error processor raises (should be `callback_error`). Closes https://linear.app/getsentry/issue/PY-2888/drop-events-if-a-processor-raises
### Description boto3 spans now use `sentry.op = rpc` instead of `http.client`. Service extensions can (and will in the future) provide a more specific op with `get_span_op()`, e.g. `db` for DynamoDB. ### Why - boto3 span represents an AWS api call, not an HTTP request; the HTTP request is already captured as the as the `http.client` child - JS already also defaults AWS SDK spans to `rpc` It also lets these spans use new name and description rules in sentry-conventions, e.g. (getsentry/sentry-conventions#714, #717), e.g. `Lambda.Invoke billing` instead of `POST https://lambda.…`
### Description Rename the boto3 span origin from `auto.http.boto3` to `auto.aws.boto3`, for every boto3 span including the streamed response body child, and removes the unused `get_span_origin()` method from `_ServiceExtension`. ### Why 1. `auto.http.boto3` describes the underlying transport/integration used; boto3 spans are AWS API calls, and (in the future) the ops will be changed from `http.client` => `rpc`, `db`, `queue.*`, `object.*`, so it doesn't really fit anymore. 2. JS does the same and uses `auto.aws.aws_sdk`. ### Other option The other option is to use `auto.rpc.boto3` as the default origin (since `aws` isn't technically part of the categories, even though JS uses it https://develop.sentry.dev/sdk/telemetry/traces/trace-origin/) and override it with the span's op for different services: - `db` => `auto.db.boto3` - `queue.*` => `auto.queue.boto3` - `object.* ` => `auto.object.boto3` - streamed body child has op `http.client.stream` => `auto.http.boto3`
### Description Move aws-lambda integraton into package; moved some constants that are now used across modules in `aws_lambda/consts.py`. Mainly done for ease of future development and diff-checks
### Description - use of f-strings instead `”...”.format()`. - move constants into `consts.py`. - add new attributes to `SPANDATA` (will be used in the next PR).
### Description Expand aws-lambda instrumentation with extra resource identity and runtime metadata following OTel (https://opentelemetry.io/docs/specs/semconv/faas/aws-lambda/). Changes: - add `sentry.kind=server`. - add `cloud.account.id`, extracted from the invoked function ARN. - `cloud.resource_id` is now normalized from the invoked function ARN; keep the original value in `aws.lambda.invoked_arn` and replace an alias with the resolved published version when the function is not `$LATEST`. - `cloud.region` is now extracted from `AWS_REGION` env. var; `faas.name` from `AWS_LAMBDA_FUNCTION_NAME`; `faas.version` from `AWS_LAMBDA_FUNCTION_VERSION` following OTel (https://opentelemetry.io/docs/specs/semconv/faas/aws-lambda/#resource-detector). - refresh `sentry_sdk` on every test setup (for easier local testing so runs don’t reuse old behavior from earlier implementation). - add `capture_internal_exceptions()` around attribute extraction so that a single failure does not affect the other instrumentation. #### Issues Resolves #7931
### Description Adds the following attributes: - `aws.secretsmanager.secret.arn` (OTel): from an ARN `SecretID`, replaced by the response `ARN` (if available). - `aws.secretsmanager.secret.name` (getsentry/sentry-conventions#716): from response `Name` or the request `Name` or `SecretId`, else the ARN without its random `-XXXXXX` suffix (in case of failed calls). The actual secret values are never read. #### Issues Resolves #7574
## Description Adds the following attributes: - `aws.step_functions.state_machine.arn`, `*.activity.arn` (OTel) and `*.execution.arn` (getsentry/sentry-conventions#672), from the request and response. - `aws.step_functions.state_machine.name` and `*.activity.name` (getsentry/sentry-conventions#716), from `name` on create calls or from the ARN without its version/alias qualifier. Whenever `StartSyncExecution` returns a failed call (`FAILED` or `TIMED_OUT`), `error.type` will be set to the respective value. ### Issues Resolves #7478
### Description Adds the following attribute: - `aws.kinesis.stream_name` (OTel): from `StreamName`, else from the stream in `StreamARN`, `ConsumerARN` or `ResourceARN` (`…:stream/<name>[/consumer/…]`). #### Issues Resolves #7575
Updated the changelog for the upcoming 3.0.0b1 release, highlighting notable changes, new features, and migration resources.
## Description Adds span links as is specified in the [specs](https://develop.sentry.dev/sdk/telemetry/spans/#span-links): a span can reference other spans, e.g. in other traces, without making them its parent. - `sentry_sdk.start_span(..., links=[...])`, `span.add_link(link)` and `span.add_links(links)`; with links being of the following format: `{"trace_id", "span_id", "sampled"?, "attributes"?}` - span without links omits `links` - `NoOpSpan` ignores links - added to the migration guide as well - [example](https://sentry-sdks.sentry.io/explore/traces/trace/8194b70609574447aa8732f24e1350a0/?mode=samples&node=span-bad54d1bc8cb9604&project=4511733153660928&source=traces&statsPeriod=24h&table=trace×tamp=1792484175.113) in sentry-test Related docs update can be found [here](getsentry/sentry-docs#20103). ## Why An SQS/SNS consumer can receive a batch of records sent by several producers, each in its own trace. A span can only have one parent, so without links there's no way to connect the consumer each producer. With links the consumer span can point to each producer's trace.
| sentry_sdk.init( | ||
| data_collection={ | ||
| "user_info": False, | ||
| "http_bodies": [], | ||
| "graphql": { | ||
| "document": False, | ||
| "variables": False, | ||
| }, | ||
| "gen_ai": { | ||
| "inputs": False, | ||
| "outputs": False, | ||
| }, | ||
| "mcp": { | ||
| "inputs": False, | ||
| "outputs": False, | ||
| }, | ||
| "database_query_data": False, | ||
| "queues": False, | ||
| "http_headers": { | ||
| "request": { | ||
| "mode": "denylist", | ||
| "terms": ["forwarded", "-ip", "remote-", "via", "-user"] | ||
| }, | ||
| }, | ||
| "cookies": { "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"] }, | ||
| "url_query_params": { | ||
| "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"], | ||
| }, | ||
| }, | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
Migration recipe leaves ordinary cookies, query parameters, and frame locals unfiltered
The send_default_pii=False recipe is not a privacy equivalent: cookies and url_query_params use denylist modes, so ordinary keys and their values remain in events; the listed IP-related terms do not disable either category. The example also omits stack_frame_variables, which defaults to True and serializes locals without filtering. Set the cookie and query modes to off if they should not be collected, and configure stack_frame_variables to {"mode": "denylist"} or {"mode": "off"} to avoid sending unfiltered locals.
Evidence
- The example configures
cookiesandurl_query_paramswithmode: "denylist"and IP-related terms, rather than turning either category off. _apply_key_value_collection_filteringreturns an empty mapping only formode: "off"; in denylist mode, keys that do not match the built-in or configured terms retain their original values.data_collection._resolvedefaultsstack_frame_variablestoTrue, andserialize_frameserializesframe.f_localsdirectly in that mode;test_stack_frame_variables_true_does_not_filter_sensitive_localsconfirms sensitive locals are retained.- The guide separately says locals are no longer scrubbed by default and recommends configuring
stack_frame_variables, but that setting is absent from this recipe.
Identified by Warden · code-review, find-bugs · Z66-EC8
| identity = aws_event.get("requestContext", {}).get("identity") | ||
| if identity is None: | ||
| return {} | ||
|
|
||
| user_info: "dict[str, Any]" = {} | ||
|
|
||
| user_arn = identity.get("userArn") | ||
| if user_arn is not None: | ||
| user_info["id"] = user_arn | ||
|
|
||
| ip = identity.get("sourceIp") | ||
| if ip is not None: | ||
| user_info["ip_address"] = ip |
There was a problem hiding this comment.
_get_user_from_event crashes on non-dict requestContext/identity
Use isinstance checks for requestContext and identity before calling .get — a null or non-dict value raises AttributeError; the handler invokes this outside capture_internal_exceptions and can fail the Lambda before user code runs, and the event processor path can drop the error event.
Evidence
_get_user_from_eventdoesaws_event.get("requestContext", {}).get("identity"); if therequestContextkey is present with valueNone(or another non-dict), the default{}is skipped and.getraises AttributeError.- After the
identity is Noneguard,identity.get("userArn")/identity.get("sourceIp")still assume a mapping, so a non-dict identity also raises. _handler.pycalls_get_user_from_event(request_data)outsidecapture_internal_exceptionswhendata_collection["user_info"]is true (the default), so the exception abortssentry_handlerbefore the user handler runs.- The same helper runs inside the request event processor;
Scope.run_event_processorstreats a processor exception as a dropped event. - The same file already guards headers with
isinstance(..., dict)(e.g._get_url), but notrequestContext/identity.
Identified by Warden · code-review, find-bugs · RSZ-TZN
| if "queryStringParameters" in request_data: | ||
| qs = request_data["queryStringParameters"] | ||
| if qs: | ||
| filtered_qs = _apply_key_value_collection_filtering( | ||
| items=qs, | ||
| behaviour=data_collection["url_query_params"], | ||
| ) | ||
| if filtered_qs: | ||
| additional_attributes[SPANDATA.URL_QUERY] = urlencode( | ||
| filtered_qs | ||
| ) |
There was a problem hiding this comment.
queryStringParameters assumed to be a Mapping
Check isinstance(qs, Mapping) (or wrap in capture_internal_exceptions) before filtering: a truthy non-dict value makes _apply_key_value_collection_filtering call .items() and crash the invocation.
Evidence
- The branch only checks
if qs:then passesqsto_apply_key_value_collection_filtering(items=qs, ...). - That helper always does
items.items()with no type guard. - Unlike
headers(explicitly coerced to{}when not a dict for EventBridge),queryStringParametershas noisinstancecheck. - This path is outside
capture_internal_exceptions, so a bad value abortssentry_handlerbefore the user handler runs.
Identified by Warden · find-bugs, code-review · FLN-MQ8
| from sentry_sdk.consts import SPANDATA | ||
| from sentry_sdk.traces import StreamedSpan | ||
| from sentry_sdk.tracing_utils import has_span_streaming_enabled | ||
| from sentry_sdk.traces import Span |
There was a problem hiding this comment.
Closing a Cohere chat stream leaves its Span active
If a consumer closes the returned chat_stream iterator before it is exhausted, GeneratorExit bypasses the except Exception handler and span.end() is skipped. Because the span was activated when chat_stream was called, the current scope can retain that unfinished span and use it as the parent for later spans.
Evidence
_wrap_chatstarts an active span before returning the wrapped stream iterator.new_iteratorcallsspan.end()only after normal exhaustion; its cleanup handler catchesException, notGeneratorExit.- There is no
finallyblock, so closing a suspended iterator skipsspan.end(). Span._end()restores the previous scope span; without it, the scope can retain the unfinished span.
Also found at 2 additional locations
sentry_sdk/integrations/cohere.py:224-246sentry_sdk/integrations/cohere.py:197-260
Identified by Warden · find-bugs · AZ7-GHE
| sample_rate: float = 1.0, | ||
| send_default_pii: "Optional[bool]" = None, | ||
| data_collection: "Optional[DataCollectionUserOptions]" = None, | ||
| data_collection: "Optional[DataCollectionUserOptions]" = {}, |
There was a problem hiding this comment.
data_collection typed Optional but None raises TypeError
Drop Optional from the annotation (or accept None in _resolve_data_collection); type checkers allow data_collection=None, but init will raise TypeError.
Evidence
ClientConstructor.__init__typesdata_collectionasOptional[DataCollectionUserOptions]with default{}.DEFAULT_OPTIONSis built from these defaults via_get_default_options()/inspect.getfullargspec._resolve_data_collection()doesisinstance(user_dc, dict)and raisesTypeErrorfor any non-dict, includingNone._get_options()always calls_resolve_data_collection(rv), sosentry_sdk.init(data_collection=None)fails at runtime despite the Optional type.
Identified by Warden · code-review · BW5-RFW
### Description S3 object operations now override default `rpc` op with `object.*`. Other S3 operations, e.g. `ListBuckets`, stay on `rpc`.
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.