From 390580a19dbd332e39c1082eab699a461637f76c Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Thu, 4 Aug 2022 18:31:53 -0500 Subject: [PATCH] =?UTF-8?q?Count=20cases=20when=20the=20a=20message?= =?UTF-8?q?=E2=80=99s=20destination=20UUID=20doesn=E2=80=99t=20match=20the?= =?UTF-8?q?=20account=E2=80=99s=20PNI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../controllers/MessageController.java | 3 + .../textsecuregcm/metrics/MessageMetrics.java | 51 ++++++++ .../websocket/WebSocketConnection.java | 2 + .../metrics/MessageMetricsTest.java | 117 ++++++++++++++++++ 4 files changed, 173 insertions(+) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/metrics/MessageMetrics.java create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/metrics/MessageMetricsTest.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 6e1f893c5..6a4e4e693 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -74,6 +74,7 @@ import org.whispersystems.textsecuregcm.entities.SendMultiRecipientMessageRespon import org.whispersystems.textsecuregcm.entities.StaleDevices; import org.whispersystems.textsecuregcm.limits.RateLimitChallengeException; import org.whispersystems.textsecuregcm.limits.RateLimiters; +import org.whispersystems.textsecuregcm.metrics.MessageMetrics; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.providers.MultiRecipientMessageProvider; import org.whispersystems.textsecuregcm.push.ApnFallbackManager; @@ -423,6 +424,8 @@ public class MessageController { outgoingMessages = new OutgoingMessageEntityList(messagesAndHasMore.first().stream() .map(OutgoingMessageEntity::fromEnvelope) + .peek(outgoingMessageEntity -> MessageMetrics.measureAccountOutgoingMessageUuidMismatches(auth.getAccount(), + outgoingMessageEntity)) .collect(Collectors.toList()), messagesAndHasMore.second()); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MessageMetrics.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MessageMetrics.java new file mode 100644 index 000000000..87cc92d7d --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MessageMetrics.java @@ -0,0 +1,51 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.metrics; + +import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; + +import io.micrometer.core.instrument.Metrics; +import java.util.UUID; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.whispersystems.textsecuregcm.entities.MessageProtos; +import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity; +import org.whispersystems.textsecuregcm.storage.Account; + +public final class MessageMetrics { + + private static final Logger logger = LoggerFactory.getLogger(MessageMetrics.class); + + private static final String MISMATCHED_ACCOUNT_ENVELOPE_UUID_COUNTER_NAME = name(MessageMetrics.class, + "mismatchedAccountEnvelopeUuid"); + + public static void measureAccountOutgoingMessageUuidMismatches(final Account account, + final OutgoingMessageEntity outgoingMessage) { + measureAccountDestinationUuidMismatches(account, outgoingMessage.destinationUuid()); + } + + public static void measureAccountEnvelopeUuidMismatches(final Account account, + final MessageProtos.Envelope envelope) { + if (envelope.hasDestinationUuid()) { + try { + final UUID destinationUuid = UUID.fromString(envelope.getDestinationUuid()); + measureAccountDestinationUuidMismatches(account, destinationUuid); + } catch (final IllegalArgumentException ignored) { + logger.warn("Envelope had invalid destination UUID: {}", envelope.getDestinationUuid()); + } + } + } + + private static void measureAccountDestinationUuidMismatches(final Account account, final UUID destinationUuid) { + if (!destinationUuid.equals(account.getUuid()) && !destinationUuid.equals(account.getPhoneNumberIdentifier())) { + // In all cases, this represents a mismatch between the account’s current PNI and its PNI when the message was + // sent. This is an expected case, but if this metric changes significantly, it could indicate an issue to + // investigate. + Metrics.counter(MISMATCHED_ACCOUNT_ENVELOPE_UUID_COUNTER_NAME).increment(); + } + } + +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java index 8e3603c98..ccfbbe242 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java @@ -39,6 +39,7 @@ import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount; import org.whispersystems.textsecuregcm.controllers.MessageController; import org.whispersystems.textsecuregcm.controllers.NoSuchUserException; +import org.whispersystems.textsecuregcm.metrics.MessageMetrics; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.push.DisplacedPresenceListener; import org.whispersystems.textsecuregcm.push.ReceiptSender; @@ -195,6 +196,7 @@ public class WebSocketConnection implements MessageAvailabilityListener, Displac sendMessageMeter.mark(); sentMessageCounter.increment(); bytesSentMeter.mark(body.map(bytes -> bytes.length).orElse(0)); + MessageMetrics.measureAccountEnvelopeUuidMismatches(auth.getAccount(), message); // X-Signal-Key: false must be sent until Android stops assuming it missing means true return client.sendRequest("PUT", "/api/v1/message", List.of("X-Signal-Key: false", TimestampHeaderUtil.getTimestampHeader()), body).whenComplete((response, throwable) -> { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MessageMetricsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MessageMetricsTest.java new file mode 100644 index 000000000..3a47f4fec --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MessageMetricsTest.java @@ -0,0 +1,117 @@ +/* + * Copyright 2022 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import java.util.Optional; +import java.util.UUID; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.whispersystems.textsecuregcm.entities.MessageProtos; +import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity; +import org.whispersystems.textsecuregcm.storage.Account; + +class MessageMetricsTest { + + private final Account account = mock(Account.class); + private final UUID aci = UUID.fromString("11111111-1111-1111-1111-111111111111"); + private final UUID pni = UUID.fromString("22222222-2222-2222-2222-222222222222"); + private final UUID otherUuid = UUID.fromString("99999999-9999-9999-9999-999999999999"); + private SimpleMeterRegistry simpleMeterRegistry; + + @BeforeEach + void setup() { + when(account.getUuid()).thenReturn(aci); + when(account.getPhoneNumberIdentifier()).thenReturn(pni); + simpleMeterRegistry = new SimpleMeterRegistry(); + Metrics.globalRegistry.add(simpleMeterRegistry); + } + + @AfterEach + void teardown() { + Metrics.globalRegistry.remove(simpleMeterRegistry); + Metrics.globalRegistry.clear(); + } + + @Test + void measureAccountOutgoingMessageUuidMismatches() { + + final OutgoingMessageEntity outgoingMessageToAci = createOutgoingMessageEntity(aci); + MessageMetrics.measureAccountOutgoingMessageUuidMismatches(account, outgoingMessageToAci); + + Optional counter = findCounter(simpleMeterRegistry); + + assertTrue(counter.isEmpty()); + + final OutgoingMessageEntity outgoingMessageToPni = createOutgoingMessageEntity(pni); + MessageMetrics.measureAccountOutgoingMessageUuidMismatches(account, outgoingMessageToPni); + counter = findCounter(simpleMeterRegistry); + + assertTrue(counter.isEmpty()); + + final OutgoingMessageEntity outgoingMessageToOtherUuid = createOutgoingMessageEntity(otherUuid); + MessageMetrics.measureAccountOutgoingMessageUuidMismatches(account, outgoingMessageToOtherUuid); + counter = findCounter(simpleMeterRegistry); + + assertEquals(1.0, counter.map(Counter::count).orElse(0.0)); + } + + private OutgoingMessageEntity createOutgoingMessageEntity(UUID destinationUuid) { + return new OutgoingMessageEntity(UUID.randomUUID(), 1, 1L, null, 1, destinationUuid, null, new byte[]{}, 1, true); + } + + @Test + void measureAccountEnvelopeUuidMismatches() { + final MessageProtos.Envelope envelopeToAci = createEnvelope(aci); + MessageMetrics.measureAccountEnvelopeUuidMismatches(account, envelopeToAci); + + Optional counter = findCounter(simpleMeterRegistry); + + assertTrue(counter.isEmpty()); + + final MessageProtos.Envelope envelopeToPni = createEnvelope(pni); + MessageMetrics.measureAccountEnvelopeUuidMismatches(account, envelopeToPni); + counter = findCounter(simpleMeterRegistry); + + assertTrue(counter.isEmpty()); + + final MessageProtos.Envelope envelopeToOtherUuid = createEnvelope(otherUuid); + MessageMetrics.measureAccountEnvelopeUuidMismatches(account, envelopeToOtherUuid); + counter = findCounter(simpleMeterRegistry); + + assertEquals(1.0, counter.map(Counter::count).orElse(0.0)); + + final MessageProtos.Envelope envelopeToNull = createEnvelope(null); + MessageMetrics.measureAccountEnvelopeUuidMismatches(account, envelopeToNull); + counter = findCounter(simpleMeterRegistry); + + assertEquals(1.0, counter.map(Counter::count).orElse(0.0)); + } + + private MessageProtos.Envelope createEnvelope(UUID destinationUuid) { + final MessageProtos.Envelope.Builder builder = MessageProtos.Envelope.newBuilder(); + + if (destinationUuid != null) { + builder.setDestinationUuid(destinationUuid.toString()); + } + + return builder.build(); + } + + private Optional findCounter(SimpleMeterRegistry meterRegistry) { + final Optional maybeMeter = meterRegistry.getMeters().stream().findFirst(); + return maybeMeter.map(meter -> meter instanceof Counter ? (Counter) meter : null); + } +}