Uri tag replaced with REDIRECTION on all HTTP redirect responses with Jersey server

[tinolazreg]

Please describe the feature request.
It would be nice to be able to choose if you want the URI tag to be replaced with REDIRECTION or not. In many cases it is interesting to get metrics on redirects. "304 Not modified" for example is interesting to track, same for "303 See other", how long those the server actually use to fulfil the request.

This line is the reason for the uri tag being set to REDIRECTION: https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jersey/server/JerseyTags.java#L91.

Rationale
Uri tag replacement for redirect responses should be configureable. In many cases it is interesting to have metrics on redirections.

[shakuzen]

Thank you for the report.

Uri tag replacement for redirect responses should be configureable.

It is already configurable because you provide a JerseyTagsProvider instance. You don't have to use the DefaultJerseyTagsProvider. We could consider providing different behavior for specific redirection status codes in DefaultJerseyTagsProvider, but if you want to customize it for just your applications, that is already possible. Is that sufficient for your use case or would you like us to consider changing DefaultJerseyTagsProvider or am I missing something?

[tinolazreg]

Thank you for the report.

Uri tag replacement for redirect responses should be configureable.

It is already configurable because you provide a JerseyTagsProvider instance. You don't have to use the DefaultJerseyTagsProvider. We could consider providing different behavior for specific redirection status codes in DefaultJerseyTagsProvider, but if you want to customize it for just your applications, that is already possible. Is that sufficient for your use case or would you like us to consider changing DefaultJerseyTagsProvider or am I missing something?

That is what we have done, so it is solvable on our side, but would be nice to be able to use the JerseyTags.uri() method, so we don't have to handle that ourself. I just thought it seemed strange that the default URI jersey tag logic sets the URI to "REDIRECTION", when we already have the "OUTCOME"-tag that should give us that information.

[shakuzen]

I just thought it seemed strange that the default URI jersey tag logic sets the URI to "REDIRECTION", when we already have the "OUTCOME"-tag that should give us that information.

The point of setting the uri tag to REDIRECTION is to avoid a tag cardinality explosion that could be caused by an unbounded number of unique URIs that result in redirection and are a result of what is requested by a client (potentially an untrusted user). It's a security issue to allow a client to cause unbounded increases in memory in your application and increases in network traffic to publish metrics and increase in storage required for retaining those metrics. This is essentially the same reason unmapped routes that result in 404 have a static NOT_FOUND value for the uri tag. Only if we know reliably that the uri value is not unbounded client input, we can safely tag it.

It was probably out of an abundance of caution that we decided to treat all redirect status codes as unsafe to tag the URI. We can revisit that decision for specific redirect status codes, but it is something we would probably want to standardize across HTTP server instrumentation.

[tinolazreg]

I just thought it seemed strange that the default URI jersey tag logic sets the URI to "REDIRECTION", when we already have the "OUTCOME"-tag that should give us that information.

The point of setting the uri tag to REDIRECTION is to avoid a tag cardinality explosion that could be caused by an unbounded number of unique URIs that result in redirection and are a result of what is requested by a client (potentially an untrusted user).

I understand the caution about tag cardinality explosion, but will that really happen when you are using URI templates? https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jersey/server/JerseyTags.java#L110

[shakuzen]

will that really happen when you are using URI templates?

As long as there is a matched resource, that assumption may hold true for Jersey server. I think filters could cause a response before the resource is matched, though. We added a check for 404 responses to tag the URI template if a resource was matched in #2587.

[tinolazreg]

will that really happen when you are using URI templates?

As long as there is a matched resource, that assumption may hold true for Jersey server. I think filters could cause a response before the resource is matched, though. We added a check for 404 responses to tag the URI template if a resource was matched in #2587.

Would this work: #3332?
Basically just adding an extra check for matched resources, like what is done for 404 responses.

Still not sure if it will ever happen that you have a redirect response without a matching URI template, but it should at least avoid tag cardinality explosion if it can happen.

What do you think?

[shakuzen]

Would this work: #3332?

Yes, I think so.
I was going to try to tackle this for all HTTP server instrumentation, but let's focus this issue on the Jersey server instrumentation, which I think is your primary concern. When we formalize the HTTP server instrumentation TCK, we should include a test for this kind of behavior.

[shakuzen]

I've merged your fix into the 1.8.x branch. Thank you for reporting and sending a fix. The fix will be in upcoming releases 1.8.10, 1.9.4, and 1.10.0-M5.