From 69ea9b0296ecf427d0b1bb599dfa3203664c45d4 Mon Sep 17 00:00:00 2001
From: Jon Chambers <jon@signal.org>
Date: Wed, 2 Aug 2023 11:33:56 -0400
Subject: [PATCH] Add a request counter tagged by client version

---
 .../textsecuregcm/WhisperServerService.java   |  6 +++---
 .../MetricsApplicationEventListener.java      |  5 +++--
 .../metrics/MetricsRequestEventListener.java  | 20 ++++++++++++++++---
 .../MetricsRequestEventListenerTest.java      |  7 ++++++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
index e13f458d3..d63d55b9f 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
@@ -669,7 +669,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
 
     environment.jersey().register(new RequestStatisticsFilter(TrafficSource.HTTP));
     environment.jersey().register(MultiRecipientMessageProvider.class);
-    environment.jersey().register(new MetricsApplicationEventListener(TrafficSource.HTTP));
+    environment.jersey().register(new MetricsApplicationEventListener(TrafficSource.HTTP, clientReleaseManager));
     environment.jersey()
         .register(new PolymorphicAuthDynamicFeature<>(ImmutableMap.of(AuthenticatedAccount.class, accountAuthFilter,
             DisabledPermittedAuthenticatedAccount.class, disabledPermittedAccountAuthFilter)));
@@ -689,7 +689,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
         .register(new WebsocketRefreshApplicationEventListener(accountsManager, clientPresenceManager));
     webSocketEnvironment.jersey().register(new RequestStatisticsFilter(TrafficSource.WEBSOCKET));
     webSocketEnvironment.jersey().register(MultiRecipientMessageProvider.class);
-    webSocketEnvironment.jersey().register(new MetricsApplicationEventListener(TrafficSource.WEBSOCKET));
+    webSocketEnvironment.jersey().register(new MetricsApplicationEventListener(TrafficSource.WEBSOCKET, clientReleaseManager));
     webSocketEnvironment.jersey().register(new KeepAliveController(clientPresenceManager));
 
     // these should be common, but use @Auth DisabledPermittedAccount, which isn’t supported yet on websocket
@@ -802,7 +802,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
         webSocketEnvironment.getRequestLog(), 60000);
     provisioningEnvironment.jersey().register(new WebsocketRefreshApplicationEventListener(accountsManager, clientPresenceManager));
     provisioningEnvironment.setConnectListener(new ProvisioningConnectListener(provisioningManager));
-    provisioningEnvironment.jersey().register(new MetricsApplicationEventListener(TrafficSource.WEBSOCKET));
+    provisioningEnvironment.jersey().register(new MetricsApplicationEventListener(TrafficSource.WEBSOCKET, clientReleaseManager));
     provisioningEnvironment.jersey().register(new KeepAliveController(clientPresenceManager));
 
     registerCorsFilter(environment);
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsApplicationEventListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsApplicationEventListener.java
index 9a4caace4..62b048277 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsApplicationEventListener.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsApplicationEventListener.java
@@ -9,6 +9,7 @@ import org.glassfish.jersey.server.monitoring.ApplicationEvent;
 import org.glassfish.jersey.server.monitoring.ApplicationEventListener;
 import org.glassfish.jersey.server.monitoring.RequestEvent;
 import org.glassfish.jersey.server.monitoring.RequestEventListener;
+import org.whispersystems.textsecuregcm.storage.ClientReleaseManager;
 
 /**
  * Delegates request events to a listener that captures and reports request-level metrics.
@@ -17,8 +18,8 @@ public class MetricsApplicationEventListener implements ApplicationEventListener
 
   private final MetricsRequestEventListener metricsRequestEventListener;
 
-  public MetricsApplicationEventListener(final TrafficSource trafficSource) {
-    this.metricsRequestEventListener = new MetricsRequestEventListener(trafficSource);
+  public MetricsApplicationEventListener(final TrafficSource trafficSource, final ClientReleaseManager clientReleaseManager) {
+    this.metricsRequestEventListener = new MetricsRequestEventListener(trafficSource, clientReleaseManager);
   }
 
   @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 d0b9cab25..988d16a96 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java
@@ -11,8 +11,10 @@ import com.google.common.net.HttpHeaders;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.Tags;
 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 javax.annotation.Nullable;
@@ -24,7 +26,10 @@ import java.util.List;
  */
 public class MetricsRequestEventListener implements RequestEventListener {
 
+  private final ClientReleaseManager clientReleaseManager;
+
   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");
 
   @VisibleForTesting
   static final String PATH_TAG = "path";
@@ -41,14 +46,18 @@ public class MetricsRequestEventListener implements RequestEventListener {
   private final TrafficSource trafficSource;
   private final MeterRegistry meterRegistry;
 
-  public MetricsRequestEventListener(final TrafficSource trafficSource) {
-    this(trafficSource, Metrics.globalRegistry);
+  public MetricsRequestEventListener(final TrafficSource trafficSource, final ClientReleaseManager clientReleaseManager) {
+    this(trafficSource, Metrics.globalRegistry, clientReleaseManager);
   }
 
   @VisibleForTesting
-  MetricsRequestEventListener(final TrafficSource trafficSource, final MeterRegistry meterRegistry) {
+  MetricsRequestEventListener(final TrafficSource trafficSource,
+      final MeterRegistry meterRegistry,
+      final ClientReleaseManager clientReleaseManager) {
+
     this.trafficSource = trafficSource;
     this.meterRegistry = meterRegistry;
+    this.clientReleaseManager = clientReleaseManager;
   }
 
   @Override
@@ -70,6 +79,11 @@ public class MetricsRequestEventListener implements RequestEventListener {
         tags.add(UserAgentTagUtil.getPlatformTag(userAgent));
 
         meterRegistry.counter(REQUEST_COUNTER_NAME, tags).increment();
+
+        UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager)
+            .ifPresent(clientVersionTag -> meterRegistry.counter(REQUESTS_BY_VERSION_COUNTER_NAME,
+                    Tags.of(clientVersionTag, UserAgentTagUtil.getPlatformTag(userAgent)))
+                .increment());
       }
     }
   }
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 0e3bf7de8..c9aeca5e9 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java
@@ -46,6 +46,7 @@ import org.glassfish.jersey.uri.UriTemplate;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import org.whispersystems.textsecuregcm.storage.ClientReleaseManager;
 import org.whispersystems.websocket.WebSocketResourceProvider;
 import org.whispersystems.websocket.auth.WebsocketAuthValueFactoryProvider;
 import org.whispersystems.websocket.logging.WebsocketRequestLog;
@@ -65,7 +66,11 @@ class MetricsRequestEventListenerTest {
   void setup() {
     meterRegistry = mock(MeterRegistry.class);
     counter = mock(Counter.class);
-    listener = new MetricsRequestEventListener(TRAFFIC_SOURCE, meterRegistry);
+
+    final ClientReleaseManager clientReleaseManager = mock(ClientReleaseManager.class);
+    when(clientReleaseManager.isVersionActive(any(), any())).thenReturn(false);
+
+    listener = new MetricsRequestEventListener(TRAFFIC_SOURCE, meterRegistry, clientReleaseManager);
   }
 
   @Test