Skip to content

Conversation

@y9v
Copy link
Member

@y9v y9v commented Oct 17, 2025

What does this PR do?
This PR removes the behavior that used the request path as a fallback when computing the route for Rack applications mounted inside Rails.

Motivation:
The current approach to setting the http.route tag for mounted Rack applications causes issues for the API Catalog. Specifically, when a mounted Rack application includes parameters in its request path, it can lead to an unbounded number of reported routes.

This issue can occur in the following cases:

  • Mounted applications such as the Sidekiq web UI, which is a Rack app with a simple router.
  • Mounted Sinatra and Grape applications, when the Datadog tracer is configured to instrument Rails but not Sinatra or Grape directly.

Before the fix route for an embedded Rack app would look like /path/for/mounted/app/users/1, after the fix it would be /path/for/mounted/app.

Change log entry
Yes. Fix http.route tag for Rack applications mounted inside Rails.

Additional Notes:
APPSEC-59663

How to test the change?
CI and app generator.

@y9v y9v self-assigned this Oct 17, 2025
@y9v y9v requested review from a team as code owners October 17, 2025 12:58
@y9v y9v requested a review from mabdinur October 17, 2025 12:58
@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Oct 17, 2025
@y9v y9v requested review from Strech and mabdinur and removed request for mabdinur October 17, 2025 13:01
Copy link
Member

@Strech Strech left a comment

Choose a reason for hiding this comment

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

🔥 Amazing! Well done!

@y9v y9v force-pushed the fix-http-route-tagging-for-mounted-rack-apps branch from 387a707 to 433b9c9 Compare October 21, 2025 11:26
@datadog-datadog-prod-us1
Copy link
Contributor

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
Patch Coverage: 100.00%
Total Coverage: 98.56% (+0.01%)

View detailed report

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 433b9c9 | Docs | Was this helpful? Give us feedback!

@pr-commenter
Copy link

pr-commenter bot commented Oct 21, 2025

Benchmarks

Benchmark execution time: 2025-10-21 11:56:56

Comparing candidate commit 433b9c9 in PR branch fix-http-route-tagging-for-mounted-rack-apps with baseline commit f8dfc57 in branch master.

Found 1 performance improvements and 1 performance regressions! Performance is the same for 42 metrics, 2 unstable metrics.

scenario:error - without error tracking, with_error=false

  • 🟥 throughput [-678.981op/s; -630.264op/s] or [-5.507%; -5.112%]

scenario:profiling - Allocations (baseline)

  • 🟩 throughput [+290078.822op/s; +303636.330op/s] or [+5.806%; +6.078%]

Copy link
Contributor

@vpellan vpellan left a comment

Choose a reason for hiding this comment

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

LGTM!

@y9v y9v merged commit 408c712 into master Oct 21, 2025
406 checks passed
@y9v y9v deleted the fix-http-route-tagging-for-mounted-rack-apps branch October 21, 2025 12:23
@github-actions github-actions bot added this to the 2.23.0 milestone Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrations Involves tracing integrations tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants