Standardize client tag version handling; add client version tags to delivery latency metrics

service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java

@@ -642,7 +642,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
     webSocketEnvironment.setAuthenticator(new WebSocketAccountAuthenticator(accountAuthenticator));
     webSocketEnvironment.setConnectListener(
         new AuthenticatedConnectListener(receiptSender, messagesManager, pushNotificationManager,
-            clientPresenceManager, websocketScheduledExecutor, messageDeliveryScheduler));
+            clientPresenceManager, websocketScheduledExecutor, messageDeliveryScheduler, dynamicConfigurationManager));
     webSocketEnvironment.jersey()
         .register(new WebsocketRefreshApplicationEventListener(accountsManager, clientPresenceManager));
     webSocketEnvironment.jersey().register(new RequestStatisticsFilter(TrafficSource.WEBSOCKET));
@@ -719,7 +719,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
             ReceiptCredentialPresentation::new),
         new MessageController(rateLimiters, messageSender, receiptSender, accountsManager, deletedAccounts,
             messagesManager, pushNotificationManager, reportMessageManager, multiRecipientMessageExecutor,
-            messageDeliveryScheduler, reportSpamTokenProvider),
+            messageDeliveryScheduler, reportSpamTokenProvider, dynamicConfigurationManager),
         new PaymentsController(currencyManager, paymentsCredentialsGenerator),
         new ProfileController(clock, rateLimiters, accountsManager, profilesManager, dynamicConfigurationManager,
             profileBadgeConverter, config.getBadges(), cdnS3Client, profileCdnPolicyGenerator, profileCdnPolicySigner,

service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfiguration.java

@@ -59,6 +59,10 @@ public class DynamicConfiguration {
   @Valid
   DynamicECPreKeyMigrationConfiguration ecPreKeyMigration = new DynamicECPreKeyMigrationConfiguration(true, false);
 
+  @JsonProperty
+  @Valid
+  DynamicDeliveryLatencyConfiguration deliveryLatency = new DynamicDeliveryLatencyConfiguration(Collections.emptyMap());
+
   public Optional<DynamicExperimentEnrollmentConfiguration> getExperimentEnrollmentConfiguration(
       final String experimentName) {
     return Optional.ofNullable(experiments.get(experimentName));
@@ -104,4 +108,8 @@ public class DynamicConfiguration {
   public DynamicECPreKeyMigrationConfiguration getEcPreKeyMigrationConfiguration() {
     return ecPreKeyMigration;
   }
+
+  public DynamicDeliveryLatencyConfiguration getDeliveryLatencyConfiguration() {
+    return deliveryLatency;
+  }
 }

service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicDeliveryLatencyConfiguration.java

@@ -0,0 +1,14 @@
+/*
+ * Copyright 2023 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.whispersystems.textsecuregcm.configuration.dynamic;
+
+import com.vdurmont.semver4j.Semver;
+import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
+import java.util.Map;
+import java.util.Set;
+
+public record DynamicDeliveryLatencyConfiguration(Map<ClientPlatform, Set<Semver>> instrumentedVersions) {
+}

service/src/main/java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicPushLatencyConfiguration.java

@@ -12,16 +12,5 @@ import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
 import java.util.Map;
 import java.util.Set;
 
-public class DynamicPushLatencyConfiguration {
-
-  private final Map<ClientPlatform, Set<Semver>> instrumentedVersions;
-
-  @JsonCreator
-  public DynamicPushLatencyConfiguration(@JsonProperty("instrumentedVersions") final Map<ClientPlatform, Set<Semver>> instrumentedVersions) {
-    this.instrumentedVersions = instrumentedVersions;
-  }
-
-  public Map<ClientPlatform, Set<Semver>> getInstrumentedVersions() {
-    return instrumentedVersions;
-  }
+public record DynamicPushLatencyConfiguration(Map<ClientPlatform, Set<Semver>> instrumentedVersions) {
 }

service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java

@@ -66,6 +66,7 @@ import org.whispersystems.textsecuregcm.auth.Anonymous;
 import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
 import org.whispersystems.textsecuregcm.auth.CombinedUnidentifiedSenderAccessKeys;
 import org.whispersystems.textsecuregcm.auth.OptionalAccess;
+import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.entities.AccountMismatchedDevices;
 import org.whispersystems.textsecuregcm.entities.AccountStaleDevices;
 import org.whispersystems.textsecuregcm.entities.IncomingMessage;
@@ -95,6 +96,7 @@ import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.AccountsManager;
 import org.whispersystems.textsecuregcm.storage.DeletedAccounts;
 import org.whispersystems.textsecuregcm.storage.Device;
+import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
 import org.whispersystems.textsecuregcm.storage.ReportMessageManager;
 import org.whispersystems.textsecuregcm.util.DestinationDeviceValidator;
@@ -122,6 +124,7 @@ public class MessageController {
   private final ExecutorService multiRecipientMessageExecutor;
   private final Scheduler messageDeliveryScheduler;
   private final ReportSpamTokenProvider reportSpamTokenProvider;
+  private final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager;
 
   private static final String REJECT_OVERSIZE_MESSAGE_COUNTER = name(MessageController.class, "rejectOversizeMessage");
   private static final String SENT_MESSAGE_COUNTER_NAME = name(MessageController.class, "sentMessages");
@@ -154,7 +157,8 @@ public class MessageController {
       ReportMessageManager reportMessageManager,
       @Nonnull ExecutorService multiRecipientMessageExecutor,
       Scheduler messageDeliveryScheduler,
-      @Nonnull ReportSpamTokenProvider reportSpamTokenProvider) {
+      @Nonnull ReportSpamTokenProvider reportSpamTokenProvider,
+      final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager) {
     this.rateLimiters = rateLimiters;
     this.messageSender = messageSender;
     this.receiptSender = receiptSender;
@@ -166,6 +170,7 @@ public class MessageController {
     this.multiRecipientMessageExecutor = Objects.requireNonNull(multiRecipientMessageExecutor);
     this.messageDeliveryScheduler = messageDeliveryScheduler;
     this.reportSpamTokenProvider = reportSpamTokenProvider;
+    this.dynamicConfigurationManager = dynamicConfigurationManager;
   }
 
   @Timed
@@ -536,7 +541,8 @@ public class MessageController {
               .map(OutgoingMessageEntity::fromEnvelope)
               .peek(outgoingMessageEntity -> {
                 MessageMetrics.measureAccountOutgoingMessageUuidMismatches(auth.getAccount(), outgoingMessageEntity);
-                MessageMetrics.measureOutgoingMessageLatency(outgoingMessageEntity.serverTimestamp(), "rest", userAgent);
+                MessageMetrics.measureOutgoingMessageLatency(outgoingMessageEntity.serverTimestamp(), "rest", userAgent,
+                    dynamicConfigurationManager.getConfiguration().getDeliveryLatencyConfiguration().instrumentedVersions());
               })
               .collect(Collectors.toList()),
               messagesAndHasMore.second());

service/src/main/java/org/whispersystems/textsecuregcm/metrics/MessageMetrics.java

@@ -7,9 +7,14 @@ package org.whispersystems.textsecuregcm.metrics;
 
 import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
 
+import com.vdurmont.semver4j.Semver;
 import io.micrometer.core.instrument.Metrics;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
@@ -18,6 +23,8 @@ import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.entities.MessageProtos;
 import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity;
 import org.whispersystems.textsecuregcm.storage.Account;
+import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
+import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil;
 
 public final class MessageMetrics {
 
@@ -54,10 +61,18 @@ public final class MessageMetrics {
     }
   }
 
-  public static void measureOutgoingMessageLatency(final long serverTimestamp, final String channel, final String userAgent) {
-    Metrics.timer(DELIVERY_LATENCY_TIMER_NAME, Tags.of(
-            UserAgentTagUtil.getPlatformTag(userAgent),
-            Tag.of("channel", channel)))
+  public static void measureOutgoingMessageLatency(final long serverTimestamp,
+      final String channel,
+      final String userAgent,
+      final Map<ClientPlatform, Set<Semver>> taggedVersions) {
+
+    final List<Tag> tags = new ArrayList<>(3);
+    tags.add(UserAgentTagUtil.getPlatformTag(userAgent));
+    tags.add(Tag.of("channel", channel));
+
+    UserAgentTagUtil.getClientVersionTag(userAgent, taggedVersions).ifPresent(tags::add);
+
+    Metrics.timer(DELIVERY_LATENCY_TIMER_NAME, tags)
         .record(Duration.between(Instant.ofEpochMilli(serverTimestamp), Instant.now()));
   }
 }

service/src/main/java/org/whispersystems/textsecuregcm/metrics/UserAgentTagUtil.java

@@ -7,14 +7,15 @@ package org.whispersystems.textsecuregcm.metrics;
 
 import com.vdurmont.semver4j.Semver;
 import io.micrometer.core.instrument.Tag;
-import org.whispersystems.textsecuregcm.util.Pair;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
 import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException;
 import org.whispersystems.textsecuregcm.util.ua.UserAgent;
 import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil;
 
-import java.util.*;
-
 /**
  * Utility class for extracting platform/version metrics tags from User-Agent strings.
  */

service/src/main/java/org/whispersystems/textsecuregcm/push/PushLatencyManager.java

@@ -8,7 +8,6 @@ package org.whispersystems.textsecuregcm.push;
 import com.codahale.metrics.MetricRegistry;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.google.common.annotations.VisibleForTesting;
-import com.vdurmont.semver4j.Semver;
 import io.lettuce.core.SetArgs;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.Tag;
@@ -16,10 +15,8 @@ import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import org.apache.commons.lang3.StringUtils;
@@ -30,9 +27,6 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster;
 import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 import org.whispersystems.textsecuregcm.util.SystemMapper;
-import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException;
-import org.whispersystems.textsecuregcm.util.ua.UserAgent;
-import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil;
 
 /**
  * Measures and records the latency between sending a push notification to a device and that device draining its queue
@@ -105,21 +99,12 @@ public class PushLatencyManager {
         tags.add(UserAgentTagUtil.getPlatformTag(userAgentString));
         tags.add(Tag.of("pushType", pushRecord.pushType().name().toLowerCase()));
 
+        UserAgentTagUtil.getClientVersionTag(userAgentString,
+                dynamicConfigurationManager.getConfiguration().getPushLatencyConfiguration().instrumentedVersions())
+            .ifPresent(tags::add);
+
         pushRecord.urgent().ifPresent(urgent -> tags.add(Tag.of("urgent", String.valueOf(urgent))));
 
-        try {
-          final UserAgent userAgent = UserAgentUtil.parseUserAgentString(userAgentString);
-
-          final Set<Semver> instrumentedVersions =
-              dynamicConfigurationManager.getConfiguration().getPushLatencyConfiguration().getInstrumentedVersions()
-                  .getOrDefault(userAgent.getPlatform(), Collections.emptySet());
-
-          if (instrumentedVersions.contains(userAgent.getVersion())) {
-            tags.add(Tag.of("clientVersion", userAgent.getVersion().toString()));
-          }
-        } catch (UnrecognizedUserAgentException ignored) {
-        }
-
         Metrics.timer(TIMER_NAME, tags).record(latency);
       }
     });

service/src/main/java/org/whispersystems/textsecuregcm/websocket/AuthenticatedConnectListener.java

@@ -23,6 +23,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
+import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
 import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.push.ClientPresenceManager;
@@ -31,6 +32,7 @@ import org.whispersystems.textsecuregcm.push.PushNotificationManager;
 import org.whispersystems.textsecuregcm.push.ReceiptSender;
 import org.whispersystems.textsecuregcm.redis.RedisOperation;
 import org.whispersystems.textsecuregcm.storage.Device;
+import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
 import org.whispersystems.textsecuregcm.util.Constants;
 import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
@@ -67,6 +69,7 @@ public class AuthenticatedConnectListener implements WebSocketConnectListener {
   private final ClientPresenceManager clientPresenceManager;
   private final ScheduledExecutorService scheduledExecutorService;
   private final Scheduler messageDeliveryScheduler;
+  private final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager;
 
   private final Map<ClientPlatform, AtomicInteger> openAuthenticatedWebsocketsByClientPlatform;
   private final Map<ClientPlatform, AtomicInteger> openUnauthenticatedWebsocketsByClientPlatform;
@@ -83,13 +86,15 @@ public class AuthenticatedConnectListener implements WebSocketConnectListener {
       PushNotificationManager pushNotificationManager,
       ClientPresenceManager clientPresenceManager,
       ScheduledExecutorService scheduledExecutorService,
-      Scheduler messageDeliveryScheduler) {
+      Scheduler messageDeliveryScheduler,
+      final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager) {
     this.receiptSender = receiptSender;
     this.messagesManager = messagesManager;
     this.pushNotificationManager = pushNotificationManager;
     this.clientPresenceManager = clientPresenceManager;
     this.scheduledExecutorService = scheduledExecutorService;
     this.messageDeliveryScheduler = messageDeliveryScheduler;
+    this.dynamicConfigurationManager = dynamicConfigurationManager;
 
     openAuthenticatedWebsocketsByClientPlatform = new EnumMap<>(ClientPlatform.class);
     openUnauthenticatedWebsocketsByClientPlatform = new EnumMap<>(ClientPlatform.class);
@@ -151,7 +156,8 @@ public class AuthenticatedConnectListener implements WebSocketConnectListener {
           messagesManager, auth, device,
           context.getClient(),
           scheduledExecutorService,
-          messageDeliveryScheduler);
+          messageDeliveryScheduler,
+          dynamicConfigurationManager);
 
       openWebsocketAtomicInteger.incrementAndGet();
       openWebsocketCounter.inc();

service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java

@@ -37,6 +37,7 @@ import org.reactivestreams.Publisher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
+import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.controllers.MessageController;
 import org.whispersystems.textsecuregcm.entities.MessageProtos.Envelope;
 import org.whispersystems.textsecuregcm.metrics.MessageMetrics;
@@ -45,6 +46,7 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
 import org.whispersystems.textsecuregcm.push.DisplacedPresenceListener;
 import org.whispersystems.textsecuregcm.push.ReceiptSender;
 import org.whispersystems.textsecuregcm.storage.Device;
+import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 import org.whispersystems.textsecuregcm.storage.MessageAvailabilityListener;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
 import org.whispersystems.textsecuregcm.util.Constants;
@@ -128,6 +130,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
   private final Random random = new Random();
   private final Scheduler messageDeliveryScheduler;
 
+  private final DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager;
+
   private enum StoredMessageState {
     EMPTY,
     CACHED_NEW_MESSAGES_AVAILABLE,
@@ -140,7 +144,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
       Device device,
       WebSocketClient client,
       ScheduledExecutorService scheduledExecutorService,
-      Scheduler messageDeliveryScheduler) {
+      Scheduler messageDeliveryScheduler,
+      DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager) {
 
     this(receiptSender,
         messagesManager,
@@ -149,7 +154,7 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
         client,
         DEFAULT_SEND_FUTURES_TIMEOUT_MILLIS,
         scheduledExecutorService,
-        messageDeliveryScheduler);
+        messageDeliveryScheduler, dynamicConfigurationManager);
   }
 
   @VisibleForTesting
@@ -160,7 +165,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
       WebSocketClient client,
       int sendFuturesTimeoutMillis,
       ScheduledExecutorService scheduledExecutorService,
-      Scheduler messageDeliveryScheduler) {
+      Scheduler messageDeliveryScheduler,
+      DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager) {
 
     this.receiptSender = receiptSender;
     this.messagesManager = messagesManager;
@@ -170,6 +176,7 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
     this.sendFuturesTimeoutMillis = sendFuturesTimeoutMillis;
     this.scheduledExecutorService = scheduledExecutorService;
     this.messageDeliveryScheduler = messageDeliveryScheduler;
+    this.dynamicConfigurationManager = dynamicConfigurationManager;
   }
 
   public void start() {
@@ -208,7 +215,8 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac
           if (throwable != null) {
             sendFailuresMeter.mark();
           } else {
-            MessageMetrics.measureOutgoingMessageLatency(message.getServerTimestamp(), "websocket", client.getUserAgent());
+            MessageMetrics.measureOutgoingMessageLatency(message.getServerTimestamp(), "websocket", client.getUserAgent(),
+                dynamicConfigurationManager.getConfiguration().getDeliveryLatencyConfiguration().instrumentedVersions());
           }
         }).thenCompose(response -> {
           final CompletableFuture<Void> result;