Don't report exceptions as part of traffic metrics.
This commit is contained in:
parent
4188cc2949
commit
08dd493f98
|
@ -18,12 +18,10 @@ import java.util.List;
|
|||
*/
|
||||
class MetricsRequestEventListener implements RequestEventListener {
|
||||
|
||||
static final String COUNTER_NAME = MetricRegistry.name(MetricsRequestEventListener.class, "request");
|
||||
|
||||
static final String PATH_TAG = "path";
|
||||
static final String STATUS_CODE_TAG = "status";
|
||||
static final String TRAFFIC_SOURCE_TAG = "trafficSource";
|
||||
static final String EXCEPTION_TAG = "exception";
|
||||
static final String COUNTER_NAME = MetricRegistry.name(MetricsRequestEventListener.class, "request");
|
||||
static final String PATH_TAG = "path";
|
||||
static final String STATUS_CODE_TAG = "status";
|
||||
static final String TRAFFIC_SOURCE_TAG = "trafficSource";
|
||||
|
||||
private final TrafficSource trafficSource;
|
||||
private final MeterRegistry meterRegistry;
|
||||
|
@ -50,14 +48,6 @@ class MetricsRequestEventListener implements RequestEventListener {
|
|||
final List<String> userAgentValues = event.getContainerRequest().getRequestHeader("User-Agent");
|
||||
tags.addAll(UserAgentTagUtil.getUserAgentTags(userAgentValues != null ? userAgentValues.stream().findFirst().orElse(null) : null));
|
||||
|
||||
if (event.getException() != null) {
|
||||
if (event.getException() instanceof MappableException) {
|
||||
tags.add(Tag.of(EXCEPTION_TAG, event.getException().getCause().getClass().getSimpleName()));
|
||||
} else {
|
||||
tags.add(Tag.of(EXCEPTION_TAG, event.getException().getClass().getSimpleName()));
|
||||
}
|
||||
}
|
||||
|
||||
meterRegistry.counter(COUNTER_NAME, tags).increment();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,7 +1,6 @@
|
|||
package org.whispersystems.textsecuregcm.metrics;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.google.protobuf.ByteString;
|
||||
import com.google.protobuf.InvalidProtocolBufferException;
|
||||
import io.dropwizard.jersey.DropwizardResourceConfig;
|
||||
import io.dropwizard.jersey.jackson.JacksonMessageBodyProvider;
|
||||
|
@ -16,7 +15,6 @@ import org.glassfish.jersey.server.ContainerRequest;
|
|||
import org.glassfish.jersey.server.ContainerResponse;
|
||||
import org.glassfish.jersey.server.ExtendedUriInfo;
|
||||
import org.glassfish.jersey.server.ResourceConfig;
|
||||
import org.glassfish.jersey.server.internal.process.MappableException;
|
||||
import org.glassfish.jersey.server.monitoring.RequestEvent;
|
||||
import org.glassfish.jersey.uri.UriTemplate;
|
||||
import org.junit.Before;
|
||||
|
@ -51,18 +49,16 @@ import static org.mockito.Mockito.when;
|
|||
|
||||
public class MetricsRequestEventListenerTest {
|
||||
|
||||
private MeterRegistry meterRegistry;
|
||||
private Counter counter;
|
||||
private MeterRegistry meterRegistry;
|
||||
private Counter counter;
|
||||
private MetricsRequestEventListener listener;
|
||||
|
||||
private MetricsRequestEventListener listener;
|
||||
|
||||
private static final TrafficSource TRAFFIC_SOURCE = TrafficSource.HTTP;
|
||||
private static final TrafficSource TRAFFIC_SOURCE = TrafficSource.HTTP;
|
||||
|
||||
@Before
|
||||
public void setup() {
|
||||
meterRegistry = mock(MeterRegistry.class);
|
||||
counter = mock(Counter.class);
|
||||
|
||||
listener = new MetricsRequestEventListener(TRAFFIC_SOURCE, meterRegistry);
|
||||
}
|
||||
|
||||
|
@ -109,51 +105,6 @@ public class MetricsRequestEventListenerTest {
|
|||
assertTrue(tags.contains(Tag.of(UserAgentTagUtil.VERSION_TAG, "4.53.7")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("unchecked")
|
||||
public void testOnEventWithException() {
|
||||
final String path = "/test";
|
||||
final int statusCode = 500;
|
||||
|
||||
final ExtendedUriInfo uriInfo = mock(ExtendedUriInfo.class);
|
||||
when(uriInfo.getMatchedTemplates()).thenReturn(Collections.singletonList(new UriTemplate(path)));
|
||||
|
||||
final ContainerRequest request = mock(ContainerRequest.class);
|
||||
when(request.getRequestHeader("User-Agent")).thenReturn(Collections.singletonList("Signal-Android 4.53.7 (Android 8.1)"));
|
||||
|
||||
final ContainerResponse response = mock(ContainerResponse.class);
|
||||
when(response.getStatus()).thenReturn(statusCode);
|
||||
|
||||
final RequestEvent event = mock(RequestEvent.class);
|
||||
when(event.getType()).thenReturn(RequestEvent.Type.FINISHED);
|
||||
when(event.getUriInfo()).thenReturn(uriInfo);
|
||||
when(event.getContainerRequest()).thenReturn(request);
|
||||
when(event.getContainerResponse()).thenReturn(response);
|
||||
when(event.getException()).thenReturn(new MappableException(new RuntimeException("OH NO")));
|
||||
|
||||
final ArgumentCaptor<Iterable<Tag>> tagCaptor = ArgumentCaptor.forClass(Iterable.class);
|
||||
when(meterRegistry.counter(eq(MetricsRequestEventListener.COUNTER_NAME), any(Iterable.class))).thenReturn(counter);
|
||||
|
||||
listener.onEvent(event);
|
||||
|
||||
verify(meterRegistry).counter(eq(MetricsRequestEventListener.COUNTER_NAME), tagCaptor.capture());
|
||||
|
||||
final Iterable<Tag> tagIterable = tagCaptor.getValue();
|
||||
final Set<Tag> tags = new HashSet<>();
|
||||
|
||||
for (final Tag tag : tagIterable) {
|
||||
tags.add(tag);
|
||||
}
|
||||
|
||||
assertEquals(6, tags.size());
|
||||
assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.PATH_TAG, path)));
|
||||
assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(statusCode))));
|
||||
assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase())));
|
||||
assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android")));
|
||||
assertTrue(tags.contains(Tag.of(UserAgentTagUtil.VERSION_TAG, "4.53.7")));
|
||||
assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.EXCEPTION_TAG, "RuntimeException")));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGetPathTemplate() {
|
||||
final UriTemplate firstComponent = new UriTemplate("/first");
|
||||
|
|
Loading…
Reference in New Issue