Skip to content
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

Current url statistics implementation for spring web can't properly collect failures #9357

Closed
ga-ram opened this issue Nov 1, 2022 · 0 comments · Fixed by #9358
Closed
Assignees
Milestone

Comments

@ga-ram
Copy link
Contributor

ga-ram commented Nov 1, 2022

Current url statistics implementation for spring web uses request attribute org.springframework.web.servlet.HandlerMapping.bestMatchingPattern when collecting urls.

When a request fails, this value changes to /error or any other url values set in spring property server.error.path.
This may lead to an illusion that there is no failed requests in each url.

e.g.) when a request with initial bestMatchingPattern /randomResponseTime/** fails, /top50 response will be:

{"uri":"/randomResponseTime/**","totalCount":2502.0,"failureCount":0.0,"maxTimeMs":13404.0,"avgTimeMs":5060.744204636291,"version":0}
{"uri":"/error","totalCount":608.0,"failureCount":9.0,"maxTimeMs":10012.0,"avgTimeMs":5039.962171052632,"version":0}

Internally spring redirects failed requests to /error, so it may be correct but still misleading.

I will add another interceptor to preserve initial bestMatchingPattern to another request attribute for Pinpoint to use when collecting url statistics for spring web.

@ga-ram ga-ram changed the title Current url statistics implementation for spring web can't collect properly failures Current url statistics implementation for spring web can't properly collect failures Nov 2, 2022
@ga-ram ga-ram self-assigned this Nov 2, 2022
@ga-ram ga-ram added this to the 2.5.0 milestone Nov 2, 2022
ga-ram added a commit to ga-ram/pinpoint that referenced this issue Nov 2, 2022
ga-ram added a commit to ga-ram/pinpoint that referenced this issue Nov 2, 2022
ga-ram added a commit to ga-ram/pinpoint that referenced this issue Nov 2, 2022
ga-ram added a commit to ga-ram/pinpoint that referenced this issue Nov 3, 2022
ga-ram added a commit that referenced this issue Nov 3, 2022
BillionaireDY pushed a commit to BillionaireDY/pinpoint that referenced this issue Dec 29, 2022
BillionaireDY pushed a commit to BillionaireDY/pinpoint that referenced this issue Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant