Skip to content

Double-check for handler mapping in MetricsFilter #7649

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

Closed
OrangeDog opened this issue Dec 14, 2016 · 1 comment
Closed

Double-check for handler mapping in MetricsFilter #7649

OrangeDog opened this issue Dec 14, 2016 · 1 comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@OrangeDog
Copy link
Contributor

If any request is dealt with by a filter (e.g. something in the security chain) then it appears in the metrics as unmapped. This makes it hard to see, for example, which endpoints clients keep failing to authenticate with (all of them, or one in particular), or the total number of requests made to an endpoint.

One solution I can see is to inject a HandlerMapping into the filter and add this at the top of getFinalStatus:

try {
    handlerMapping.getHandler(request);
} catch (Exception ignored) { }

This then adds the appropriate attribute if the request would have been mapped to something before a filter dealt with it. As this method is private, I cannot easily just replace this behaviour.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 14, 2016
@wilkinsona
Copy link
Member

Injecting a HandlerMapping would further couple MetricsFilter to Spring MVC and we actually want to move things in the other direction (see #7505). Furthermore, if a filter has rejected a request and prevented further processing of the request from being performed, I don't think it would be appropriate for MetricsFilter to circumvent that out of the box.

The change in #7505 will see MetricsFilter defining its own request attribute that's used to determine the metric name based on a mapping, in the meantime it uses Spring MVC's HandlerMapping.BEST_MATCHING_PATTERN. If you want the behaviour that you've described, you could add your own, appropriately ordered Filter that sets this attribute before the filter chain unwinds back to the metrics filter.

@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

3 participants