Add support to ignore request in metrics #6177
Conversation
The committers listed above are authorized under a signed CLA. |
|
Welcome @jan-stanek! It looks like this is your first PR to knative/docs 🎉 |
knative-prow
bot
added
the
size/XS
✅ Deploy Preview for knative ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
jan-stanek
force-pushed
the
ignore-metrics
branch
from
389810a to
74cf40b
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jan-stanek The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -19,6 +19,9 @@ Requests endpoint. | |||
| !!! note | |||
| The `revision_queue_depth` metric will be exported only if the revision concurrency hard limit is set to a value greater than 1. | |||
|
|
|||
| !!! note | |||
| To exclude request from metrics (e.g. health check requests), set the `K-Ignore-Metrics` header to non-empty value. | |||
There was a problem hiding this comment.
What are the security implications of this header? I'm concerned that someone could use this header to sneak in a DDoS attack that wouldn't show up on monitoring.
There was a problem hiding this comment.
Good point, would it be better to have possibility to specify additional label for metrics?
There was a problem hiding this comment.
I think a single additional label would be safe; extracting a label into a header has a different DoS risk by expanding the cardinality of the metric.
There was a problem hiding this comment.
I think this feature needs to go through a feature track document to outline possible approaches and to confirm the problem we're trying to address. In the serving repo all this work seems to go towards external health checks but we need to outline the behaviour when we've scaled to zero etc.
Details on the feature track process is here:
https://github.com/knative/community/blob/main/mechanics/FEATURE-TRACKS.md
There was a problem hiding this comment.
I think this can be fairly short, but it would be good to explain why external health checks should be treated differently from other traffic -- I'm assuming this is because the artificial traffic would perturb dashboards and other monitoring, but I don't have full context.
If you have external health checks that don't execute the application, it seems like you could just deploy (for example) a dummy Revision and use tag-routed headers or hostnames for the external probe. If the health checks do execute some part of the application, it seems like you'd want to scale them in the same way as other requests (which may mean you always have one instance live).
|
/hold |
knative-prow
bot
added
the
do-not-merge/hold
Proposed Changes
K-Ignore-Metrics