Add HTTP response to HttpServerMetricsTagsContributor.Context #45744
Conversation
|
/cc @brunobat (micrometer), @ebullient (micrometer) |
|
🙏🏽 |
| @@ -20,5 +21,7 @@ public interface HttpServerMetricsTagsContributor { | |||
|
|
|||
| interface Context { | |||
| HttpServerRequest request(); | |||
|
|
|||
| HttpResponse response(); | |||
There was a problem hiding this comment.
I have serious doubts about this. I think we are giving users a way to shoot their feet.
This can explode the cardinality and it should be accompanied by a documentation warning.
What do you think @ebullient ?
There was a problem hiding this comment.
How would they shoot theirselves in the foot more than what we already give them?
There was a problem hiding this comment.
Data from the request can also explode the cardinality, yet it is included. I've already seen how improper usage of request headers explode the cardinality.
People need to be careful how they use the data from the request and response, so I think a warning in the documention would be a good idea 🙂.
There was a problem hiding this comment.
so I think a warning in the documention would be a good idea 🙂.
Definitely +1 for this.
There was a problem hiding this comment.
We need to think in a broad case.
Server side will not have control over the response nor the different amount of values they are getting.
To be used safely, users should not just grab a field from the response but map it to a finite amount of values they control, otherwise there will be a cardinality explosion and in some cases OoM.
This is, most likely for debugging purposes and is the type of thing that should be placed on the request's trace, not metrics.
There was a problem hiding this comment.
But if we document it, I'm fine.
There was a problem hiding this comment.
Server side will not have control over the response
Could you clarify that? I would think you have more control on the response than on the request so I might be missing something?
There was a problem hiding this comment.
Server side will not have control over the response
Could you clarify that? I would think you have more control on the response than on the request so I might be missing something?
From the metrics system perspective there is not much control.
This comment has been minimized.
This comment has been minimized.
This is useful for creating tags based on HTTP response headers for example Closes: quarkusio#45717
|
🙏🏽 |
|
🙈 The PR is closed and the preview is expired. |
Status for workflow
|
Status for workflow
|
This is useful for creating tags based on HTTP response headers for example