Skip to content

Conversation

@y9v
Copy link
Member

@y9v y9v commented Oct 29, 2025

What does this PR do?
This PR changes AppSec::APISecuity::RouteExtractor to infer the route if it cannot be extracted, instead of falling back to request path. This route is then used for AppSec API security sampler.

Motivation:
We want to have more accurate sampling for requests to barebones Rack applications, or Rack-based frameworks that we don't instrument directly.

Change log entry
None. This is an internal change.

Additional Notes:
APPSEC-59868

How to test the change?
CI.

@y9v y9v self-assigned this Oct 29, 2025
@y9v y9v requested review from a team as code owners October 29, 2025 14:33
@y9v y9v requested a review from mabdinur October 29, 2025 14:33
@github-actions github-actions bot added integrations Involves tracing integrations appsec Application Security monitoring product tracing labels Oct 29, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Oct 29, 2025

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
Patch Coverage: 104.55%
Total Coverage: 98.45% (-0.11%)

View detailed report

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

@pr-commenter
Copy link

pr-commenter bot commented Oct 29, 2025

Benchmarks

Benchmark execution time: 2025-11-03 13:14:47

Comparing candidate commit f400f71 in PR branch appsec-security-sampling-with-inferred-route with baseline commit f060ebf in branch master.

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

scenario:profiling - Allocations (baseline)

  • 🟩 throughput [+269529.109op/s; +282408.131op/s] or [+5.391%; +5.649%]

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.

I think we should change the route inference interface to encapsulate some additional logic which now exposed to all calls to this module

@github-actions
Copy link

github-actions bot commented Oct 30, 2025

Typing analysis

Note: Ignored files are excluded from the next sections.

Untyped methods

This PR introduces 1 partially typed method.

Partially typed methods (+1-0)Introduced:
sig/datadog/tracing/contrib/rack/route_inference.rbs:16
└── def self?.read_or_infer: (Hash[::String, untyped] request_env) -> ::String?

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept to the end of the line to remove it from the stats.

@y9v y9v requested a review from Strech October 30, 2025 17:09
@y9v y9v requested a review from a team as a code owner November 3, 2025 10:12
@y9v y9v force-pushed the appsec-security-sampling-with-inferred-route branch from 2ab8036 to 2cc3588 Compare November 3, 2025 10:18
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.

🌟 Exceptionally well done! 🔥

@y9v y9v merged commit cbbffca into master Nov 3, 2025
557 checks passed
@y9v y9v deleted the appsec-security-sampling-with-inferred-route branch November 3, 2025 14:33
@github-actions github-actions bot added this to the 2.23.0 milestone Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

appsec Application Security monitoring product integrations Involves tracing integrations tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants