From 4c07176f274b2d40a36556afc9b3b1a709f14b3b Mon Sep 17 00:00:00 2001 From: Mark Miller Date: Wed, 23 Aug 2023 13:34:45 -0700 Subject: [PATCH] FEAT: Jersey: check impl method for annotations before interface method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Metrics annotations (@Timed, @Metered, etc) don’t allow annotating a resource on the *implementing* method, only on the *defining* (interface) method (or more accurately, the method corresponding to the @Path definition, more or less). This patch enhances dropwizard-metrics to extract metrics annotations more intelligently, preferring the annotation on the implementation (and falling back to the interface / definitionMethod if the implementation annotation is absent). --- ...ntedResourceMethodApplicationListener.java | 27 ++++++++++++++++--- ...ntedResourceMethodApplicationListener.java | 27 ++++++++++++++++--- ...ntedResourceMethodApplicationListener.java | 27 ++++++++++++++++--- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java b/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java index 7697e11597..056c33f91a 100644 --- a/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java +++ b/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java @@ -405,6 +405,25 @@ private T getClassLevelAnnotation(final Resource resource return annotation; } + /** + * @param method the method to examine for the annotation + * @param annotation the annotation class (e.g. Timed, Metered, etc) + * @return the annotation (of type {@code T})from the given method, or null. + * @param the type of annotation to return + */ + private static T findAnnotation(final ResourceMethod method, final Class annotation) { + // In code-generation situations, developers define their API (via OpenAPI or gRPC or ___), and JAX-RS + // interfaces are generated for the defined services (which define @Path, @Produces, etc). + // The developer is then responsible for implementing those service interfaces. + // For fetching metrics annotations, we presume that any annotation on the implementing method should + // take precedence over the interface method, as the developer controls the former more directly. + T result = method.getInvocable().getHandlingMethod().getAnnotation(annotation); + if (result == null) { + result = method.getInvocable().getDefinitionMethod().getAnnotation(annotation); + } + return result; + } + private void registerTimedAnnotations(final ResourceMethod method, final Timed classLevelTimed) { final Method definitionMethod = method.getInvocable().getDefinitionMethod(); if (classLevelTimed != null) { @@ -412,7 +431,7 @@ private void registerTimedAnnotations(final ResourceMethod method, final Timed c return; } - final Timed annotation = definitionMethod.getAnnotation(Timed.class); + final Timed annotation = findAnnotation(method, Timed.class); if (annotation != null) { registerTimers(method, definitionMethod, annotation); } @@ -434,7 +453,7 @@ private void registerMeteredAnnotations(final ResourceMethod method, final Meter meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, classLevelMetered)); return; } - final Metered annotation = definitionMethod.getAnnotation(Metered.class); + final Metered annotation = findAnnotation(method, Metered.class); if (annotation != null) { meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, annotation)); @@ -448,7 +467,7 @@ private void registerExceptionMeteredAnnotations(final ResourceMethod method, fi exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, classLevelExceptionMetered)); return; } - final ExceptionMetered annotation = definitionMethod.getAnnotation(ExceptionMetered.class); + final ExceptionMetered annotation = findAnnotation(method, ExceptionMetered.class); if (annotation != null) { exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, annotation)); @@ -462,7 +481,7 @@ private void registerResponseMeteredAnnotations(final ResourceMethod method, fin responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, classLevelResponseMetered)); return; } - final ResponseMetered annotation = definitionMethod.getAnnotation(ResponseMetered.class); + final ResponseMetered annotation = findAnnotation(method, ResponseMetered.class); if (annotation != null) { responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, annotation)); diff --git a/metrics-jersey3/src/main/java/com/codahale/metrics/jersey3/InstrumentedResourceMethodApplicationListener.java b/metrics-jersey3/src/main/java/com/codahale/metrics/jersey3/InstrumentedResourceMethodApplicationListener.java index 0e0b39af04..178f6c1977 100644 --- a/metrics-jersey3/src/main/java/com/codahale/metrics/jersey3/InstrumentedResourceMethodApplicationListener.java +++ b/metrics-jersey3/src/main/java/com/codahale/metrics/jersey3/InstrumentedResourceMethodApplicationListener.java @@ -406,6 +406,25 @@ private T getClassLevelAnnotation(final Resource resource return annotation; } + /** + * @param method the method to examine for the annotation + * @param annotation the annotation class (e.g. Timed, Metered, etc) + * @return the annotation (of type {@code T})from the given method, or null. + * @param the type of annotation to return + */ + private static T findAnnotation(final ResourceMethod method, final Class annotation) { + // In code-generation situations, developers define their API (via OpenAPI or gRPC or ___), and JAX-RS + // interfaces are generated for the defined services (which define @Path, @Produces, etc). + // The developer is then responsible for implementing those service interfaces. + // For fetching metrics annotations, we presume that any annotation on the implementing method should + // take precedence over the interface method, as the developer controls the former more directly. + T result = method.getInvocable().getHandlingMethod().getAnnotation(annotation); + if (result == null) { + result = method.getInvocable().getDefinitionMethod().getAnnotation(annotation); + } + return result; + } + private void registerTimedAnnotations(final ResourceMethod method, final Timed classLevelTimed) { final Method definitionMethod = method.getInvocable().getDefinitionMethod(); if (classLevelTimed != null) { @@ -413,7 +432,7 @@ private void registerTimedAnnotations(final ResourceMethod method, final Timed c return; } - final Timed annotation = definitionMethod.getAnnotation(Timed.class); + final Timed annotation = findAnnotation(method, Timed.class); if (annotation != null) { registerTimers(method, definitionMethod, annotation); } @@ -435,7 +454,7 @@ private void registerMeteredAnnotations(final ResourceMethod method, final Meter meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, classLevelMetered)); return; } - final Metered annotation = definitionMethod.getAnnotation(Metered.class); + final Metered annotation = findAnnotation(method, Metered.class); if (annotation != null) { meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, annotation)); @@ -449,7 +468,7 @@ private void registerExceptionMeteredAnnotations(final ResourceMethod method, fi exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, classLevelExceptionMetered)); return; } - final ExceptionMetered annotation = definitionMethod.getAnnotation(ExceptionMetered.class); + final ExceptionMetered annotation = findAnnotation(method, ExceptionMetered.class); if (annotation != null) { exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, annotation)); @@ -463,7 +482,7 @@ private void registerResponseMeteredAnnotations(final ResourceMethod method, fin responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, classLevelResponseMetered)); return; } - final ResponseMetered annotation = definitionMethod.getAnnotation(ResponseMetered.class); + final ResponseMetered annotation = findAnnotation(method, ResponseMetered.class); if (annotation != null) { responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, annotation)); diff --git a/metrics-jersey31/src/main/java/io/dropwizard/metrics/jersey31/InstrumentedResourceMethodApplicationListener.java b/metrics-jersey31/src/main/java/io/dropwizard/metrics/jersey31/InstrumentedResourceMethodApplicationListener.java index eeaf808151..c42fef2240 100644 --- a/metrics-jersey31/src/main/java/io/dropwizard/metrics/jersey31/InstrumentedResourceMethodApplicationListener.java +++ b/metrics-jersey31/src/main/java/io/dropwizard/metrics/jersey31/InstrumentedResourceMethodApplicationListener.java @@ -405,6 +405,25 @@ private T getClassLevelAnnotation(final Resource resource return annotation; } + /** + * @param method the method to examine for the annotation + * @param annotation the annotation class (e.g. Timed, Metered, etc) + * @return the annotation (of type {@code T})from the given method, or null. + * @param the type of annotation to return + */ + private static T findAnnotation(final ResourceMethod method, final Class annotation) { + // In code-generation situations, developers define their API (via OpenAPI or gRPC or ___), and JAX-RS + // interfaces are generated for the defined services (which define @Path, @Produces, etc). + // The developer is then responsible for implementing those service interfaces. + // For fetching metrics annotations, we presume that any annotation on the implementing method should + // take precedence over the interface method, as the developer controls the former more directly. + T result = method.getInvocable().getHandlingMethod().getAnnotation(annotation); + if (result == null) { + result = method.getInvocable().getDefinitionMethod().getAnnotation(annotation); + } + return result; + } + private void registerTimedAnnotations(final ResourceMethod method, final Timed classLevelTimed) { final Method definitionMethod = method.getInvocable().getDefinitionMethod(); if (classLevelTimed != null) { @@ -412,7 +431,7 @@ private void registerTimedAnnotations(final ResourceMethod method, final Timed c return; } - final Timed annotation = definitionMethod.getAnnotation(Timed.class); + final Timed annotation = findAnnotation(method, Timed.class); if (annotation != null) { registerTimers(method, definitionMethod, annotation); } @@ -434,7 +453,7 @@ private void registerMeteredAnnotations(final ResourceMethod method, final Meter meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, classLevelMetered)); return; } - final Metered annotation = definitionMethod.getAnnotation(Metered.class); + final Metered annotation = findAnnotation(method, Metered.class); if (annotation != null) { meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, annotation)); @@ -448,7 +467,7 @@ private void registerExceptionMeteredAnnotations(final ResourceMethod method, fi exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, classLevelExceptionMetered)); return; } - final ExceptionMetered annotation = definitionMethod.getAnnotation(ExceptionMetered.class); + final ExceptionMetered annotation = findAnnotation(method, ExceptionMetered.class); if (annotation != null) { exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, annotation)); @@ -462,7 +481,7 @@ private void registerResponseMeteredAnnotations(final ResourceMethod method, fin responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, classLevelResponseMetered)); return; } - final ResponseMetered annotation = definitionMethod.getAnnotation(ResponseMetered.class); + final ResponseMetered annotation = findAnnotation(method, ResponseMetered.class); if (annotation != null) { responseMeters.putIfAbsent(definitionMethod, new ResponseMeterMetric(metrics, method, annotation));