Reduce Regex object creation in SignalFx Naming Convention (#3747)

Reduces object creation while keeping the same behavior as before and adds more tests. Resolves gh-3745 Co-authored-by: Antoine Toulme <antoine@toulme.name>
micrometer-metrics · Apr 25, 2023 · e2e603a · e2e603a
1 parent e5074a7
commit e2e603a
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 23 deletions.
diff --git a/...eter-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalFxNamingConvention.java b/...eter-registry-signalfx/src/main/java/io/micrometer/signalfx/SignalFxNamingConvention.java
@@ -27,26 +27,18 @@
 /**
  * {@link NamingConvention} for SignalFx.
  *
- * See
- * https://developers.signalfx.com/metrics/data_ingest_overview.html#_criteria_for_metric_and_dimension_names_and_values
- *
+ * @see <a href=
+ * "https://docs.splunk.com/Observability/metrics-and-metadata/metric-names.html"> Naming
+ * conventions for metrics and dimensions</a>
  * @author Jon Schneider
  * @author Johnny Lim
  */
 public class SignalFxNamingConvention implements NamingConvention {
 
     private static final WarnThenDebugLogger logger = new WarnThenDebugLogger(SignalFxNamingConvention.class);
 
-    private static final Pattern START_UNDERSCORE_PATTERN = Pattern.compile("^_");
-
-    private static final Pattern SF_PATTERN = Pattern.compile("^sf_");
-
-    private static final Pattern START_LETTERS_PATTERN = Pattern.compile("^[a-zA-Z].*");
-
     private static final Pattern PATTERN_TAG_KEY_DENYLISTED_CHARS = Pattern.compile("[^\\w_\\-]");
 
-    private static final Pattern PATTERN_TAG_KEY_DENYLISTED_PREFIX = Pattern.compile("^(aws|gcp|azure)_.*");
-
     private static final int NAME_MAX_LENGTH = 256;
 
     private static final int TAG_VALUE_MAX_LENGTH = 256;
@@ -80,22 +72,48 @@ public String name(String name, Meter.Type type, @Nullable String baseUnit) {
     @Override
     public String tagKey(String key) {
         String conventionKey = delegate.tagKey(key);
+        conventionKey = PATTERN_TAG_KEY_DENYLISTED_CHARS.matcher(conventionKey).replaceAll("_");
+
+        if (conventionKey.length() < 1) {
+            return conventionKey;
+        }
 
-        conventionKey = START_UNDERSCORE_PATTERN.matcher(conventionKey).replaceAll(""); // 2
-        conventionKey = SF_PATTERN.matcher(conventionKey).replaceAll(""); // 2
+        int i = 0;
+        while (conventionKey.length() > i) {
+            if (conventionKey.startsWith("sf_", i)) {
+                i += 3;
+                continue;
+            }
+            if (conventionKey.startsWith("_", i)) {
+                i += 1;
+                continue;
+            }
+            break;
+        }
 
-        conventionKey = PATTERN_TAG_KEY_DENYLISTED_CHARS.matcher(conventionKey).replaceAll("_");
-        if (!START_LETTERS_PATTERN.matcher(conventionKey).matches()) { // 3
+        if (i > 0) {
+            conventionKey = conventionKey.substring(i);
+            if (conventionKey.length() < 1) {
+                return conventionKey;
+            }
+        }
+
+        if (!Character.isLetter(conventionKey.charAt(0))) {
+            logger.log(conventionKey
+                    + " doesn't adhere to SignalFx naming standards. Prefixing the tag/dimension key with 'a'.");
             conventionKey = "a" + conventionKey;
         }
-        if (PATTERN_TAG_KEY_DENYLISTED_PREFIX.matcher(conventionKey).matches()) {
+
+        if (conventionKey.startsWith("aws_") || conventionKey.startsWith("gcp_")
+                || conventionKey.startsWith("azure_")) {
             String finalConventionKey = conventionKey;
             logger.log(() -> "'" + finalConventionKey + "' (original name: '" + key + "') is not a valid tag key. "
                     + "Must not start with any of these prefixes: aws_, gcp_, or azure_. "
                     + "Please rename it to conform to the constraints. "
                     + "If it comes from a third party, please use MeterFilter to rename it.");
         }
-        return StringUtils.truncate(conventionKey, KEY_MAX_LENGTH); // 1
+
+        return StringUtils.truncate(conventionKey, KEY_MAX_LENGTH);
     }
 
     // Dimension value can be any non-empty UTF-8 string, with a maximum length <= 256

diff --git a/...-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxNamingConventionTest.java b/...-registry-signalfx/src/test/java/io/micrometer/signalfx/SignalFxNamingConventionTest.java
@@ -16,6 +16,8 @@
 package io.micrometer.signalfx;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -27,19 +29,30 @@
  */
 class SignalFxNamingConventionTest {
 
-    private SignalFxNamingConvention convention = new SignalFxNamingConvention();
+    private final SignalFxNamingConvention convention = new SignalFxNamingConvention();
 
-    @Test
-    void tagKey() {
-        assertThat(convention.tagKey("_boo")).isEqualTo("boo");
-        assertThat(convention.tagKey("sf_boo")).isEqualTo("boo");
+    @ParameterizedTest
+    @ValueSource(strings = { "_boo", "__boo", "sf_boo", "sf__boo", "sf_sf_boo", "sf.boo", "àboo", "sf_àboo", "boo" })
+    void tagKeyShouldStartWithAlphabet(String key) {
+        assertThat(convention.tagKey(key)).isEqualTo("boo");
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "123", "_123", "sf_123", "sf.123", "_.123" })
+    void tagKeyShouldBePrefixed(String key) {
+        assertThat(convention.tagKey(key)).isEqualTo("a123");
+    }
 
-        assertThat(convention.tagKey("123")).isEqualTo("a123");
+    @ParameterizedTest
+    @ValueSource(strings = { "", "_", ".", "à", "sf_", "sf_.", "sf_à" })
+    void tagKeyShouldBeEmpty(String key) {
+        assertThat(convention.tagKey(key)).isEmpty();
     }
 
     @Test
     void tagKeyWhenKeyHasDenylistedCharShouldSanitize() {
         assertThat(convention.tagKey("a.b")).isEqualTo("a_b");
+        assertThat(convention.tagKey("booàboo")).isEqualTo("boo_boo");
     }
 
 }