GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||
GeezerVison is showing github.com — fix(aws-lambda): Remove messaging.batch.message_count from the Lambda span by sentry-junior[bot] · Pull Request #8229 · getsentry/sentry-python Conversation
Contributor
DescriptionThe Lambda span set This PR removes the attribute from the Lambda span. The count belongs on messaging spans, as in the OTel messaging conventions:
The JS SDK doesn't set it on the Lambda span either. The attribute was added in 3.0 (#7941) and never shipped in 2.x, so no released behavior changes. Replaces #8219 (same commits). GitHub never ran CI on #8219. Note for review: this is narrower than PY-3015, which asks to keep the attribute for SQS/SNS deliveries. @alexander-alderman-webb, please confirm that's OK. Issues
via @dingsdax. --
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…NS batches The Lambda span set messaging.batch.message_count to 1 for every non-list event and to len(event) for a top-level list. An SQS or SNS delivery is a dict with Records, so a batch of 10 messages reported 1, and HTTP or direct invocations reported a batch they don't have. Set it only when all records come from SQS or SNS, to the number of records. Co-Authored-By: Johannes Daxböck <[email protected]> … span Replace the SQS/SNS record counting with a plain removal. The count belongs on messaging spans (boto3 receive and batch-send spans, and the queue.process span for SQS/SNS triggers), as in the OTel messaging conventions. The JS SDK doesn't set it on the Lambda span either. The attribute was added in 3.0 and never shipped in 2.x. Co-Authored-By: johdax <[email protected]> …event Co-Authored-By: Johannes Daxböck <[email protected]> All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
This was referenced
|
There was an error while loading. Please reload this page.
Coverage Results 📊✅ 58405 passed | ❌ 1 failed | ⏭️ 3071 skipped | Total: 61477 | Pass Rate: 95% | Execution Time: 155m 2s ❌ Failed Tests
|
| File | Coverage % | Lines |
|---|---|---|
sentry_sdk/integrations/aws_lambda/_handler.py |
16.54% |
@@ Coverage Diff @@ ## master #8229 +/-## ========================================== + Coverage 90.21% 90.30% +0.09% ========================================== Files 203 199 -4 Tracked lines 27335 21824 -5511 Branches 10246 7604 -2642 ========================================== - Hits 24660 19708 -4952 + Misses 2675 2116 -559 + Partials 1559 1255 -304
Generated by Coverage Action
Sorry, something went wrong.
There was an error while loading. Please reload this page.
) ### Description Since #8137, #8142, and #8145, boto3 puts `sentry-trace` in SQS and SNS message attributes, but nothing reads it on the Lambda side. This PR is the smallest change that closes the loop for SQS- and SNS-triggered functions. The Lambda integration now reads `sentry-trace` from each record and adds a `message_origin` link from the invocation span to each distinct producer span. This matches what the boto3 `ReceiveMessage` span already does. Messages never become the invocation's parent. It reads: - SQS records: `messageAttributes["sentry-trace"].stringValue` - SNS records: `Sns.MessageAttributes["sentry-trace"].Value` - SNS → SQS without raw message delivery: the SNS envelope in the SQS `body`, the same as boto3 does Links are deduplicated, because a batch send gives many messages the same producer span, and capped at 128, OTel's default, because Lambda can deliver up to 10,000 SQS messages at once. **Scope.** This is intentionally kept separate from the stretch stack. It doesn't add `faas.trigger` (#8222) or `queue.process` spans (#8223), and it doesn't touch `messaging.batch.message_count` (#8229). The invocation span is otherwise unchanged. Synchronous `Invoke` propagation through `ClientContext` is #8221. That PR changes the `continue_trace` call and this one only adds `links=`, so the two are independent. The link-building code repeats boto3's `_message_link` in a Lambda-event shape: Lambda records use `messageAttributes` / `stringValue` / `body`, while the boto3 response uses `MessageAttributes` / `StringValue` / `Body`. Importing from the boto3 integration would make `aws_lambda` depend on botocore. **Testing.** `test_producer_links` sends SQS, SNS, and SNS-to-SQS events to `BasicOkSpanStreaming`. It asserts the segment span's `links` and that the segment span has no parent. It passes in CI's Cloud (Lambda) jobs, and `ruff` and `mypy sentry_sdk` pass. `codecov/patch` is red because the Lambda tests run in `sam local` containers, so their coverage isn't counted. #### Issues * Part of #8081 <!-- junior-request-attribution:start --> via **webb**, @sentrivana. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC09M26BC6A1%3A1794211222.463239 --> -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC09M26BC6A1%3A1794211222.463239) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC09M26BC6A1%3A1794211222.463239/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Alexander Webb <[email protected]> Co-authored-by: Ivana Kellyer <[email protected]>
There was an error while loading. Please reload this page.
Restores the batch count on the invocation span, as on major/3.0, so this PR only adds faas.trigger.
|
@sentry-junior Why was this attribute added in the first place if it was wrong? Can you double-check that removing it is the right way to go? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Checked again. Short version: it came from the old Where it came from. Why it's wrong on that span.
Where the count lives instead. boto3 send-batch and receive spans (#8137, #8142), and the The one behavior change: span-streaming users on 2.63+ who invoke with a top-level list lose this attribute in 3.0. If we want to keep that case, a better home would be the old |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
If this is a change from 2.x, also add it to MIGRATION_GUIDE.md |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
### Description Sets `faas.trigger` on the Lambda span based on the shape of the delivered event: - `http`: API Gateway REST/HTTP, function URLs, ALB, CloudFront, AppSync - `pubsub`: SQS, SNS, Kinesis, Kafka, MQ, SES, EventBridge - `datasource`: S3, DynamoDB Streams, DocumentDB - `timer`: EventBridge schedules - `other`: anything else The trigger is read from the event envelope, never from its payload. `test_trigger` has one case per code path. Other event sources use the same lookup table, so they have no extra cases. This PR doesn't touch `messaging.batch.message_count`. That change is in #8229. This is Pablo's code from `pablo/aws-lambda-trigger`, without his batch counting for the invocation span. It's the second of three stacked PRs (#8221 → this → #8223). #### Issues * resolves: PY-3018 (`faas.trigger` part; the per-source attributes in the issue aren't in this PR) * Part of #8081 <!-- junior-request-attribution:start --> via @dingsdax. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC0C9X9V5NMT%3A1793904671.980379 --> -- [View Session](https://junior-prod.sentry.dev/conversations/slack%3AC0C9X9V5NMT%3A1793904671.980379) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0C9X9V5NMT%3A1793904671.980379/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: Pablo Deputter <[email protected]> Co-authored-by: Pablo Deputter <[email protected]> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Johannes Daxböck <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.