[DI] Add source map support #5205

watson · 2025-02-05T14:56:33Z

What does this PR do?

Resolve probe location using a source map if available.

Motivation

A lot of users will transpile their source code before deploying. For example if they develop in TypeScript. Without source map support, these users will not be able to use Dynamic Instrumentation reliably, as the probe file/line might not match a file/line in the running app.

Plugin Checklist

Additional Notes

watson · 2025-02-05T14:56:48Z

[DI] Add source map support #5205 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2025-02-05T14:57:33Z

Overall package size

Self size: 8.69 MB
Deduped: 94.89 MB
No deduping: 95.41 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 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 | 835.4 kB | | source-map | 0.7.4 | 226 kB | 226 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 | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 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 | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 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 | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 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}

codecov · 2025-02-05T14:57:53Z

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.22%. Comparing base (8e8898d) to head (f020cab).
Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/datadog-plugin-cucumber/src/index.js	0.00%	3 Missing ⚠️
packages/datadog-plugin-mocha/src/index.js	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5205      +/-   ##
==========================================
- Coverage   81.24%   81.22%   -0.03%     
==========================================
  Files         487      487              
  Lines       21703    21709       +6     
==========================================
  Hits        17633    17633              
- Misses       4070     4076       +6

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

pr-commenter · 2025-02-05T15:06:33Z

Benchmarks

Benchmark execution time: 2025-02-13 10:44:06

Comparing candidate commit f020cab in PR branch watson/DEBUG-3353/source-map-support with baseline commit 8e8898d in branch master.

Found 58 performance improvements and 1 performance regressions! Performance is the same for 848 metrics, 26 unstable metrics.

scenario:debugger-enabled-but-breakpoint-not-hit-18

🟩 cpu_usage_percentage [-29.519%; -27.644%]
🟩 cpu_user_time [-427.515ms; -411.386ms] or [-61.266%; -58.955%]
🟩 execution_time [-404.378ms; -378.941ms] or [-37.523%; -35.163%]
🟩 instructions [-714.3M instructions; -710.4M instructions] or [-55.191%; -54.887%]
🟩 max_rss_usage [-11.419MB; -11.001MB] or [-14.059%; -13.545%]

scenario:debugger-enabled-but-breakpoint-not-hit-20

🟩 cpu_usage_percentage [-22.040%; -21.374%]
🟩 cpu_user_time [-322.688ms; -310.984ms] or [-58.936%; -56.798%]
🟩 execution_time [-371.185ms; -355.333ms] or [-35.985%; -34.448%]
🟩 instructions [-715.8M instructions; -709.2M instructions] or [-48.949%; -48.503%]
🟩 max_rss_usage [-8.686MB; -8.326MB] or [-11.844%; -11.354%]

scenario:debugger-enabled-but-breakpoint-not-hit-22

🟩 cpu_usage_percentage [-22.653%; -21.693%]
🟩 cpu_user_time [-309.780ms; -296.955ms] or [-60.985%; -58.460%]
🟩 execution_time [-364.387ms; -345.445ms] or [-36.420%; -34.527%]
🟩 instructions [-707.0M instructions; -704.2M instructions] or [-49.825%; -49.628%]
🟩 max_rss_usage [-10.142MB; -9.215MB] or [-11.923%; -10.834%]

scenario:debugger-line-probe-with-snapshot-default-18

🟩 cpu_usage_percentage [-53.756%; -53.089%]
🟩 cpu_user_time [-18.596s; -18.232s] or [-99.474%; -97.531%]
🟩 execution_time [-19.688s; -18.749s] or [-98.898%; -94.183%]
🟩 instructions [-78.2G instructions; -76.1G instructions] or [-100.563%; -97.939%]
🟩 max_rss_usage [-66.932MB; -64.885MB] or [-49.242%; -47.736%]

scenario:debugger-line-probe-with-snapshot-default-20

🟩 cpu_usage_percentage [-58.503%; -57.876%]
🟩 cpu_user_time [-17.343s; -17.058s] or [-99.505%; -97.872%]
🟩 execution_time [-18.851s; -18.494s] or [-97.501%; -95.658%]
🟩 instructions [-76.2G instructions; -76.1G instructions] or [-99.079%; -98.962%]
🟩 max_rss_usage [-58.338MB; -56.277MB] or [-47.797%; -46.108%]

scenario:debugger-line-probe-with-snapshot-default-22

🟩 cpu_usage_percentage [-61.863%; -61.318%]
🟩 cpu_user_time [-16.500s; -16.204s] or [-99.657%; -97.868%]
🟩 execution_time [-17.951s; -17.605s] or [-97.442%; -95.562%]
🟩 instructions [-73.3G instructions; -73.2G instructions] or [-99.077%; -98.998%]
🟩 max_rss_usage [-63.590MB; -62.044MB] or [-45.990%; -44.872%]

scenario:debugger-line-probe-with-snapshot-minimal-18

🟩 cpu_usage_percentage [-12.861%; -10.537%]
🟩 cpu_user_time [-174.870ms; -164.482ms] or [-38.626%; -36.331%]
🟩 execution_time [-189.542ms; -164.944ms] or [-21.978%; -19.126%]
🟩 instructions [-375.1M instructions; -369.9M instructions] or [-39.294%; -38.753%]

scenario:debugger-line-probe-with-snapshot-minimal-20

🟩 cpu_usage_percentage [-12.456%; -11.510%]
🟩 cpu_user_time [-156.268ms; -146.416ms] or [-41.199%; -38.601%]
🟩 execution_time [-177.252ms; -160.750ms] or [-21.280%; -19.299%]
🟩 instructions [-369.1M instructions; -365.6M instructions] or [-32.989%; -32.675%]

scenario:debugger-line-probe-with-snapshot-minimal-22

🟩 cpu_usage_percentage [-12.316%; -11.297%]
🟩 cpu_user_time [-147.676ms; -139.253ms] or [-42.408%; -39.989%]
🟩 execution_time [-176.262ms; -156.112ms] or [-21.847%; -19.349%]
🟩 instructions [-368.6M instructions; -366.6M instructions] or [-34.102%; -33.915%]

scenario:debugger-line-probe-without-snapshot-18

🟩 cpu_usage_percentage [-27.593%; -25.307%]
🟩 cpu_user_time [-392.183ms; -377.492ms] or [-58.810%; -56.607%]
🟩 execution_time [-371.597ms; -341.020ms] or [-35.687%; -32.751%]
🟩 instructions [-638.9M instructions; -635.9M instructions] or [-52.395%; -52.153%]
🟩 max_rss_usage [-9.950MB; -9.631MB] or [-12.490%; -12.089%]

scenario:debugger-line-probe-without-snapshot-20

🟩 cpu_usage_percentage [-20.806%; -19.986%]
🟩 cpu_user_time [-293.856ms; -284.771ms] or [-56.514%; -54.767%]
🟩 execution_time [-345.130ms; -330.767ms] or [-34.462%; -33.028%]
🟩 instructions [-628.7M instructions; -624.9M instructions] or [-45.603%; -45.327%]
🟩 max_rss_usage [-8.383MB; -8.082MB] or [-11.484%; -11.071%]

scenario:debugger-line-probe-without-snapshot-22

🟩 cpu_usage_percentage [-21.804%; -21.017%]
🟩 cpu_user_time [-278.412ms; -265.883ms] or [-58.382%; -55.755%]
🟩 execution_time [-323.766ms; -307.354ms] or [-33.513%; -31.814%]
🟩 instructions [-633.4M instructions; -629.0M instructions] or [-47.140%; -46.809%]
🟩 max_rss_usage [-9.763MB; -9.523MB] or [-11.462%; -11.181%]

scenario:plugin-graphql-with-async-hooks-18

🟥 max_rss_usage [+86.044MB; +93.304MB] or [+16.263%; +17.635%]

scenario:plugin-graphql-with-async-hooks-20

🟩 max_rss_usage [-111.951MB; -108.849MB] or [-17.393%; -16.911%]

datadog-datadog-prod-us1 · 2025-02-05T15:59:23Z

Datadog Report

Branch report: watson/DEBUG-3353/source-map-support
Commit report: eb72976
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 630 Passed, 0 Skipped, 15m 49.97s Total Time

watson · 2025-02-06T11:56:48Z

packages/dd-trace/src/ci-visibility/dynamic-instrumentation/worker/index.js

    try {
-      lineNumber = await processScriptWithInlineSourceMap({ file, line, sourceMapURL })
+      lineNumber = await getSourceMappedLine(url, source, line, sourceMapURL)
    } catch (err) {
-      log.error('Error processing script with inline source map', err)
+      log.error('Error processing script with source map', err)
+    }
+    if (lineNumber === null) {
+      log.error('Could not find generated position for %s:%s', url, line)
+      lineNumber = line
    }


Hopefully the new algorithm is more stable in regards to finding the right line. If we still can't find it, shouldn't this code just throw so it's aborted?

I wanted to avoid throwing so that the next test may have a chance to still set a probe

Shouldn't you then at least return early and not try to attach the breakpoint to a line number that is most likely incorrect?

I know you try-catch around the code that adds the breakpoint takes care of that for you, but if you don't think it will work, I think it makes the purpose of the code more readable if we return early in that case

Shouldn't you then at least return early and not try to attach the breakpoint to a line number that is most likely incorrect?

you're right. I remember finding a situation where the source map was there, processScriptWithInlineSourceMap failed, but setting the probe in the original line number worked 😓. I'm definitely missing something here

Resolve probe location using source maps if available

juan-fernandez · 2025-02-13T11:41:19Z

packages/datadog-plugin-mocha/src/index.js

+            while (Date.now() < waitUntil) {
+              // TODO: To avoid a race condition, we should wait until `probeInformation.setProbePromise` has resolved.
+              // However, Mocha doesn't have a mechanism for waiting asyncrounously here, so for now, we'll have to
+              // fall back to a fixed syncronous delay.


I think this makes sense and it's likely a good tradeoff, as the user opts in to having a debugger probe in their test. If they're debugging a problematic test, they're likely OK paying a 200ms active wait, as the test is likely to take much longer.

Thanks for this idea!

juan-fernandez · 2025-02-13T11:41:44Z

packages/datadog-plugin-cucumber/src/index.js

+            // TODO: To avoid a race condition, we should wait until `probeInformation.setProbePromise` has resolved.
+            // However, Cucumber doesn't have a mechanism for waiting asyncrounously here, so for now, we'll have to
+            // fall back to a fixed syncronous delay.
+          }


same comment as https://github.com/DataDog/dd-trace-js/pull/5205/files#r1954345426

juan-fernandez

thanks for worrying about test optimization code and for finding this nice trick 😄 . LGTM!

watson self-assigned this Feb 5, 2025

watson added semver-patch dynamic-instrumentation semver-minor and removed semver-patch labels Feb 5, 2025

watson force-pushed the watson/DEBUG-3353/source-map-support branch from e5d2e4a to 0e13626 Compare February 5, 2025 15:06

watson force-pushed the watson/DEBUG-3353/source-map-support branch from 0e13626 to 76de624 Compare February 5, 2025 15:58

watson force-pushed the watson/DEBUG-3353/source-map-support branch 2 times, most recently from cf1d731 to d354b44 Compare February 5, 2025 16:18

watson commented Feb 6, 2025

View reviewed changes

watson force-pushed the watson/DEBUG-3353/source-map-support branch from d354b44 to b29d37f Compare February 11, 2025 13:31

[DI] Add source map support

d75657e

Resolve probe location using source maps if available

watson force-pushed the watson/DEBUG-3353/source-map-support branch from b29d37f to 7962024 Compare February 13, 2025 10:21

watson requested a review from juan-fernandez February 13, 2025 10:21

Refactor ci-visibility

f020cab

watson force-pushed the watson/DEBUG-3353/source-map-support branch from 7962024 to f020cab Compare February 13, 2025 10:34

watson marked this pull request as ready for review February 13, 2025 11:26

watson requested review from a team as code owners February 13, 2025 11:26

juan-fernandez reviewed Feb 13, 2025

View reviewed changes

juan-fernandez approved these changes Feb 13, 2025

View reviewed changes

watson merged commit efb8e44 into master Feb 17, 2025
358 checks passed

watson deleted the watson/DEBUG-3353/source-map-support branch February 17, 2025 08:51

watson added a commit that referenced this pull request Feb 17, 2025

[DI] Add source map support (#5205)

26b3585

watson mentioned this pull request Feb 17, 2025

v5.37.0 proposal #5281

Merged

watson added a commit that referenced this pull request Feb 17, 2025

[DI] Add source map support (#5205)

00c9a09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DI] Add source map support #5205

[DI] Add source map support #5205

watson commented Feb 5, 2025

watson commented Feb 5, 2025

github-actions bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

pr-commenter bot commented Feb 5, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 5, 2025 •

edited

Loading

watson Feb 6, 2025

juan-fernandez Feb 6, 2025

watson Feb 6, 2025 •

edited

Loading

watson Feb 6, 2025

juan-fernandez Feb 6, 2025

juan-fernandez Feb 13, 2025

juan-fernandez Feb 13, 2025

juan-fernandez left a comment

[DI] Add source map support #5205

[DI] Add source map support #5205

Conversation

watson commented Feb 5, 2025

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

watson commented Feb 5, 2025

github-actions bot commented Feb 5, 2025 • edited Loading

Overall package size

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

pr-commenter bot commented Feb 5, 2025 • edited Loading

Benchmarks

scenario:debugger-enabled-but-breakpoint-not-hit-18

scenario:debugger-enabled-but-breakpoint-not-hit-20

scenario:debugger-enabled-but-breakpoint-not-hit-22

scenario:debugger-line-probe-with-snapshot-default-18

scenario:debugger-line-probe-with-snapshot-default-20

scenario:debugger-line-probe-with-snapshot-default-22

scenario:debugger-line-probe-with-snapshot-minimal-18

scenario:debugger-line-probe-with-snapshot-minimal-20

scenario:debugger-line-probe-with-snapshot-minimal-22

scenario:debugger-line-probe-without-snapshot-18

scenario:debugger-line-probe-without-snapshot-20

scenario:debugger-line-probe-without-snapshot-22

scenario:plugin-graphql-with-async-hooks-18

scenario:plugin-graphql-with-async-hooks-20

datadog-datadog-prod-us1 bot commented Feb 5, 2025 • edited Loading

Datadog Report

watson Feb 6, 2025

Choose a reason for hiding this comment

juan-fernandez Feb 6, 2025

Choose a reason for hiding this comment

watson Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

watson Feb 6, 2025

Choose a reason for hiding this comment

juan-fernandez Feb 6, 2025

Choose a reason for hiding this comment

juan-fernandez Feb 13, 2025

Choose a reason for hiding this comment

juan-fernandez Feb 13, 2025

Choose a reason for hiding this comment

juan-fernandez left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

pr-commenter bot commented Feb 5, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 5, 2025 •

edited

Loading

watson Feb 6, 2025 •

edited

Loading