diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java index ce182883d..a2fbb3495 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java @@ -34,8 +34,8 @@ import org.whispersystems.textsecuregcm.util.logging.UriInfoUtil; /** * Gathers and reports HTTP request metrics at the Jetty container level, which sits above Jersey. In order to get - * templated Jersey request paths, it implements {@link jakarta.ws.rs.container.ContainerResponseFilter}. It is limited - * to {@link TrafficSource#HTTP} requests. + * templated Jersey request paths, it implements {@link jakarta.ws.rs.container.ContainerResponseFilter}, in order to give + * itself access to the template. It is limited to {@link TrafficSource#HTTP} requests. *

* It implements {@link LifeCycle.Listener} without overriding methods, so that it can be an event listener that * Dropwizard will attach to the container—the {@link Container.Listener} implementation is where it attaches @@ -58,8 +58,6 @@ public class MetricsHttpChannelListener implements HttpChannel.Listener, Contain // Use the same counter namespace as MetricsRequestEventListener for continuity public static final String REQUEST_COUNTER_NAME = MetricsRequestEventListener.REQUEST_COUNTER_NAME; public static final String REQUESTS_BY_VERSION_COUNTER_NAME = MetricsRequestEventListener.REQUESTS_BY_VERSION_COUNTER_NAME; - public static final String REQUESTS_BY_AUTHENTICATION_COUNTER_NAME = MetricsRequestEventListener.REQUESTS_BY_AUTHENTICATION_COUNTER_NAME; - @VisibleForTesting static final String URI_INFO_PROPERTY_NAME = MetricsHttpChannelListener.class.getName() + ".uriInfo"; @@ -143,19 +141,9 @@ public class MetricsHttpChannelListener implements HttpChannel.Listener, Contain meterRegistry.counter(REQUEST_COUNTER_NAME, tags).increment(); - final Tag clientPlatformTag = UserAgentTagUtil.getPlatformTag(requestInfo.userAgent()); - UserAgentTagUtil.getClientVersionTag(requestInfo.userAgent(), clientReleaseManager).ifPresent( clientVersionTag -> meterRegistry.counter(REQUESTS_BY_VERSION_COUNTER_NAME, - Tags.of(clientVersionTag, clientPlatformTag)).increment()); - - meterRegistry.counter(REQUESTS_BY_AUTHENTICATION_COUNTER_NAME, Tags.of( - TRAFFIC_SOURCE_TAG, TrafficSource.HTTP.name().toLowerCase(), - PATH_TAG, requestInfo.path(), - METHOD_TAG, requestInfo.method(), - "authenticated", String.valueOf(request.getHeader(HttpHeaders.AUTHORIZATION) != null)) - .and(clientPlatformTag)) - .increment(); + Tags.of(clientVersionTag, UserAgentTagUtil.getPlatformTag(requestInfo.userAgent()))).increment()); } @Override diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java index b1f327dcc..e567a6c6b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java @@ -17,8 +17,6 @@ import org.glassfish.jersey.server.monitoring.RequestEvent; import org.glassfish.jersey.server.monitoring.RequestEventListener; import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.util.logging.UriInfoUtil; -import org.whispersystems.websocket.ReusableAuth; -import org.whispersystems.websocket.WebSocketResourceProvider; import javax.annotation.Nullable; import java.util.ArrayList; @@ -34,7 +32,6 @@ public class MetricsRequestEventListener implements RequestEventListener { public static final String REQUEST_COUNTER_NAME = MetricRegistry.name(MetricsRequestEventListener.class, "request"); public static final String REQUESTS_BY_VERSION_COUNTER_NAME = MetricRegistry.name(MetricsRequestEventListener.class, "requestByVersion"); - public static final String REQUESTS_BY_AUTHENTICATION_COUNTER_NAME = MetricRegistry.name(MetricsRequestEventListener.class, "requestByAuthentication"); @VisibleForTesting static final String PATH_TAG = "path"; @@ -81,31 +78,17 @@ public class MetricsRequestEventListener implements RequestEventListener { @Nullable final String userAgent; { final List userAgentValues = event.getContainerRequest().getRequestHeader(HttpHeaders.USER_AGENT); - userAgent = userAgentValues != null && !userAgentValues.isEmpty() ? userAgentValues.getFirst() : null; + userAgent = userAgentValues != null && !userAgentValues.isEmpty() ? userAgentValues.get(0) : null; } tags.addAll(UserAgentTagUtil.getLibsignalAndPlatformTags(userAgent)); meterRegistry.counter(REQUEST_COUNTER_NAME, tags).increment(); - final Tag clientPlatformTag = UserAgentTagUtil.getPlatformTag(userAgent); - UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager) .ifPresent(clientVersionTag -> meterRegistry.counter(REQUESTS_BY_VERSION_COUNTER_NAME, - Tags.of(clientVersionTag, clientPlatformTag)) + Tags.of(clientVersionTag, UserAgentTagUtil.getPlatformTag(userAgent))) .increment()); - - final boolean authenticated = - event.getContainerRequest().getProperty(WebSocketResourceProvider.REUSABLE_AUTH_PROPERTY) instanceof ReusableAuth reusableAuth && - reusableAuth.ref().isPresent(); - - meterRegistry.counter(REQUESTS_BY_AUTHENTICATION_COUNTER_NAME, Tags.of( - TRAFFIC_SOURCE_TAG, trafficSource.name().toLowerCase(), - PATH_TAG, UriInfoUtil.getPathTemplate(event.getUriInfo()), - METHOD_TAG, event.getContainerRequest().getMethod(), - "authenticated", String.valueOf(authenticated)) - .and(clientPlatformTag)) - .increment(); } } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java index 5ee70a65e..9089cbba5 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java @@ -9,7 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.ArgumentMatchers.anyIterable; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -89,12 +89,6 @@ class MetricsHttpChannelListenerIntegrationTest { private static final DropwizardAppExtension EXTENSION = new DropwizardAppExtension<>( MetricsHttpChannelListenerIntegrationTest.TestApplication.class); - @BeforeEach - void setUpBefore() { - when(METER_REGISTRY.counter(anyString(), anyIterable())).thenReturn(mock(Counter.class)); - when(METER_REGISTRY.counter(eq(MetricsHttpChannelListener.REQUEST_COUNTER_NAME), anyIterable())).thenReturn(COUNTER); - } - @AfterEach void teardown() { reset(METER_REGISTRY); @@ -110,7 +104,14 @@ class MetricsHttpChannelListenerIntegrationTest { final CountDownLatch countDownLatch = new CountDownLatch(1); COUNT_DOWN_LATCH_FUTURE_REFERENCE.set(countDownLatch); - final Client client = EXTENSION.client(); + final ArgumentCaptor> tagCaptor = ArgumentCaptor.forClass(Iterable.class); + when(METER_REGISTRY.counter(anyString(), any(Iterable.class))) + .thenAnswer(a -> MetricsHttpChannelListener.REQUEST_COUNTER_NAME.equals(a.getArgument(0, String.class)) + ? COUNTER + : mock(Counter.class)) + .thenReturn(COUNTER); + + Client client = EXTENSION.client(); final Supplier request = () -> client.target( String.format("http://localhost:%d%s", EXTENSION.getLocalPort(), requestPath)) @@ -139,7 +140,6 @@ class MetricsHttpChannelListenerIntegrationTest { assertTrue(countDownLatch.await(1000, TimeUnit.MILLISECONDS)); - @SuppressWarnings("unchecked") final ArgumentCaptor> tagCaptor = ArgumentCaptor.forClass(Iterable.class); verify(METER_REGISTRY).counter(eq(MetricsHttpChannelListener.REQUEST_COUNTER_NAME), tagCaptor.capture()); verify(COUNTER).increment(); @@ -185,6 +185,13 @@ class MetricsHttpChannelListenerIntegrationTest { final CountDownLatch countDownLatch = new CountDownLatch(1); COUNT_DOWN_LATCH_FUTURE_REFERENCE.set(countDownLatch); + final ArgumentCaptor> tagCaptor = ArgumentCaptor.forClass(Iterable.class); + when(METER_REGISTRY.counter(anyString(), any(Iterable.class))) + .thenAnswer(a -> MetricsHttpChannelListener.REQUEST_COUNTER_NAME.equals(a.getArgument(0, String.class)) + ? COUNTER + : mock(Counter.class)) + .thenReturn(COUNTER); + client.connect(new WebSocketListener() { @Override public void onWebSocketConnect(final Session session) { @@ -195,7 +202,6 @@ class MetricsHttpChannelListenerIntegrationTest { assertTrue(countDownLatch.await(1000, TimeUnit.MILLISECONDS)); - @SuppressWarnings("unchecked") final ArgumentCaptor> tagCaptor = ArgumentCaptor.forClass(Iterable.class); verify(METER_REGISTRY).counter(eq(MetricsHttpChannelListener.REQUEST_COUNTER_NAME), tagCaptor.capture()); verify(COUNTER).increment(); @@ -300,7 +306,7 @@ class MetricsHttpChannelListenerIntegrationTest { @Override public void beanAdded(final Container parent, final Object child) { if (child instanceof Connector connector) { - connector.addBean(this); + connector.addBean(this); } } @@ -325,7 +331,7 @@ class MetricsHttpChannelListenerIntegrationTest { @GET @Path("/greet/{name}") - public String testGreetByName(@PathParam("name") String name, @Context ContainerRequestContext ignored) { + public String testGreetByName(@PathParam("name") String name, @Context ContainerRequestContext context) { if ("exception".equals(name)) { throw new InternalServerErrorException(); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java index 70fd78fd9..974497ad0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java @@ -8,7 +8,6 @@ package org.whispersystems.textsecuregcm.metrics; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyIterable; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -49,15 +48,12 @@ class MetricsHttpChannelListenerTest { requestCounter = mock(Counter.class); requestsByVersionCounter = mock(Counter.class); - when(meterRegistry.counter(eq(MetricsHttpChannelListener.REQUEST_COUNTER_NAME), anyIterable())) + when(meterRegistry.counter(eq(MetricsHttpChannelListener.REQUEST_COUNTER_NAME), any(Iterable.class))) .thenReturn(requestCounter); - when(meterRegistry.counter(eq(MetricsHttpChannelListener.REQUESTS_BY_VERSION_COUNTER_NAME), anyIterable())) + when(meterRegistry.counter(eq(MetricsHttpChannelListener.REQUESTS_BY_VERSION_COUNTER_NAME), any(Iterable.class))) .thenReturn(requestsByVersionCounter); - when(meterRegistry.counter(eq(MetricsHttpChannelListener.REQUESTS_BY_AUTHENTICATION_COUNTER_NAME), anyIterable())) - .thenReturn(mock(Counter.class)); - clientReleaseManager = mock(ClientReleaseManager.class); listener = new MetricsHttpChannelListener(meterRegistry, clientReleaseManager, Collections.emptySet()); @@ -138,8 +134,8 @@ class MetricsHttpChannelListenerTest { final ArgumentCaptor> tagCaptor = ArgumentCaptor.forClass(Iterable.class); verify(meterRegistry).counter(eq(MetricsHttpChannelListener.REQUESTS_BY_VERSION_COUNTER_NAME), tagCaptor.capture()); - final Set tags = new HashSet<>(); + tags.clear(); for (final Tag tag : tagCaptor.getValue()) { tags.add(tag); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java index 9e28bf763..232dbdd6c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java @@ -9,8 +9,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyIterable; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -73,8 +71,6 @@ class MetricsRequestEventListenerTest { meterRegistry = mock(MeterRegistry.class); counter = mock(Counter.class); - when(meterRegistry.counter(anyString(), anyIterable())).thenReturn(mock(Counter.class)); - final ClientReleaseManager clientReleaseManager = mock(ClientReleaseManager.class); when(clientReleaseManager.isVersionActive(any(), any())).thenReturn(false);