Revert "Count API calls by authentication status"

This reverts commit 9b835633ab.

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
+ * templated Jersey request paths, it implements {@link jakarta.ws.rs.container.ContainerResponseFilter}, in order to give
- * to {@link TrafficSource#HTTP} requests.
+ * itself access to the template. It is limited to {@link TrafficSource#HTTP} requests.
  * <p>
  * It implements {@link LifeCycle.Listener} without overriding methods, so that it can be an event listener that
  * Dropwizard will attach to the container&mdash;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());
+            Tags.of(clientVersionTag, UserAgentTagUtil.getPlatformTag(requestInfo.userAgent()))).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();
   }
 
   @Override

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<String> 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();
       }
     }
   }

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<Configuration> 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<Iterable<Tag>> 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<String> 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<Iterable<Tag>> 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<Iterable<Tag>> 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<Iterable<Tag>> 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();

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<Iterable<Tag>> tagCaptor = ArgumentCaptor.forClass(Iterable.class);
       verify(meterRegistry).counter(eq(MetricsHttpChannelListener.REQUESTS_BY_VERSION_COUNTER_NAME),
           tagCaptor.capture());
-
       final Set<Tag> tags = new HashSet<>();
+      tags.clear();
       for (final Tag tag : tagCaptor.getValue()) {
         tags.add(tag);
       }

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);