Skip to content
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.

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

WIP api security sampling #4766

Draft
wants to merge 7 commits into
base: api-security-sampling
Choose a base branch
from

Conversation

iunanua
Copy link
Contributor

@iunanua iunanua commented Oct 9, 2024

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

@iunanua iunanua changed the base branch from api-security-sampling to master October 9, 2024 13:02
@iunanua iunanua changed the base branch from master to api-security-sampling October 9, 2024 13:02
@iunanua iunanua changed the title api security sampling WIP api security sampling Oct 9, 2024
@pr-commenter
Copy link

pr-commenter bot commented Oct 9, 2024

Benchmarks

Benchmark execution time: 2024-10-09 13:22:36

Comparing candidate commit 832800a in PR branch igor/api-security-sampling-ilyas with baseline commit bba5f3d in branch api-security-sampling.

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

@iunanua iunanua force-pushed the igor/api-security-sampling-ilyas branch from 448d9dd to 832800a Compare October 9, 2024 13:13
@@ -70,7 +65,7 @@ function isSampled (req, res) {
function computeKey (req, res) {
const route = req.route?.path || req.url
const method = req.method.toLowerCase()
const statusCode = res.statusCode
const statusCode = res.statusCode === 304 ? 200 : res.statusCode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is funny because sometimes, for the same request, status code is 200 in onResponseBody but 304 in incomingHttpEndTranslator. It depends on when you are trying to get it.

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.

2 participants