Skip to content

[Code Origin for Spans] Add support for Express entry spans #5423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[Code Origin for Spans] Add support for Express entry spans #5423

wants to merge 1 commit into from

Conversation

watson
Copy link
Collaborator

@watson watson commented Mar 17, 2025

What does this PR do?

Add support for Express entry spans in Code Origin for Spans.

To enable, set DD_CODE_ORIGIN_FOR_SPANS_ENABLED=true.

This is a follow up to PR #4449, which added support for Fastify entry spans.

Motivation

Express is the most popular web framework in the Node.js ecosystem. Having support for this framework is vital for adoption.

Plugin Checklist

Additional Notes

@watson watson self-assigned this Mar 17, 2025
@watson Graphite App
Copy link
Collaborator Author

watson commented Mar 17, 2025

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 17, 2025 •
edited
Loading

Overall package size

Self size: 8.97 MB
Deduped: 101.51 MB
No deduping: 102.02 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.5.0 | 29.83 MB | 29.83 MB | | @datadog/native-appsec | 8.5.0 | 19.26 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 | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.4.0 | 2.77 MB | 5.42 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 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.13.1 | 117.64 kB | 839.26 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.1 | 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 | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | dc-polyfill | 0.1.6 | 24.56 kB | 24.56 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 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 Codecov
Copy link

codecov bot commented Mar 17, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.48%. Comparing base (4f0481e) to head (c5f529d).

Files with missing lines Patch % Lines
packages/datadog-instrumentations/src/router.js 75.00% 1 Missing ⚠️
packages/datadog-plugin-express/src/code_origin.js 93.33% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #5423   +/-   ##
=======================================
  Coverage   79.47%   79.48%           
=======================================
  Files         510      511    +1     
  Lines       22819    22835   +16     
=======================================
+ Hits        18136    18150   +14     
- Misses       4683     4685    +2

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

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@watson watson force-pushed the watson/DEBUG-2988/express-code-origin branch from 4478a99 to 7cd6e72 Compare March 17, 2025 10:07
@watson
[Code Origin for Spans] Add support for Express entry spans
c5f529d 
To enable, set `DD_CODE_ORIGIN_FOR_SPANS_ENABLED=true`.

This is a follow up to PR #4449, which added support for Fastify entry
spans.
@watson watson force-pushed the watson/DEBUG-2988/express-code-origin branch from 7cd6e72 to c5f529d Compare March 17, 2025 10:07
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Mar 17, 2025 •
edited
Loading

Datadog Report

Branch report: watson/DEBUG-2988/express-code-origin
Commit report: aa4b244
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 805 Passed, 0 Skipped, 11m 35.13s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Mar 17, 2025

Benchmarks

Benchmark execution time: 2025-03-17 10:25:13

Comparing candidate commit c5f529d in PR branch watson/DEBUG-2988/express-code-origin with baseline commit 4f0481e in branch master.

Found 0 performance improvements and 5 performance regressions! Performance is the same for 910 metrics, 18 unstable metrics.

scenario:appsec-iast-startup-time-iast-enabled-18

  • 🟥 execution_time [+98.019ms; +115.308ms] or [+5.107%; +6.007%]
  • 🟥 max_rss_usage [+11.497MB; +13.556MB] or [+8.505%; +10.027%]

scenario:appsec-iast-startup-time-iast-enabled-20

  • 🟥 cpu_user_time [+100.595ms; +120.770ms] or [+5.560%; +6.675%]
  • 🟥 execution_time [+111.249ms; +134.869ms] or [+5.479%; +6.642%]

scenario:appsec-iast-with-vulnerability-iast-enabled-always-active-20

  • 🟥 cpu_user_time [+89.351ms; +134.518ms] or [+5.018%; +7.554%]

@watson watson marked this pull request as ready for review March 17, 2025 10:26
@watson watson requested review from a team as code owners March 17, 2025 10:26
watson
watson commented Mar 24, 2025
View reviewed changes
packages/datadog-plugin-express/src/code_origin.js
this.addSub('apm:express:route:added', ({ topOfStackFunc, layer }) => {
if (Object.hasOwn(layer, kCodeOriginForSpansTagsSym)) return

layer[kCodeOriginForSpansTagsSym] = entryTags(topOfStackFunc, 1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing this on a real life Express app, I found that the top frame this 1 is skipping (shimmer.wrapFunction) isn't always present, in which case the code is skipping the stack frame we actually care about.

More research is needed to make this code more resilient. However, I'll mark this PR as "Draft" and table this work to focus on other more pressing issues.

@watson watson marked this pull request as draft March 24, 2025 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylfin tylfin tylfin approved these changes

Assignees

@watson watson

Labels
semver-minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@watson @tylfin