Revert "Add diagnostic dimensions to the "get keys" counter"

This reverts commit cd64390141.
This commit is contained in:
Jon Chambers 2024-02-18 19:56:18 -05:00 committed by Jon Chambers
parent 8c55f39cdf
commit a2139ee236
3 changed files with 15 additions and 33 deletions

View File

@ -894,7 +894,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
new DirectoryV2Controller(directoryV2CredentialsGenerator), new DirectoryV2Controller(directoryV2CredentialsGenerator),
new DonationController(clock, zkReceiptOperations, redeemedReceiptsManager, accountsManager, config.getBadges(), new DonationController(clock, zkReceiptOperations, redeemedReceiptsManager, accountsManager, config.getBadges(),
ReceiptCredentialPresentation::new), ReceiptCredentialPresentation::new),
new KeysController(rateLimiters, keysManager, accountsManager, clientReleaseManager), new KeysController(rateLimiters, keysManager, accountsManager),
new MessageController(rateLimiters, messageByteLimitCardinalityEstimator, messageSender, receiptSender, new MessageController(rateLimiters, messageByteLimitCardinalityEstimator, messageSender, receiptSender,
accountsManager, messagesManager, pushNotificationManager, reportMessageManager, accountsManager, messagesManager, pushNotificationManager, reportMessageManager,
multiRecipientMessageExecutor, messageDeliveryScheduler, reportSpamTokenProvider, clientReleaseManager, multiRecipientMessageExecutor, messageDeliveryScheduler, reportSpamTokenProvider, clientReleaseManager,

View File

@ -58,7 +58,6 @@ import org.whispersystems.textsecuregcm.metrics.MetricsUtil;
import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.ClientReleaseManager;
import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.Device;
import org.whispersystems.textsecuregcm.storage.KeysManager; import org.whispersystems.textsecuregcm.storage.KeysManager;
import org.whispersystems.textsecuregcm.util.HeaderUtils; import org.whispersystems.textsecuregcm.util.HeaderUtils;
@ -73,7 +72,6 @@ public class KeysController {
private final RateLimiters rateLimiters; private final RateLimiters rateLimiters;
private final KeysManager keysManager; private final KeysManager keysManager;
private final AccountsManager accounts; 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 KEY_COUNT_DISTRIBUTION_NAME = MetricsUtil.name(KeysController.class, "getKeyCount");
private static final String GET_KEYS_COUNTER_NAME = MetricsUtil.name(KeysController.class, "getKeys"); private static final String GET_KEYS_COUNTER_NAME = MetricsUtil.name(KeysController.class, "getKeys");
@ -84,11 +82,10 @@ public class KeysController {
private static final CompletableFuture<?>[] EMPTY_FUTURE_ARRAY = new CompletableFuture[0]; private static final CompletableFuture<?>[] EMPTY_FUTURE_ARRAY = new CompletableFuture[0];
public KeysController(RateLimiters rateLimiters, KeysManager keysManager, AccountsManager accounts, ClientReleaseManager clientReleaseManager) { public KeysController(RateLimiters rateLimiters, KeysManager keysManager, AccountsManager accounts) {
this.rateLimiters = rateLimiters; this.rateLimiters = rateLimiters;
this.keysManager = keysManager; this.keysManager = keysManager;
this.accounts = accounts; this.accounts = accounts;
this.clientReleaseManager = clientReleaseManager;
} }
@GET @GET
@ -285,26 +282,14 @@ public class KeysController {
final ECPreKey unsignedEcPreKey = unsignedEcPreKeyFuture.join().orElse(null); final ECPreKey unsignedEcPreKey = unsignedEcPreKeyFuture.join().orElse(null);
final ECSignedPreKey signedEcPreKey = signedEcPreKeyFuture.join().orElse(null); final ECSignedPreKey signedEcPreKey = signedEcPreKeyFuture.join().orElse(null);
Tags tags = Tags.of( Metrics.counter(GET_KEYS_COUNTER_NAME, Tags.of(
Tag.of(PRIMARY_DEVICE_TAG_NAME, String.valueOf(device.isPrimary())), Tag.of(PRIMARY_DEVICE_TAG_NAME, String.valueOf(device.isPrimary())),
UserAgentTagUtil.getPlatformTag(userAgent), UserAgentTagUtil.getPlatformTag(userAgent),
Tag.of("targetPlatform", getDevicePlatform(device).map(Enum::name).orElse("unknown")), Tag.of("targetPlatform", getDevicePlatform(device).map(Enum::name).orElse("unknown")),
Tag.of(IDENTITY_TYPE_TAG_NAME, targetIdentifier.identityType().name()), Tag.of(IDENTITY_TYPE_TAG_NAME, targetIdentifier.identityType().name()),
Tag.of("isStale", String.valueOf(isDeviceStale(device))), Tag.of("isStale", String.valueOf(isDeviceStale(device))),
Tag.of("oneTimeEcKeyAvailable", String.valueOf(unsignedEcPreKey != null)), Tag.of("oneTimeEcKeyAvailable", String.valueOf(unsignedEcPreKey != null))))
Tag.of("authenticationType", auth.isPresent() ? "authenticated" : "anonymous")); .increment();
if (auth.isPresent()) {
tags = tags.and(Tag.of("targetIsSelf", String.valueOf(auth.get().getAccount().getUuid().equals(target.getUuid()))));
}
final Optional<Tag> 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) { if (signedEcPreKey != null || unsignedEcPreKey != null || pqPreKey != null) {
final int registrationId = switch (targetIdentifier.identityType()) { final int registrationId = switch (targetIdentifier.identityType()) {

View File

@ -32,6 +32,7 @@ import java.util.Optional;
import java.util.OptionalInt; import java.util.OptionalInt;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import javax.ws.rs.client.Entity; import javax.ws.rs.client.Entity;
import javax.ws.rs.core.MediaType; import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
@ -63,7 +64,6 @@ import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper
import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper; import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper;
import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.ClientReleaseManager;
import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.Device;
import org.whispersystems.textsecuregcm.storage.KeysManager; import org.whispersystems.textsecuregcm.storage.KeysManager;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
@ -122,10 +122,9 @@ class KeysControllerTest {
private final ECSignedPreKey VALID_DEVICE_SIGNED_KEY = KeysHelper.signedECPreKey(89898, IDENTITY_KEY_PAIR); 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 ECSignedPreKey VALID_DEVICE_PNI_SIGNED_KEY = KeysHelper.signedECPreKey(7777, PNI_IDENTITY_KEY_PAIR);
private final static KeysManager KEYS = mock(KeysManager.class); private final static KeysManager KEYS = mock(KeysManager.class );
private final static AccountsManager accounts = mock(AccountsManager.class); private final static AccountsManager accounts = mock(AccountsManager.class );
private final static Account existsAccount = mock(Account.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 RateLimiters rateLimiters = mock(RateLimiters.class);
private static final RateLimiter rateLimiter = mock(RateLimiter.class ); private static final RateLimiter rateLimiter = mock(RateLimiter.class );
@ -137,7 +136,7 @@ class KeysControllerTest {
.addProvider(new AuthValueFactoryProvider.Binder<>(AuthenticatedAccount.class)) .addProvider(new AuthValueFactoryProvider.Binder<>(AuthenticatedAccount.class))
.setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .setTestContainerFactory(new GrizzlyWebTestContainerFactory())
.addResource(new ServerRejectedExceptionMapper()) .addResource(new ServerRejectedExceptionMapper())
.addResource(new KeysController(rateLimiters, KEYS, accounts, clientReleaseManager)) .addResource(new KeysController(rateLimiters, KEYS, accounts))
.addResource(new RateLimitExceededExceptionMapper()) .addResource(new RateLimitExceededExceptionMapper())
.build(); .build();
@ -277,8 +276,6 @@ class KeysControllerTest {
when(KEYS.getEcSignedPreKey(AuthHelper.VALID_PNI, AuthHelper.VALID_DEVICE.getId())) when(KEYS.getEcSignedPreKey(AuthHelper.VALID_PNI, AuthHelper.VALID_DEVICE.getId()))
.thenReturn(CompletableFuture.completedFuture(Optional.of(VALID_DEVICE_PNI_SIGNED_KEY))); .thenReturn(CompletableFuture.completedFuture(Optional.of(VALID_DEVICE_PNI_SIGNED_KEY)));
when(clientReleaseManager.isVersionActive(any(), any())).thenReturn(true);
} }
@AfterEach @AfterEach