Instrument webmvc #27936
Instrument webmvc #27936
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| @@ -121,11 +119,7 @@ private TimingContext startAndAttachTimingContext(HttpServletRequest request) { | |||
| } | |||
|
|
|||
| private Throwable fetchException(HttpServletRequest request) { | |||
| Throwable exception = (Throwable) request.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE); | |||
There was a problem hiding this comment.
What should we do with this?
jonatan-ivanov
force-pushed
the
instrumentation-webmvc
branch
from
1da3cac
to
6b8245f
Compare
(as-is to see changes in subsequent commits)
- Fix imports - Use Outcome from Micrometer instead of Boot - ErrorAttributes.ERROR_ATTRIBUTE is not used - Use stop(Timer.Builder) instead of the deprecated stop(Timer) - Fix license - Fix @SInCE
jonatan-ivanov
force-pushed
the
instrumentation-webmvc
branch
from
6b8245f
to
bc649af
Compare
| * @since 6.0.0 | ||
| */ | ||
| @FunctionalInterface | ||
| public interface AutoTimer { |
There was a problem hiding this comment.
Would this class be better in io.micrometer? Other than CollectionUtils there's no direct Spring connection.
There was a problem hiding this comment.
This is something we need to figure out. I think based on what we have right now, it would be but we need to check if we can/want to provide a different disable mechanism (for Meters this mechanism is provided by MeterFilters but that won't work in this use-case).
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public final class HttpServletRequestWrapper implements HttpServerRequest { |
There was a problem hiding this comment.
I wonder if this can be package private? It looks like it's not pluggable. Possible also worth renaming since it's specifically for MVC.
There was a problem hiding this comment.
Couple of related Boot issues if we're redesigning this:
spring-projects/spring-boot#29303
spring-projects/spring-boot#13064
| * | ||
| * @since 6.0.0 | ||
| */ | ||
| public final class HttpServletResponseWrapper implements HttpServerResponse { |
There was a problem hiding this comment.
As above. I wonder if this can be package private.
| * @param request the request to wrap | ||
| * @return an {@link HttpServletRequestWrapper} instance that uses the provided request. | ||
| */ | ||
| static HttpServletRequestWrapper wrap(HttpServletRequest request) { |
There was a problem hiding this comment.
Not sure about framework, but boot often keeps static factory methods like this at the bottom of the file.
|
Superseded by #28880 |
bclozel
added
in: web
Questions/todo items:
methodandhttp.method)