feat: add support for event bridge DSM context extraction - #836
jeastham1993 wants to merge 14 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 888ab65a55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
We should eventually add a new |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fc0cd0295
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff42108b08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24da389683
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
purple4reina
left a comment
purple4reina
left a comment
There was a problem hiding this comment.
Looks good.
There is a small bug here though, but I think it's okay as long as we call it out.
In the case where Eventbridge and SNS get routed to the same SQS queue, then are batched into a single event payload, we'll miss the SNS dsm context. This is because we search the entire batch for an eventbridge context and if found anywhere in the batch, it will ignore SNS dsm extraction.
It's a problem, but I think very minor. I think we should either add a comment about this or fix it if it's easy enough to address.
When EventBridge and SNS are both routed to the same SQS queue and their messages are batched into a single event payload, the SNS DSM context was dropped. _dsm_set_eventbridge_sqs_batch_checkpoints classifies every record in the batch and falls back to _extract_sqs_record_message_attribute_context for non-EventBridge records. That helper only read record.messageAttributes and never unwrapped an SNS notification carried in the SQS body (an SNS => SQS subscription without raw message delivery), so the SNS carrier resolved to None. Because the batch contained an EventBridge delivery, dsm_handled was True for the whole batch and the SNS-aware fallback path further down in extract_context_from_sqs_or_sns_event_or_context never ran. Teach the helper to unwrap the SNS envelope the same way the main SQS/SNS extraction path already does, so each record in a mixed batch is classified correctly regardless of whether it is a direct SQS send or an SNS delivery. The shared attribute decoding and envelope detection are factored out into _decode_dd_message_attribute and _parse_sns_notification_from_sqs_body.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66fdc28cdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The zipped layer size check started failing at 9273 kb against the 9231 kb limit. This is unrelated to any source change in this branch: the ddtrace cp311 manylinux x86_64 wheel grew ~470 kb in 4.15.0 (8.87 MB in 4.14.2 -> 9.33 MB in 4.15.0), and pyproject.toml pins ddtrace ">=4.1.1,<5,!=4.6.*" with no upper bound inside 4.x, so CI resolves to the newest 4.15.x. Raise MAX_LAYER_COMPRESSED_SIZE_KB from 9*1024+15 (9231 KB) to 9*1024+128 (9344 KB), which clears the observed 9273 kb with ~71 kb of headroom while keeping the guardrail tight enough to catch further growth. x86_64 is the larger of the two published arches (~245 kb above aarch64), so the failing cp311 x86_64 job is the worst case across the build matrix. The uncompressed limit is unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d54041de8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| detail = event.get("detail") or {} | ||
| dd_context = detail.get("_datadog") | ||
|
|
||
| _dsm_set_eventbridge_checkpoint(dd_context, event.get("detail-type")) |
There was a problem hiding this comment.
For standard scheduled EventBridge events (detail-type: "Scheduled Event", source: "aws.events", as in tests/event_samples/cloudwatch-events.json), parse_event_source overwrites the initial EVENTBRIDGE classification with CLOUDWATCH_EVENTS at datadog_lambda/trigger.py:158-159. Consequently extract_dd_trace_context never calls this function, and DSM emits no consume checkpoint for these contextless EventBridge invocations despite the new handling for events without _datadog; route the CloudWatch Events classification through the EventBridge checkpoint logic as well.
Useful? React with 👍 / 👎.
|
You may want to update README with this new config |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98c9ac1b28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not is_eventbridge_sqs: | ||
| return None, False, dsm_handled | ||
|
|
||
| if is_step_function_event(dd_context): |
There was a problem hiding this comment.
When DSM is enabled and the first SQS record is a valid EventBridge envelope whose detail lacks _datadog, the batch helper has already emitted its checkpoints, but this call passes None to is_step_function_event, which calls event.get(...) and raises AttributeError. The caller catches that exception before assigning the returned dsm_handled value, so it remains False and the regular SQS fallback emits a second disconnected checkpoint for the first record. This re-raises the prior double-counting issue with fresh evidence that the current missing-carrier path still throws before the new short-circuit can run.
Useful? React with 👍 / 👎.
This branch has not been deployed
4 participants
What does this PR do?
Adds support for automatically extracting DSM context from Amazon Event Bridge events. Configures the DSM
exchangetag using an environment variable.Motivation
Adding DSM support for all serverless messaging technologies.
Testing Guidelines
Wrote unit test to test logical locally.
Compiled test version of layer and pushed to AWS account, tested with sample app.
Additional Notes
Types of Changes
Check all that apply