Merge pull request #599 from govuk-one-login/enhance-clean

feat(EventProbe): error handling + enhance clean

Author: surajkumar

build.gradle

@@ -11,7 +11,7 @@ plugins {
 }
 
 // Important: see README on publishing a new version to Maven
-def buildVersion = "8.0.1"
+def buildVersion = "8.0.2"
 
 defaultTasks 'clean', 'spotlessApply', 'build'
 

src/main/java/uk/gov/di/ipv/cri/common/library/util/EventProbe.java

@@ -50,17 +50,21 @@ private void logErrorCause(Throwable throwable) {
     }
 
     public EventProbe counterMetric(String key) {
-        metrics.addMetric(key, 1d);
+        addMetric(key, 1d);
         return this;
     }
 
     public EventProbe counterMetric(String key, double value) {
-        metrics.addMetric(key, value);
+        addMetric(key, value);
         return this;
     }
 
     public EventProbe counterMetric(String key, double value, MetricUnit unit) {
-        metrics.addMetric(key, value, unit);
+        try {
+            metrics.addMetric(key, value, unit);
+        } catch (Exception e) {
+            LOGGER.error("Counter metric failed", e);
+        }
         return this;
     }
 
@@ -83,16 +87,86 @@ public EventProbe addJourneyIdToLoggingContext(String journeyId) {
 
     public void addDimensions(Map<String, String> dimensions) {
         if (dimensions != null) {
-            DimensionSet dimensionSet = new DimensionSet();
-            dimensions.forEach(dimensionSet::addDimension);
-            metrics.addDimension(dimensionSet);
+            DimensionSet dimensionSet = newDimensionSet();
+            dimensions.forEach(
+                    (key, value) -> {
+                        try {
+                            dimensionSet.addDimension(key, value);
+                        } catch (Exception e) {
+                            LOGGER.error("Metric failed validation: {}={}", key, value, e);
+                        }
+                    });
+            try {
+                metrics.addDimension(dimensionSet);
+            } catch (Exception e) {
+                LOGGER.error("Failed to add dimensions", e);
+            }
+        }
+    }
+
+    private void addMetric(String key, double value) {
+        try {
+            metrics.addMetric(key, value);
+        } catch (Exception e) {
+            LOGGER.error("Failed to add metric: {}, trying again with clean", key, e);
         }
     }
 
-    public static String clean(String metricValue) {
-        if (metricValue == null || metricValue.isBlank()) {
+    public static String clean(String value) {
+        final int maxLen = 250;
+
+        if (value == null || value.isBlank()) {
+            return "no_content";
+        }
+
+        String stripped = removePrefixedColons(value);
+        char[] chars = stripped.toCharArray();
+
+        if (chars.length == 0 || stripped.isBlank()) {
             return "no_content";
         }
-        return metricValue.replaceAll("\\s+", "_").trim();
+
+        StringBuilder sb = new StringBuilder();
+
+        for (char c : chars) {
+            if (Character.isWhitespace(c)) {
+                sb.append("_");
+            } else if (isAsciiPrintable(c)) {
+                sb.append(c);
+            } else {
+                sb.append("_");
+            }
+        }
+
+        String cleaned = sb.toString();
+
+        if (cleaned.length() > maxLen) {
+            cleaned = cleaned.substring(0, maxLen);
+        }
+
+        String result = cleaned.replaceAll("\\s+", "_").trim();
+        if (!result.equalsIgnoreCase(value)) {
+            LOGGER.warn(
+                    "Dimension value of {} has been transformed into {} to match Powertools validations",
+                    value,
+                    result);
+        }
+        return result;
+    }
+
+    private static String removePrefixedColons(String value) {
+        if (value.startsWith(":")) {
+            return removePrefixedColons(value.substring(1));
+        } else {
+            return value;
+        }
+    }
+
+    private static boolean isAsciiPrintable(final char ch) {
+        return ch >= 32 && ch < 127;
+    }
+
+    public DimensionSet newDimensionSet() {
+        return new DimensionSet();
     }
 }

src/test/java/uk/gov/di/ipv/cri/common/library/util/EventProbeTest.java

@@ -14,21 +14,28 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.slf4j.MDC;
 import software.amazon.lambda.powertools.metrics.Metrics;
+import software.amazon.lambda.powertools.metrics.internal.Validator;
 import software.amazon.lambda.powertools.metrics.model.DimensionSet;
 import software.amazon.lambda.powertools.metrics.model.MetricUnit;
 
 import java.util.Map;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
 @ExtendWith(MockitoExtension.class)
@@ -203,7 +210,79 @@ void shouldCleanMetricDimension() {
         assertEquals("this_has_whitespace", EventProbe.clean("this has whitespace"));
         assertEquals("correct_value", EventProbe.clean("correct_value"));
         assertEquals("no_content", EventProbe.clean(""));
+        assertEquals("no_content", EventProbe.clean("  "));
         assertEquals("no_content", EventProbe.clean(null));
         assertEquals("test_test", EventProbe.clean("test\ntest"));
+        assertEquals("test", EventProbe.clean(":test"));
+        assertEquals("test", EventProbe.clean("::test"));
+        assertEquals("no_content", EventProbe.clean(":"));
+        assertEquals(250, EventProbe.clean("1".repeat(300)).length());
+        assertEquals(
+                "includes_non_ascii_printable___char",
+                EventProbe.clean("includes_non_ascii_printable_¬_char"));
+    }
+
+    @Test
+    void shouldCleanAComplexString() {
+        String complexString =
+                EventProbe.clean(
+                        """
+                :Hello   world\t😀
+                éñö @@##!! 12345 \f \\slashes// ""quotes""
+                line2-with-stuff 🧨🚀✨
+                more-text---here---###$$$%%%&&&
+                control-\u0007-char
+                𝔘𝔫𝔦𝔠𝔬𝔡𝔢 block
+                final-line-with: symbols_*&^%$#@!+=()[]{}<>?/|~`
+                😀😀   end
+                """
+                                .repeat(500));
+
+        assertDoesNotThrow(() -> Validator.validateDimension("_", complexString));
+        assertEquals(250, complexString.length());
+    }
+
+    @Test
+    void shouldFallbackOnBadMetric() {
+        assertDoesNotThrow(() -> eventProbe.counterMetric("with space"));
+        assertDoesNotThrow(() -> eventProbe.counterMetric("with space", 1d));
+    }
+
+    @Test
+    void counterMetricHandlesExceptionGracefully() {
+        doThrow(new RuntimeException("error")).when(mockMetrics).addMetric("bad metric", 1d);
+
+        EventProbe result = assertDoesNotThrow(() -> eventProbe.counterMetric("bad-metric"));
+
+        verify(mockMetrics).addMetric("bad-metric", 1d);
+        assertSame(eventProbe, result);
+    }
+
+    @Test
+    void counterMetricWithValueHandlesExceptionGracefully() {
+        doThrow(new RuntimeException("error")).when(mockMetrics).addMetric("bad-metric-2", 42d);
+
+        EventProbe result = assertDoesNotThrow(() -> eventProbe.counterMetric("bad-metric-2", 42d));
+
+        verify(mockMetrics).addMetric("bad-metric-2", 42d);
+        assertSame(eventProbe, result);
+    }
+
+    @Test
+    void addDimensionsHandlesExceptionsGracefully() {
+        DimensionSet mockSet = mock(DimensionSet.class);
+
+        doThrow(new RuntimeException("error")).when(mockSet).addDimension(anyString(), anyString());
+
+        EventProbe spyProbe = spy(new EventProbe(mockMetrics));
+        doReturn(mockSet).when(spyProbe).newDimensionSet();
+
+        doThrow(new RuntimeException("error"))
+                .when(mockMetrics)
+                .addDimension(any(DimensionSet.class));
+
+        assertDoesNotThrow(() -> spyProbe.addDimensions(Map.of("bad Key", "bad Value")));
+
+        verify(mockMetrics).addDimension(mockSet);
     }
 }