From 0122b410be20a1db6f02a09c53597190feb3b6a9 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Wed, 21 Jun 2023 12:38:53 -0400 Subject: [PATCH] Include push notification urgency in push latency metrics --- .../textsecuregcm/push/MessageSender.java | 2 +- .../push/PushLatencyManager.java | 11 +++++--- .../push/PushLatencyManagerTest.java | 26 ++++++++++++++----- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java index 60e839c6c..f693c8458 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java @@ -89,7 +89,7 @@ public class MessageSender { pushNotificationManager.sendNewMessageNotification(account, device.getId(), message.getUrgent()); final boolean useVoip = StringUtils.isNotBlank(device.getVoipApnId()); - RedisOperation.unchecked(() -> pushLatencyManager.recordPushSent(account.getUuid(), device.getId(), useVoip)); + RedisOperation.unchecked(() -> pushLatencyManager.recordPushSent(account.getUuid(), device.getId(), useVoip, message.getUrgent())); } catch (final NotPushRegisteredException e) { if (!device.getFetchesMessages()) { throw e; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushLatencyManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushLatencyManager.java index 0e46f5b75..6b59e6602 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushLatencyManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushLatencyManager.java @@ -18,6 +18,7 @@ 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; @@ -60,7 +61,7 @@ public class PushLatencyManager { VOIP } - record PushRecord(Instant timestamp, PushType pushType) { + record PushRecord(Instant timestamp, PushType pushType, Optional urgent) { } public PushLatencyManager(final FaultTolerantRedisCluster redisCluster, @@ -79,10 +80,10 @@ public class PushLatencyManager { this.clock = clock; } - void recordPushSent(final UUID accountUuid, final long deviceId, final boolean isVoip) { + void recordPushSent(final UUID accountUuid, final long deviceId, final boolean isVoip, final boolean isUrgent) { try { final String recordJson = SystemMapper.jsonMapper().writeValueAsString( - new PushRecord(Instant.now(clock), isVoip ? PushType.VOIP : PushType.STANDARD)); + new PushRecord(Instant.now(clock), isVoip ? PushType.VOIP : PushType.STANDARD, Optional.of(isUrgent))); redisCluster.useCluster(connection -> connection.async().set(getFirstUnacknowledgedPushKey(accountUuid, deviceId), @@ -99,11 +100,13 @@ public class PushLatencyManager { if (pushRecord != null) { final Duration latency = Duration.between(pushRecord.timestamp(), Instant.now()); - final List tags = new ArrayList<>(2); + final List tags = new ArrayList<>(3); tags.add(UserAgentTagUtil.getPlatformTag(userAgentString)); tags.add(Tag.of("pushType", pushRecord.pushType().name().toLowerCase())); + pushRecord.urgent().ifPresent(urgent -> tags.add(Tag.of("urgent", String.valueOf(urgent)))); + try { final UserAgent userAgent = UserAgentUtil.parseUserAgentString(userAgentString); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushLatencyManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushLatencyManagerTest.java index bbf82f8c7..bb335b1bd 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushLatencyManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushLatencyManagerTest.java @@ -15,15 +15,17 @@ import java.time.Clock; import java.time.Instant; import java.time.ZoneId; import java.util.Collections; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.ExecutionException; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicPushLatencyConfiguration; -import org.whispersystems.textsecuregcm.push.PushLatencyManager; import org.whispersystems.textsecuregcm.push.PushLatencyManager.PushRecord; import org.whispersystems.textsecuregcm.push.PushLatencyManager.PushType; import org.whispersystems.textsecuregcm.redis.RedisClusterExtension; @@ -49,8 +51,8 @@ class PushLatencyManagerTest { } @ParameterizedTest - @ValueSource(booleans = {true, false}) - void testTakeRecord(final boolean isVoip) throws ExecutionException, InterruptedException { + @MethodSource + void testTakeRecord(final boolean isVoip, final boolean isUrgent) throws ExecutionException, InterruptedException { final UUID accountUuid = UUID.randomUUID(); final long deviceId = 1; @@ -61,14 +63,24 @@ class PushLatencyManagerTest { assertNull(pushLatencyManager.takePushRecord(accountUuid, deviceId).get()); - pushLatencyManager.recordPushSent(accountUuid, deviceId, isVoip); + pushLatencyManager.recordPushSent(accountUuid, deviceId, isVoip, isUrgent); final PushRecord pushRecord = pushLatencyManager.takePushRecord(accountUuid, deviceId).get(); assertNotNull(pushRecord); - assertEquals(pushTimestamp, pushRecord.getTimestamp()); - assertEquals(isVoip ? PushType.VOIP : PushType.STANDARD, pushRecord.getPushType()); + assertEquals(pushTimestamp, pushRecord.timestamp()); + assertEquals(isVoip ? PushType.VOIP : PushType.STANDARD, pushRecord.pushType()); + assertEquals(Optional.of(isUrgent), pushRecord.urgent()); assertNull(pushLatencyManager.takePushRecord(accountUuid, deviceId).get()); } + + private static Stream testTakeRecord() { + return Stream.of( + Arguments.of(true, true), + Arguments.of(true, false), + Arguments.of(false, true), + Arguments.of(false, false) + ); + } }