Skip to content

Remove AppSec::instrumentation::Gateway#pushed?#5487

Merged
y9v merged 9 commits intomasterfrom
appsec-remove-pushed-events
Mar 23, 2026
Merged

Remove AppSec::instrumentation::Gateway#pushed?#5487
y9v merged 9 commits intomasterfrom
appsec-remove-pushed-events

Conversation

@y9v
Copy link
Copy Markdown
Member

@y9v y9v commented Mar 20, 2026

What does this PR do?
Moves identity event tracking from the global Gateway singleton to the per-request AppSec Context. Removes Gateway#pushed? and @pushed_events entirely.

The fix adds an identity_event_collected flag to the per-request Context#state hash, set by the identity watchers (watch_user_id, watch_user_login), and read by watch_request_finish. This correctly scopes header collection to the request that triggered the identity event.

Motivation:
Gateway#pushed? tracked events on a process-lifetime singleton that was never reset between requests. Once any request pushed an identity event (e.g. a Devise login), pushed? returned true for all subsequent requests forever. This caused watch_request_finish to collect identity headers on every request after the first login, rather than only on requests where an identity event actually occurred.

Change log entry
None.

Additional Notes:
Related to #5466

How to test the change?
CI and manual testing

@y9v y9v self-assigned this Mar 20, 2026
@y9v y9v requested review from a team as code owners March 20, 2026 16:54
@github-actions github-actions bot added integrations Involves tracing integrations appsec Application Security monitoring product labels Mar 20, 2026
@datadog-prod-us1-5
Copy link
Copy Markdown

datadog-prod-us1-5 bot commented Mar 20, 2026

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 95.13% (+0.00%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 39f93b3 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@pr-commenter
Copy link
Copy Markdown

pr-commenter bot commented Mar 20, 2026

Benchmarks

Benchmark execution time: 2026-03-23 16:36:12

Comparing candidate commit 39f93b3 in PR branch appsec-remove-pushed-events with baseline commit 8d5d914 in branch master.

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

@y9v y9v requested a review from Strech March 23, 2026 13:21
@y9v y9v force-pushed the appsec-remove-pushed-events branch from 6819dea to 20ef4b8 Compare March 23, 2026 15:05
Copy link
Copy Markdown
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.

👍🏼

@y9v y9v force-pushed the appsec-remove-pushed-events branch from 4489ca1 to 39f93b3 Compare March 23, 2026 16:11
@y9v y9v merged commit cd078c7 into master Mar 23, 2026
630 checks passed
@y9v y9v deleted the appsec-remove-pushed-events branch March 23, 2026 16:40
@github-actions github-actions bot added this to the 2.31.0 milestone Mar 23, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants