diff --git a/spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/http/PathSanitizer.java b/spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/http/PathSanitizer.java index f351ad9e0..81bebe5ad 100644 --- a/spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/http/PathSanitizer.java +++ b/spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/http/PathSanitizer.java @@ -24,6 +24,8 @@ */ public final class PathSanitizer { + private static final int MAX_LENGTH = 120; + private static final AsciiSet ALPHA_CHARS = AsciiSet.fromPattern("a-zA-Z"); private static final AsciiSet DIGITS = AsciiSet.fromPattern("0-9"); @@ -35,11 +37,15 @@ private PathSanitizer() { /** Returns a sanitized path string for use as an endpoint tag value. */ public static String sanitize(String path) { - return sanitizeSegments(removeMatixParameters(path)); + return sanitizeSegments(removeParameters(path)); + } + + private static String removeParameters(String path) { + return removeParameters(removeParameters(path, '?'), ';'); } - private static String removeMatixParameters(String path) { - int i = path.indexOf(';'); + private static String removeParameters(String path, char c) { + int i = path.indexOf(c); return i > 0 ? path.substring(0, i) : path; } @@ -62,9 +68,9 @@ private static String sanitizeSegments(String path) { if (!segment.isEmpty()) { if (shouldSuppressSegment(segment)) - builder.append("_-"); + appendIfSpaceAvailable(builder, "-"); else - builder.append('_').append(segment); + appendIfSpaceAvailable(builder, segment); ++segmentsAdded; } } @@ -102,4 +108,13 @@ private static boolean shouldSuppressSegment(String segment) { return !version && n == 2; } + + private static void appendIfSpaceAvailable(StringBuilder builder, String segment) { + int spaceRemaining = MAX_LENGTH - builder.length() - 1; + if (segment.length() < spaceRemaining) { + builder.append('_').append(segment); + } else if (spaceRemaining >= 2) { + builder.append("_-"); + } + } } diff --git a/spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/http/PathSanitizerTest.java b/spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/http/PathSanitizerTest.java index efc08de42..7cf0eee6d 100644 --- a/spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/http/PathSanitizerTest.java +++ b/spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/http/PathSanitizerTest.java @@ -47,6 +47,12 @@ public void matrixParameters() { Assertions.assertEquals("_api_v1_foo", sanitize(path)); } + @Test + public void requestParameters() { + String path = "/api/v1/foo?user=bob&version=1"; + Assertions.assertEquals("_api_v1_foo", sanitize(path)); + } + @Test public void decimalNumbers() { String path = "/api/v1/foo/1234567890/123"; @@ -240,4 +246,18 @@ public void allowMostWords() throws Exception { "suppressed " + suppressed + " of " + total); } } + + @Test + public void restrictsLength() { + // Alternate consonants and vowels to avoid consonant run check + String pattern = "abecidofug"; + StringBuilder path = new StringBuilder().append('/'); + for (int i = 0; i < 100; ++i) { + path.append(pattern); + } + Assertions.assertEquals("_-", PathSanitizer.sanitize(path.toString())); + + path.append("/foo/bar/baz"); + Assertions.assertEquals("_-_foo_bar_baz", PathSanitizer.sanitize(path.toString())); + } }