From cd643901418ddd58dce933c90f031bbd4b47041b Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Sun, 18 Feb 2024 17:42:30 -0500 Subject: [PATCH] Add diagnostic dimensions to the "get keys" counter --- .../textsecuregcm/WhisperServerService.java | 2 +- .../controllers/KeysController.java | 33 ++++++++++++++----- .../controllers/KeysControllerTest.java | 13 +++++--- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 9570d82c9..53b1bdd9b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -894,7 +894,7 @@ public void run(WhisperServerConfiguration config, Environment environment) thro new DirectoryV2Controller(directoryV2CredentialsGenerator), new DonationController(clock, zkReceiptOperations, redeemedReceiptsManager, accountsManager, config.getBadges(), ReceiptCredentialPresentation::new), - new KeysController(rateLimiters, keysManager, accountsManager), + new KeysController(rateLimiters, keysManager, accountsManager, clientReleaseManager), new MessageController(rateLimiters, messageByteLimitCardinalityEstimator, messageSender, receiptSender, accountsManager, messagesManager, pushNotificationManager, reportMessageManager, multiRecipientMessageExecutor, messageDeliveryScheduler, reportSpamTokenProvider, clientReleaseManager, diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java index aa881b3df..c89ad9d46 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/KeysController.java @@ -58,6 +58,7 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; +import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.KeysManager; import org.whispersystems.textsecuregcm.util.HeaderUtils; @@ -72,6 +73,7 @@ public class KeysController { private final RateLimiters rateLimiters; private final KeysManager keysManager; private final AccountsManager accounts; + private final ClientReleaseManager clientReleaseManager; private static final String KEY_COUNT_DISTRIBUTION_NAME = MetricsUtil.name(KeysController.class, "getKeyCount"); private static final String GET_KEYS_COUNTER_NAME = MetricsUtil.name(KeysController.class, "getKeys"); @@ -82,10 +84,11 @@ public class KeysController { private static final CompletableFuture[] EMPTY_FUTURE_ARRAY = new CompletableFuture[0]; - public KeysController(RateLimiters rateLimiters, KeysManager keysManager, AccountsManager accounts) { + public KeysController(RateLimiters rateLimiters, KeysManager keysManager, AccountsManager accounts, ClientReleaseManager clientReleaseManager) { this.rateLimiters = rateLimiters; this.keysManager = keysManager; this.accounts = accounts; + this.clientReleaseManager = clientReleaseManager; } @GET @@ -282,14 +285,26 @@ public PreKeyResponse getDeviceKeys(@Auth Optional auth, final ECPreKey unsignedEcPreKey = unsignedEcPreKeyFuture.join().orElse(null); final ECSignedPreKey signedEcPreKey = signedEcPreKeyFuture.join().orElse(null); - Metrics.counter(GET_KEYS_COUNTER_NAME, Tags.of( - Tag.of(PRIMARY_DEVICE_TAG_NAME, String.valueOf(device.isPrimary())), - UserAgentTagUtil.getPlatformTag(userAgent), - Tag.of("targetPlatform", getDevicePlatform(device).map(Enum::name).orElse("unknown")), - Tag.of(IDENTITY_TYPE_TAG_NAME, targetIdentifier.identityType().name()), - Tag.of("isStale", String.valueOf(isDeviceStale(device))), - Tag.of("oneTimeEcKeyAvailable", String.valueOf(unsignedEcPreKey != null)))) - .increment(); + Tags tags = Tags.of( + Tag.of(PRIMARY_DEVICE_TAG_NAME, String.valueOf(device.isPrimary())), + UserAgentTagUtil.getPlatformTag(userAgent), + Tag.of("targetPlatform", getDevicePlatform(device).map(Enum::name).orElse("unknown")), + Tag.of(IDENTITY_TYPE_TAG_NAME, targetIdentifier.identityType().name()), + Tag.of("isStale", String.valueOf(isDeviceStale(device))), + Tag.of("oneTimeEcKeyAvailable", String.valueOf(unsignedEcPreKey != null)), + Tag.of("authenticationType", auth.isPresent() ? "authenticated" : "anonymous")); + + if (auth.isPresent()) { + tags = tags.and(Tag.of("targetIsSelf", String.valueOf(auth.get().getAccount().getUuid().equals(target.getUuid())))); + } + + final Optional maybeClientVersionTag = UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager); + + if (maybeClientVersionTag.isPresent()) { + tags = tags.and(maybeClientVersionTag.get()); + } + + Metrics.counter(GET_KEYS_COUNTER_NAME, tags).increment(); if (signedEcPreKey != null || unsignedEcPreKey != null || pqPreKey != null) { final int registrationId = switch (targetIdentifier.identityType()) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java index 1e76cbceb..86fea1855 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/KeysControllerTest.java @@ -32,7 +32,6 @@ import java.util.OptionalInt; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.function.Consumer; import javax.ws.rs.client.Entity; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -64,6 +63,7 @@ import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; +import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.KeysManager; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; @@ -122,9 +122,10 @@ class KeysControllerTest { private final ECSignedPreKey VALID_DEVICE_SIGNED_KEY = KeysHelper.signedECPreKey(89898, IDENTITY_KEY_PAIR); private final ECSignedPreKey VALID_DEVICE_PNI_SIGNED_KEY = KeysHelper.signedECPreKey(7777, PNI_IDENTITY_KEY_PAIR); - private final static KeysManager KEYS = mock(KeysManager.class ); - private final static AccountsManager accounts = mock(AccountsManager.class ); - private final static Account existsAccount = mock(Account.class ); + private final static KeysManager KEYS = mock(KeysManager.class); + private final static AccountsManager accounts = mock(AccountsManager.class); + private final static Account existsAccount = mock(Account.class); + private final static ClientReleaseManager clientReleaseManager = mock(ClientReleaseManager.class); private static final RateLimiters rateLimiters = mock(RateLimiters.class); private static final RateLimiter rateLimiter = mock(RateLimiter.class ); @@ -136,7 +137,7 @@ class KeysControllerTest { .addProvider(new AuthValueFactoryProvider.Binder<>(AuthenticatedAccount.class)) .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .addResource(new ServerRejectedExceptionMapper()) - .addResource(new KeysController(rateLimiters, KEYS, accounts)) + .addResource(new KeysController(rateLimiters, KEYS, accounts, clientReleaseManager)) .addResource(new RateLimitExceededExceptionMapper()) .build(); @@ -276,6 +277,8 @@ void setup() { when(KEYS.getEcSignedPreKey(AuthHelper.VALID_PNI, AuthHelper.VALID_DEVICE.getId())) .thenReturn(CompletableFuture.completedFuture(Optional.of(VALID_DEVICE_PNI_SIGNED_KEY))); + + when(clientReleaseManager.isVersionActive(any(), any())).thenReturn(true); } @AfterEach