Add outcome tag to Web MVC and reactive metrics #14486
Conversation
|
@michaelmcfadyen Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@michaelmcfadyen Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
michaelmcfadyen
force-pushed
the
master
branch
from
4ab3d70 to
10d3c81
Compare
wilkinsona
added
type: enhancement
|
It would be great to give control to developer to be able to override the default bucketing strategy in someway. In some of the APIs we have, 404 is considered to be a successful outcome. |
| String outcome; | ||
| if (status.is1xxInformational() || status.is2xxSuccessful() | ||
| || status.is3xxRedirection()) { | ||
| outcome = "SUCCESS"; |
There was a problem hiding this comment.
These should be constant values or an enum since they are used in multiple places
There was a problem hiding this comment.
Should these outcome values be treated the same as OUTCOME_UNKNOWN and made into a constant value to avoid the creation for each request?
There was a problem hiding this comment.
These should be constant values or an enum since they are used in multiple places
Perhaps. In terms of main code, they're used once on the MVC side and once on the WebFlux side. Sometimes duplication is preferable to creating an artificial construct that would allow them to be shared.
There was a problem hiding this comment.
Should these outcome values be treated the same as OUTCOME_UNKNOWN and made into a constant value to avoid the creation for each request?
Yes, I think so. While the strings themselves aren't a problem, it would be better to avoid creating a new Tag each time.
@michaelmcfadyen Thank you for the pull request. Would you like to make this amendment?
There was a problem hiding this comment.
sure, will update the PR later today
There was a problem hiding this comment.
updated PR to use constants for each outcome tag
You can override how the outcome tag is created by providing your own implementation of |
michaelmcfadyen
force-pushed
the
master
branch
from
10d3c81 to
96f9aeb
Compare
|
@wilkinsona I am slightly confused by the build failure as the test failures don't seem to have any relevance to the code changes in my PR. Can you advise? |
|
@michaelmcfadyen I am a bit confused by them too. I agree that they're unrelated to these changes. Please just ignore them. I think we have all we need now to be able to merge this. Thanks again for the PR. |
* gh-14486: Polish "Add outcome tag to MVC and WebFlux HTTP request metrics" Add outcome tag to MVC and WebFlux HTTP request metrics
|
@michaelmcfadyen Thank you very much for making your first contribution to Spring Boot. The proposed changes have now been merged into master. I also applied a little bit of polish, the main parts of which were to update the documentation and to divide the success outcome up to separate out informational and redirection. |
wilkinsona
changed the title
5 participants
Add a new tag "outcome" to web mvc metrics for both servlet and reactive models. The outcome tag can have 4 possible values based on the http status code of the response.
1xx, 2xx, 3xx -> SUCCESS
4xx -> CLIENT_ERROR
5xx -> SERVER_ERROR
no status code -> UNKNOWN
Related Issue: #13801