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

#859 fix inherited timed annotation #861

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Initial attempt to fix the issue with inherited resource classes when…
… using @timed, @metered, and @ExceptionMetered by using the handler class and method name when registering the methods.
  • Loading branch information
mtakaki committed Aug 31, 2015
commit 2e584482634104829de5e299b4ed901af17aed7b
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import com.codahale.metrics.annotation.ExceptionMetered;
import com.codahale.metrics.annotation.Metered;
import com.codahale.metrics.annotation.Timed;

import org.glassfish.jersey.server.model.Invocable;
import org.glassfish.jersey.server.model.ModelProcessor;
import org.glassfish.jersey.server.model.Resource;
import org.glassfish.jersey.server.model.ResourceMethod;
Expand All @@ -17,6 +19,7 @@

import javax.ws.rs.core.Configuration;
import javax.ws.rs.ext.Provider;

import java.lang.reflect.Method;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand Down Expand Up @@ -64,10 +67,11 @@ private static class ExceptionMeterMetric {
public final Class<? extends Throwable> cause;

public ExceptionMeterMetric(final MetricRegistry registry,
final Class<?> handlerClass,
final ResourceMethod method,
final ExceptionMetered exceptionMetered) {
final String name = chooseName(exceptionMetered.name(),
exceptionMetered.absolute(), method, ExceptionMetered.DEFAULT_NAME_SUFFIX);
exceptionMetered.absolute(), handlerClass, method, ExceptionMetered.DEFAULT_NAME_SUFFIX);
this.meter = registry.meter(name);
this.cause = exceptionMetered.cause();
}
Expand All @@ -85,7 +89,7 @@ public TimerRequestEventListener(final ConcurrentMap<Method, Timer> timers) {
public void onEvent(RequestEvent event) {
if (event.getType() == RequestEvent.Type.RESOURCE_METHOD_START) {
final Timer timer = this.timers.get(event.getUriInfo()
.getMatchedResourceMethod().getInvocable().getDefinitionMethod());
.getMatchedResourceMethod().getInvocable().getHandlingMethod());
if (timer != null) {
this.context = timer.time();
}
Expand Down Expand Up @@ -204,56 +208,64 @@ public RequestEventListener onRequest(final RequestEvent event) {
}

private void registerTimedAnnotations(final ResourceMethod method) {
final Method definitionMethod = method.getInvocable().getDefinitionMethod();
final Timed annotation = definitionMethod.getAnnotation(Timed.class);
final Invocable methodInvocable = method.getInvocable();
final Class<?> handlerClass = methodInvocable.getHandler().getHandlerClass();
final Method handlingMethod = methodInvocable.getHandlingMethod();
final Timed annotation = handlingMethod.getAnnotation(Timed.class);

if (annotation != null) {
timers.putIfAbsent(definitionMethod, timerMetric(this.metrics, method, annotation));
timers.putIfAbsent(handlingMethod, timerMetric(this.metrics, handlerClass, method, annotation));
}
}

private void registerMeteredAnnotations(final ResourceMethod method) {
final Method definitionMethod = method.getInvocable().getDefinitionMethod();
final Metered annotation = definitionMethod.getAnnotation(Metered.class);
final Invocable methodInvocable = method.getInvocable();
final Class<?> handlerClass = methodInvocable.getHandler().getHandlerClass();
final Method handlingMethod = methodInvocable.getHandlingMethod();
final Metered annotation = handlingMethod.getAnnotation(Metered.class);

if (annotation != null) {
meters.putIfAbsent(definitionMethod, meterMetric(metrics, method, annotation));
meters.putIfAbsent(handlingMethod, meterMetric(metrics, handlerClass, method, annotation));
}
}

private void registerExceptionMeteredAnnotations(final ResourceMethod method) {
final Method definitionMethod = method.getInvocable().getDefinitionMethod();
final ExceptionMetered annotation = definitionMethod.getAnnotation(ExceptionMetered.class);
final Invocable methodInvocable = method.getInvocable();
final Class<?> handlerClass = methodInvocable.getHandler().getHandlerClass();
final Method handlingMethod = methodInvocable.getHandlingMethod();
final ExceptionMetered annotation = handlingMethod.getAnnotation(ExceptionMetered.class);

if (annotation != null) {
exceptionMeters.putIfAbsent(definitionMethod, new ExceptionMeterMetric(metrics, method, annotation));
exceptionMeters.putIfAbsent(handlingMethod, new ExceptionMeterMetric(metrics, handlerClass, method, annotation));
}
}

private static Timer timerMetric(final MetricRegistry registry,
final Class<?> handlerClass,
final ResourceMethod method,
final Timed timed) {
final String name = chooseName(timed.name(), timed.absolute(), method);
final String name = chooseName(timed.name(), timed.absolute(), handlerClass, method);
return registry.timer(name);
}

private static Meter meterMetric(final MetricRegistry registry,
final Class<?> handlerClass,
final ResourceMethod method,
final Metered metered) {
final String name = chooseName(metered.name(), metered.absolute(), method);
final String name = chooseName(metered.name(), metered.absolute(), handlerClass, method);
return registry.meter(name);
}

protected static String chooseName(final String explicitName, final boolean absolute, final ResourceMethod method, final String... suffixes) {
protected static String chooseName(final String explicitName, final boolean absolute, final Class<?> handlerClass,
final ResourceMethod method, final String... suffixes) {
if (explicitName != null && !explicitName.isEmpty()) {
if (absolute) {
return explicitName;
}
return name(method.getInvocable().getDefinitionMethod().getDeclaringClass(), explicitName);
return name(handlerClass, explicitName);
}

return name(name(method.getInvocable().getDefinitionMethod().getDeclaringClass(),
method.getInvocable().getDefinitionMethod().getName()),
return name(name(handlerClass, method.getInvocable().getDefinitionMethod().getName()),
suffixes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import com.codahale.metrics.Meter;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.codahale.metrics.jersey2.resources.InstrumentedChildResource;
import com.codahale.metrics.jersey2.resources.InstrumentedParentResource;
import com.codahale.metrics.jersey2.resources.InstrumentedResource;
import com.codahale.metrics.jersey2.resources.InstrumentedSecondChildResource;
import com.codahale.metrics.jersey2.resources.InstrumentedSubResource;
import org.glassfish.jersey.client.ClientResponse;
import org.glassfish.jersey.server.ResourceConfig;
Expand Down Expand Up @@ -41,6 +44,8 @@ protected Application configure() {
ResourceConfig config = new ResourceConfig();
config = config.register(new MetricsFeature(this.registry));
config = config.register(InstrumentedResource.class);
config = config.register(InstrumentedChildResource.class);
config = config.register(InstrumentedSecondChildResource.class);

return config;
}
Expand All @@ -56,6 +61,18 @@ public void timedMethodsAreTimed() {

assertThat(timer.getCount()).isEqualTo(1);
}

@Test
public void childTimedMethodsAreTimed() {
assertThat(target("/child/timed")
.request()
.get(String.class))
.isEqualTo("yay");

final Timer timer = registry.timer(name(InstrumentedParentResource.class, "timed"));

assertThat(timer.getCount()).isEqualTo(1);
}

@Test
public void meteredMethodsAreMetered() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.codahale.metrics.jersey2.resources;

import javax.ws.rs.Path;

@Path("/child")
public class InstrumentedChildResource extends InstrumentedParentResource {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.codahale.metrics.jersey2.resources;

import com.codahale.metrics.annotation.Timed;

import javax.ws.rs.*;
import javax.ws.rs.core.MediaType;

@Produces(MediaType.TEXT_PLAIN)
public class InstrumentedParentResource {
@GET
@Timed
@Path("/timed")
public String timed() {
return "yay";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.codahale.metrics.jersey2.resources;

import javax.ws.rs.Path;

@Path("/secondchild")
public class InstrumentedSecondChildResource extends InstrumentedParentResource {
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import javax.ws.rs.*;
import javax.ws.rs.core.MediaType;
import java.io.IOException;

@Produces(MediaType.TEXT_PLAIN)
public class InstrumentedSubResource {
Expand Down