From 556eec649da46b54df4cd7399aa3826f510a7776 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 6 Sep 2024 11:56:29 -0400 Subject: [PATCH] Add platform and push token presence dimensions to account creation metrics --- .../controllers/RegistrationController.java | 3 ++- .../storage/AccountsManager.java | 12 ++++++++++-- .../RegistrationControllerTest.java | 19 ++++++++++--------- ...ccountCreationDeletionIntegrationTest.java | 12 ++++++++---- ...ConcurrentModificationIntegrationTest.java | 3 ++- .../storage/AccountsManagerTest.java | 3 ++- .../tests/util/AccountsHelper.java | 3 ++- 7 files changed, 36 insertions(+), 19 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RegistrationController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RegistrationController.java index 91c7c9c1e..0f0ee854c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RegistrationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RegistrationController.java @@ -157,7 +157,8 @@ public AccountIdentityResponse register( registrationRequest.deviceActivationRequest().aciSignedPreKey(), registrationRequest.deviceActivationRequest().pniSignedPreKey(), registrationRequest.deviceActivationRequest().aciPqLastResortPreKey(), - registrationRequest.deviceActivationRequest().pniPqLastResortPreKey())); + registrationRequest.deviceActivationRequest().pniPqLastResortPreKey()), + userAgent); Metrics.counter(ACCOUNT_CREATED_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent), Tag.of(COUNTRY_CODE_TAG_NAME, Util.getCountryCode(number)), diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index bb24ff387..43cad4bd2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -15,6 +15,8 @@ import io.lettuce.core.RedisException; import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands; import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; import java.io.IOException; import java.io.UncheckedIOException; @@ -52,6 +54,7 @@ import org.whispersystems.textsecuregcm.entities.KEMSignedPreKey; import org.whispersystems.textsecuregcm.identity.IdentityType; import org.whispersystems.textsecuregcm.identity.ServiceIdentifier; +import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.push.ClientPresenceManager; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; import org.whispersystems.textsecuregcm.redis.RedisOperation; @@ -167,7 +170,8 @@ public Account create(final String number, final List accountBadges, final IdentityKey aciIdentityKey, final IdentityKey pniIdentityKey, - final DeviceSpec primaryDeviceSpec) throws InterruptedException { + final DeviceSpec primaryDeviceSpec, + @Nullable final String userAgent) throws InterruptedException { final Account account = new Account(); @@ -251,7 +255,11 @@ public Account create(final String number, redisSet(account); - Metrics.counter(CREATE_COUNTER_NAME, "type", accountCreationType).increment(); + Metrics.counter(CREATE_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent), + Tag.of("type", accountCreationType), + Tag.of("hasPushToken", String.valueOf( + primaryDeviceSpec.apnRegistrationId().isPresent() || primaryDeviceSpec.gcmRegistrationId().isPresent())))) + .increment(); accountAttributes.recoveryPassword().ifPresent(registrationRecoveryPassword -> registrationRecoveryPasswordsManager.storeForCurrentNumber(account.getNumber(), registrationRecoveryPassword)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java index 017670ba6..706b64aef 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/RegistrationControllerTest.java @@ -170,7 +170,7 @@ void invalidRegistrationId(Optional registrationId, Optional p final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final String json = requestJson("sessionId", new byte[0], true, registrationId.orElse(0), pniRegistrationId.orElse(0)); @@ -295,7 +295,7 @@ void recoveryPasswordManagerVerificationTrue() throws InterruptedException { final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -331,7 +331,7 @@ void registrationRecoveryCheckerAllowsAttempt() throws InterruptedException { final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -353,7 +353,7 @@ void registrationRecoveryCheckerDisallowsAttempt() throws InterruptedException { final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -397,7 +397,7 @@ void registrationLockAndDeviceTransfer( final Account createdAccount = mock(Account.class); when(createdAccount.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(createdAccount); expectedStatus = 200; @@ -451,7 +451,7 @@ void deviceTransferAvailable(final boolean existingAccount, final boolean transf final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -475,7 +475,7 @@ void registrationSuccess() throws Exception { final Account account = mock(Account.class); when(account.getPrimaryDevice()).thenReturn(mock(Device.class)); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -728,7 +728,7 @@ void atomicAccountCreationSuccess(final RegistrationRequest registrationRequest, when(a.getPrimaryDevice()).thenReturn(device); }); - when(accountsManager.create(any(), any(), any(), any(), any(), any())) + when(accountsManager.create(any(), any(), any(), any(), any(), any(), any())) .thenReturn(account); final Invocation.Builder request = resources.getJerseyTest() @@ -746,7 +746,8 @@ void atomicAccountCreationSuccess(final RegistrationRequest registrationRequest, eq(Collections.emptyList()), eq(expectedAciIdentityKey), eq(expectedPniIdentityKey), - eq(expectedDeviceSpec)); + eq(expectedDeviceSpec), + any()); } private static boolean accountAttributesEqual(final AccountAttributes a, final AccountAttributes b) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java index 0312b8d94..6a55fc109 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java @@ -237,7 +237,8 @@ void createAccount(final DeliveryChannels deliveryChannels, aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, - pniPqLastResortPreKey)); + pniPqLastResortPreKey), + null); assertExpectedStoredAccount(account, number, @@ -315,7 +316,8 @@ void reregisterAccount(final DeliveryChannels deliveryChannels, aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, - pniPqLastResortPreKey)); + pniPqLastResortPreKey), + null); existingAccountUuid = originalAccount.getUuid(); } @@ -381,7 +383,8 @@ void reregisterAccount(final DeliveryChannels deliveryChannels, aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, - pniPqLastResortPreKey)); + pniPqLastResortPreKey), + null); assertExpectedStoredAccount(reregisteredAccount, number, @@ -464,7 +467,8 @@ void deleteAccount() throws InterruptedException { aciSignedPreKey, pniSignedPreKey, aciPqLastResortPreKey, - pniPqLastResortPreKey)); + pniPqLastResortPreKey), + null); clientPublicKeysManager.setPublicKey(account, Device.PRIMARY_ID, Curve.generateKeyPair().getPublicKey()).join(); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java index 926152982..7216d298d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java @@ -165,7 +165,8 @@ void testConcurrentUpdate() throws IOException, InterruptedException { KeysHelper.signedECPreKey(1, aciKeyPair), KeysHelper.signedECPreKey(2, pniKeyPair), KeysHelper.signedKEMPreKey(3, aciKeyPair), - KeysHelper.signedKEMPreKey(4, pniKeyPair))), + KeysHelper.signedKEMPreKey(4, pniKeyPair)), + null), a -> { a.setUnidentifiedAccessKey(new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); a.removeDevice(Device.PRIMARY_ID); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index b456273b6..3e0dae3b9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -1556,6 +1556,7 @@ private Account createAccount(final String e164, final AccountAttributes account KeysHelper.signedECPreKey(1, aciKeyPair), KeysHelper.signedECPreKey(2, pniKeyPair), KeysHelper.signedKEMPreKey(3, aciKeyPair), - KeysHelper.signedKEMPreKey(4, pniKeyPair))); + KeysHelper.signedKEMPreKey(4, pniKeyPair)), + null); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java index f273685a3..7c4b9c026 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/util/AccountsHelper.java @@ -210,6 +210,7 @@ public static Account createAccount(final AccountsManager accountsManager, KeysHelper.signedECPreKey(1, aciKeyPair), KeysHelper.signedECPreKey(2, pniKeyPair), KeysHelper.signedKEMPreKey(3, aciKeyPair), - KeysHelper.signedKEMPreKey(4, pniKeyPair))); + KeysHelper.signedKEMPreKey(4, pniKeyPair)), + null); } }