diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java index e96767116..0897b8efe 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java @@ -28,7 +28,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Stream; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,8 +80,8 @@ public class MetricServerInterceptor implements ServerInterceptor { tagList.add(Tag.of(TAG_METHOD_TYPE, call.getMethodDescriptor().getType().name())); RequestAttributesUtil.getUserAgent() - .map(UserAgentTagUtil::getLibsignalAndPlatformTags) - .ifPresent(tagList::addAll); + .map(UserAgentTagUtil::getPlatformTag) + .ifPresent(tagList::add); userAgentString .flatMap(ua -> UserAgentTagUtil.getClientVersionTag(ua, clientReleaseManager)) .ifPresent(tagList::add); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java index 03bf14466..c2526d534 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java @@ -141,7 +141,7 @@ public class MetricsHttpChannelListener implements HttpChannel.Listener, Contain tags.add(Tag.of(METHOD_TAG, requestInfo.method())); tags.add(Tag.of(STATUS_CODE_TAG, String.valueOf(requestInfo.statusCode()))); tags.add(Tag.of(TRAFFIC_SOURCE_TAG, TrafficSource.HTTP.name().toLowerCase())); - tags.addAll(UserAgentTagUtil.getLibsignalAndPlatformTags(requestInfo.userAgent())); + tags.add(UserAgentTagUtil.getPlatformTag(requestInfo.userAgent())); final Optional maybeClientVersionTag = UserAgentTagUtil.getClientVersionTag(requestInfo.userAgent, clientReleaseManager); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java index 9abd2e651..ca86af361 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListener.java @@ -17,7 +17,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; import javax.annotation.Nullable; -import org.eclipse.jetty.io.ArrayByteBufferPool; import org.glassfish.jersey.server.ContainerResponse; import org.glassfish.jersey.server.monitoring.RequestEvent; import org.glassfish.jersey.server.monitoring.RequestEventListener; @@ -106,7 +105,7 @@ public class MetricsRequestEventListener implements RequestEventListener { userAgent = userAgentValues != null && !userAgentValues.isEmpty() ? userAgentValues.getFirst() : null; } - tags.addAll(UserAgentTagUtil.getLibsignalAndPlatformTags(userAgent)); + tags.add(UserAgentTagUtil.getPlatformTag(userAgent)); final Optional maybeClientVersionTag = UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/UserAgentTagUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/UserAgentTagUtil.java index d840da5e3..c44124f92 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/UserAgentTagUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/UserAgentTagUtil.java @@ -6,18 +6,16 @@ package org.whispersystems.textsecuregcm.metrics; import io.micrometer.core.instrument.Tag; -import java.util.List; -import java.util.Optional; -import java.util.UUID; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.HttpHeaders; -import org.apache.commons.lang3.StringUtils; +import java.util.Optional; +import java.util.UUID; +import javax.annotation.Nullable; import org.whispersystems.textsecuregcm.WhisperServerVersion; import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException; import org.whispersystems.textsecuregcm.util.ua.UserAgent; import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil; -import javax.annotation.Nullable; /** * Utility class for extracting platform/version metrics tags from User-Agent strings. @@ -26,7 +24,6 @@ public class UserAgentTagUtil { public static final String PLATFORM_TAG = "platform"; public static final String VERSION_TAG = "clientVersion"; - public static final String LIBSIGNAL_TAG = "libsignal"; public static final String SERVER_UA = String.format("Signal-Server/%s (%s)", WhisperServerVersion.getServerVersion(), UUID.randomUUID()); @@ -70,21 +67,4 @@ public class UserAgentTagUtil { return Optional.empty(); } - - public static List getLibsignalAndPlatformTags(final String userAgentString) { - String platform; - boolean libsignal; - - try { - final UserAgent userAgent = UserAgentUtil.parseUserAgentString(userAgentString); - platform = userAgent.platform().name().toLowerCase(); - libsignal = StringUtils.contains(userAgent.additionalSpecifiers(), "libsignal"); - } catch (final UnrecognizedUserAgentException e) { - platform = "unrecognized"; - libsignal = false; - } - - return List.of(Tag.of(PLATFORM_TAG, platform), Tag.of(LIBSIGNAL_TAG, String.valueOf(libsignal))); - } - } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java index faafc2412..0c3619e05 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java @@ -104,7 +104,6 @@ public class MetricServerInterceptorTest { final Tags commonTags = Tags.of( "platform", "android", - "libsignal", "true", "grpcService", "org.signal.chat.rpc.EchoService", "method", "echo"); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java index 40b7d040d..4cabe364b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java @@ -157,14 +157,13 @@ class MetricsHttpChannelListenerIntegrationTest { tags.add(tag); } - assertEquals(6, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.PATH_TAG, expectedTagPath))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(expectedStatus)))); - assertTrue( - tags.contains(Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); + assertEquals(Set.of( + Tag.of(MetricsHttpChannelListener.PATH_TAG, expectedTagPath), + Tag.of(MetricsHttpChannelListener.METHOD_TAG, "GET"), + Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(expectedStatus)), + Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android")), + tags); } @@ -221,14 +220,13 @@ class MetricsHttpChannelListenerIntegrationTest { tags.add(tag); } - assertEquals(6, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.PATH_TAG, "/v1/websocket"))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(101)))); - assertTrue( - tags.contains(Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); + assertEquals(Set.of( + Tag.of(MetricsHttpChannelListener.PATH_TAG, "/v1/websocket"), + Tag.of(MetricsHttpChannelListener.METHOD_TAG, "GET"), + Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(101)), + Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android")), + tags); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java index 0696a9e33..b7a299e2e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerTest.java @@ -18,18 +18,17 @@ import com.google.common.net.HttpHeaders; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; -import io.micrometer.core.instrument.Tags; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.glassfish.jersey.server.ExtendedUriInfo; import org.glassfish.jersey.uri.UriTemplate; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; @@ -113,15 +112,18 @@ class MetricsHttpChannelListenerTest { tags.add(tag); } - assertEquals(versionActive ? 7 : 6, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.PATH_TAG, path))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.METHOD_TAG, method))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(statusCode)))); - assertTrue( - tags.contains(Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TrafficSource.HTTP.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); - assertEquals(versionActive, tags.contains(Tag.of(UserAgentTagUtil.VERSION_TAG, "6.53.7"))); + final Set expectedTags = new HashSet<>(Set.of( + Tag.of(MetricsHttpChannelListener.PATH_TAG, path), + Tag.of(MetricsHttpChannelListener.METHOD_TAG, method), + Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(statusCode)), + Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TrafficSource.HTTP.name().toLowerCase()), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); + + if (versionActive) { + expectedTags.add(Tag.of(UserAgentTagUtil.VERSION_TAG, "6.53.7")); + } + + assertEquals(expectedTags, tags); } @ParameterizedTest diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java index d289ce86f..46f3bcace 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsRequestEventListenerTest.java @@ -7,7 +7,6 @@ package org.whispersystems.textsecuregcm.metrics; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -142,15 +141,19 @@ class MetricsRequestEventListenerTest { tags.add(tag); } - assertEquals(versionActive ? 8 : 7, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.PATH_TAG, path))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.METHOD_TAG, method))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(statusCode)))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "false"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "true"))); - assertEquals(versionActive, tags.contains(Tag.of(UserAgentTagUtil.VERSION_TAG, "7.6.2"))); + final Set expectedTags = new HashSet<>(Set.of( + Tag.of(MetricsRequestEventListener.PATH_TAG, path), + Tag.of(MetricsRequestEventListener.METHOD_TAG, method), + Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(statusCode)), + Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "false"), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); + + if (versionActive) { + expectedTags.add(Tag.of(UserAgentTagUtil.VERSION_TAG, "7.6.2")); + } + + assertEquals(expectedTags, tags); } @Test @@ -211,15 +214,15 @@ class MetricsRequestEventListenerTest { tags.add(tag); } - assertEquals(8, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "true"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); + assertEquals(Set.of( + Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"), + Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"), + Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)), + Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "true"), + Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android")), + tags); } @Test @@ -278,15 +281,15 @@ class MetricsRequestEventListenerTest { tags.add(tag); } - assertEquals(8, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "true"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "unrecognized"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); + assertEquals(Set.of( + Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"), + Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"), + Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)), + Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, "true"), + Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "unrecognized")), + tags); } @@ -348,15 +351,15 @@ class MetricsRequestEventListenerTest { tags.add(tag); } - assertEquals(8, tags.size()); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, String.valueOf(authenticated)))); - assertTrue(tags.contains(Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "unrecognized"))); - assertTrue(tags.contains(Tag.of(UserAgentTagUtil.LIBSIGNAL_TAG, "false"))); + assertEquals(Set.of( + Tag.of(MetricsRequestEventListener.PATH_TAG, "/v1/test/hello"), + Tag.of(MetricsRequestEventListener.METHOD_TAG, "GET"), + Tag.of(MetricsRequestEventListener.STATUS_CODE_TAG, String.valueOf(200)), + Tag.of(MetricsRequestEventListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()), + Tag.of(MetricsRequestEventListener.AUTHENTICATED_TAG, String.valueOf(authenticated)), + Tag.of(MetricsRequestEventListener.LISTEN_PORT_TAG, Integer.toString(LISTEN_PORT)), + Tag.of(UserAgentTagUtil.PLATFORM_TAG, "unrecognized")), + tags); } private static SubProtocol.WebSocketResponseMessage getResponse(ArgumentCaptor responseCaptor)