Skip to content

Conversation

crysmags
Copy link
Collaborator

@crysmags crysmags commented Oct 2, 2025

What does this PR do?

This pull request closes #6575

Motivation

Current implementation of the ws library uses the full req.url which can potentially expose query params, this pr will instead uses the req route instead. There are also the addition of units tests to test the resource is set properly.

@crysmags crysmags requested review from a team as code owners October 2, 2025 19:54
Copy link

github-actions bot commented Oct 2, 2025

Overall package size

Self size: 12.55 MB
Deduped: 112.77 MB
No deduping: 113.17 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.2.1 | 20.64 MB | 20.65 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.10.0 | 9.91 MB | 10.3 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.73 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.4 | 123.18 kB | 851.76 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 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 | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 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 | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |

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

Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.98%. Comparing base (18e5ccc) to head (5ad4afc).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6587   +/-   ##
=======================================
  Coverage   83.98%   83.98%           
=======================================
  Files         488      489    +1     
  Lines       20452    20454    +2     
=======================================
+ Hits        17176    17178    +2     
  Misses       3276     3276           

☔ 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.

This comment has been minimized.

@crysmags crysmags force-pushed the crysmags/ws-route-update branch from ed198a7 to 5ad4afc Compare October 2, 2025 21:23
@pr-commenter
Copy link

pr-commenter bot commented Oct 2, 2025

Benchmarks

Benchmark execution time: 2025-10-02 21:32:00

Comparing candidate commit 5ad4afc in PR branch crysmags/ws-route-update with baseline commit 18e5ccc in branch master.

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

@chiawendt
Copy link

While this is not exactly ”http route", which normalizes path parameters, this is good enough for me and a valuable improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: use http route name not path for websocket span resource name
2 participants