[DSM] Set checkpoints for DSM even when there is no context if the service is instrumented and fix typo #4851

ericfirth · 2024-10-31T19:26:25Z

What does this PR do?

In investigating this support ticket, I found an issue with the SQS instrumentation for DSM. There were two issues, one is that if the messages were already parsed, we weren't using that because of a typo. Secondarily, we were not sending DSM checkpoints unless there was a context, which is not what we want to do. If a service is instrumented, we want to send checkpoints, as sometimes customers will instrument their consumers before their producers and we want them to see that DSM is working (even if its less useful without the producers being present)

Motivation

Additional Notes

github-actions · 2024-10-31T19:27:13Z

Overall package size

Self size: 7.99 MB
Deduped: 94.28 MB
No deduping: 94.62 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.2.2 | 29.27 MB | 29.27 MB | | @datadog/native-appsec | 8.2.1 | 19.18 MB | 19.19 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2024-10-31T19:34:51Z

Benchmarks

Benchmark execution time: 2024-11-15 15:25:01

Comparing candidate commit 13c2baf in PR branch eric.firth/fix-sqs-to-send-dsm-checkpoints with baseline commit 25ae8e7 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 260 metrics, 6 unstable metrics.

wconti27

Code change looks good to me, can we also get a test case added here. Also, can you please make the same change for Kinesis here?

ericfirth · 2024-11-08T19:20:56Z

Code change looks good to me, can we also get a test case added here. Also, can you please make the same change for Kinesis here?

Took me a while, but I did both

packages/datadog-plugin-aws-sdk/src/services/kinesis.js

ericfirth · 2024-11-14T21:58:27Z

/merge

dd-devflow · 2024-11-14T21:58:38Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-14 21:58:38 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-14 21:58:40 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-15 01:58:41 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

codecov · 2024-11-15T14:03:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.12%. Comparing base (985cb1d) to head (ccaaf4c).

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4851       +/-   ##
===========================================
- Coverage   62.11%   47.12%   -14.99%     
===========================================
  Files         280       98      -182     
  Lines       13009     2856    -10153     
===========================================
- Hits         8081     1346     -6735     
+ Misses       4928     1510     -3418

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…hen the producer did not have DSM enabled

…amName

ericfirth · 2024-11-15T15:35:16Z

/merge

dd-devflow · 2024-11-15T15:35:47Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-15 15:35:46 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-15 15:35:53 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-15 18:27:04 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

ericfirth · 2024-11-15T18:26:54Z

/remove

dd-devflow · 2024-11-15T18:26:59Z

Devflow running: `/remove`

View all feedbacks in Devflow UI.

2024-11-15 18:26:59 UTC ℹ️ Devflow: /remove

ericfirth · 2024-11-15T18:27:21Z

/merge

dd-devflow · 2024-11-15T18:27:33Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-15 18:27:33 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-15 18:27:36 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-15 18:41:18 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

ericfirth · 2024-11-15T18:41:08Z

/remove

dd-devflow · 2024-11-15T18:41:14Z

Devflow running: `/remove`

View all feedbacks in Devflow UI.

2024-11-15 18:41:14 UTC ℹ️ Devflow: /remove

…rvice is instrumented and fix typo (#4851) * [DSM] Set checkpoints for DSM with SQS & Kinesis for consumers even when the producer did not have DSM enabled * [DSM] Send checkpoints to DSM if its enabled even if there is no streamName

ericfirth requested review from a team as code owners October 31, 2024 19:26

wconti27 reviewed Oct 31, 2024

View reviewed changes

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch 3 times, most recently from ce406ad to bb8f958 Compare November 8, 2024 19:20

ericfirth requested a review from wconti27 November 8, 2024 19:21

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch from bb8f958 to 996f669 Compare November 8, 2024 19:22

ericfirth added the semver-patch label Nov 8, 2024

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch 2 times, most recently from c3f0538 to 84dd4f0 Compare November 11, 2024 15:56

wconti27 reviewed Nov 12, 2024

View reviewed changes

packages/datadog-plugin-aws-sdk/src/services/kinesis.js Outdated Show resolved Hide resolved

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch from e8641e7 to c59321d Compare November 13, 2024 15:05

ericfirth marked this pull request as draft November 13, 2024 18:04

ericfirth marked this pull request as ready for review November 13, 2024 18:04

ericfirth requested review from wconti27 and piochelepiotr November 13, 2024 18:41

piochelepiotr approved these changes Nov 14, 2024

View reviewed changes

dd-devflow bot added mergequeue-status: waiting mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 14, 2024

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch from c59321d to ccaaf4c Compare November 15, 2024 14:02

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch from c59321d to 562bbe9 Compare November 15, 2024 14:24

[DSM] Set checkpoints for DSM with SQS & Kinesis for consumers even w…

0536fa0

…hen the producer did not have DSM enabled

[DSM] Send checkpoints to DSM if its enabled even if there is no stre…

13c2baf

…amName

ericfirth force-pushed the eric.firth/fix-sqs-to-send-dsm-checkpoints branch from 562bbe9 to 13c2baf Compare November 15, 2024 15:18

dd-devflow bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Nov 15, 2024

wconti27 approved these changes Nov 15, 2024

View reviewed changes

dd-devflow bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 15, 2024

dd-devflow bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Nov 15, 2024

dd-devflow bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 15, 2024

bengl merged commit 51bea54 into master Nov 15, 2024
231 of 232 checks passed

bengl deleted the eric.firth/fix-sqs-to-send-dsm-checkpoints branch November 15, 2024 18:57

rochdev mentioned this pull request Nov 15, 2024

v4.50.0 proposal #4887

Merged

rochdev mentioned this pull request Nov 15, 2024

v5.26.0 proposal #4888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DSM] Set checkpoints for DSM even when there is no context if the service is instrumented and fix typo #4851

[DSM] Set checkpoints for DSM even when there is no context if the service is instrumented and fix typo #4851

ericfirth commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading

pr-commenter bot commented Oct 31, 2024 •

edited

Loading

wconti27 left a comment

ericfirth commented Nov 8, 2024

ericfirth commented Nov 14, 2024

dd-devflow bot commented Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024 •

edited

Loading

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024 •

edited

Loading

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024 •

edited

Loading

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024

[DSM] Set checkpoints for DSM even when there is no context if the service is instrumented and fix typo #4851

[DSM] Set checkpoints for DSM even when there is no context if the service is instrumented and fix typo #4851

Conversation

ericfirth commented Oct 31, 2024 • edited Loading

What does this PR do?

Motivation

Additional Notes

github-actions bot commented Oct 31, 2024 • edited Loading

Overall package size

pr-commenter bot commented Oct 31, 2024 • edited Loading

Benchmarks

wconti27 left a comment

Choose a reason for hiding this comment

ericfirth commented Nov 8, 2024

ericfirth commented Nov 14, 2024

dd-devflow bot commented Nov 14, 2024 • edited Loading

Devflow running: /merge

codecov bot commented Nov 15, 2024 • edited Loading

Codecov Report

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024 • edited Loading

Devflow running: /merge

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024

Devflow running: /remove

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024 • edited Loading

Devflow running: /merge

ericfirth commented Nov 15, 2024

dd-devflow bot commented Nov 15, 2024

Devflow running: /remove

ericfirth commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading

pr-commenter bot commented Oct 31, 2024 •

edited

Loading

dd-devflow bot commented Nov 14, 2024 •

edited

Loading

Devflow running: `/merge`

codecov bot commented Nov 15, 2024 •

edited

Loading

dd-devflow bot commented Nov 15, 2024 •

edited

Loading

Devflow running: `/merge`

Devflow running: `/remove`

dd-devflow bot commented Nov 15, 2024 •

edited

Loading

Devflow running: `/merge`

Devflow running: `/remove`