Fix DimensionSet validation to be inline with most recent specification.

Author: phipag

pom.xml

@@ -279,6 +279,11 @@
                 <artifactId>aws-embedded-metrics</artifactId>
                 <version>${aws-embedded-metrics.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.apache.commons</groupId>
+                <artifactId>commons-lang3</artifactId>
+                <version>3.15.0</version>
+            </dependency>
 
             <!-- Test dependencies -->
             <dependency>
@@ -294,12 +299,6 @@
                 <version>1.9.1</version>
                 <scope>test</scope>
             </dependency>
-            <dependency>
-                <groupId>org.apache.commons</groupId>
-                <artifactId>commons-lang3</artifactId>
-                <version>3.15.0</version>
-                <scope>test</scope>
-            </dependency>
             <dependency>
                 <groupId>org.aspectj</groupId>
                 <artifactId>aspectjweaver</artifactId>

powertools-metrics/pom.xml

@@ -65,6 +65,10 @@
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
 
         <!-- Test dependencies -->
         <dependency>
@@ -97,11 +101,6 @@
             <artifactId>junit-pioneer</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.apache.commons</groupId>
-            <artifactId>commons-lang3</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.aspectj</groupId>
             <artifactId>aspectjweaver</artifactId>

powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/model/DimensionSet.java

@@ -18,11 +18,15 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.lang3.StringUtils;
+
 /**
  * Represents a set of dimensions for CloudWatch metrics
  */
 public class DimensionSet {
-    private static final int MAX_DIMENSION_SET_SIZE = 9;
+    private static final int MAX_DIMENSION_SET_SIZE = 30;
+    private static final int MAX_DIMENSION_NAME_LENGTH = 250;
+    private static final int MAX_DIMENSION_VALUE_LENGTH = 1024;
 
     private final Map<String, String> dimensions = new LinkedHashMap<>();
 
@@ -186,12 +190,42 @@ public Map<String, String> getDimensions() {
     }
 
     private void validateDimension(String key, String value) {
-        if (key == null || key.isEmpty()) {
+        if (key == null || key.trim().isEmpty()) {
             throw new IllegalArgumentException("Dimension key cannot be null or empty");
         }
 
-        if (value == null) {
-            throw new IllegalArgumentException("Dimension value cannot be null");
+        if (value == null || value.trim().isEmpty()) {
+            throw new IllegalArgumentException("Dimension value cannot be null or empty");
+        }
+
+        if (StringUtils.containsWhitespace(key)) {
+            throw new IllegalArgumentException("Dimension key cannot contain whitespaces: " + key);
+        }
+
+        if (StringUtils.containsWhitespace(value)) {
+            throw new IllegalArgumentException("Dimension value cannot contain whitespaces: " + value);
+        }
+
+        if (key.startsWith(":")) {
+            throw new IllegalArgumentException("Dimension key cannot start with colon: " + key);
+        }
+
+        if (key.length() > MAX_DIMENSION_NAME_LENGTH) {
+            throw new IllegalArgumentException(
+                    "Dimension name exceeds maximum length of " + MAX_DIMENSION_NAME_LENGTH + ": " + key);
+        }
+
+        if (value.length() > MAX_DIMENSION_VALUE_LENGTH) {
+            throw new IllegalArgumentException(
+                    "Dimension value exceeds maximum length of " + MAX_DIMENSION_VALUE_LENGTH + ": " + value);
+        }
+
+        if (!StringUtils.isAsciiPrintable(key)) {
+            throw new IllegalArgumentException("Dimension name has invalid characters: " + key);
+        }
+
+        if (!StringUtils.isAsciiPrintable(value)) {
+            throw new IllegalArgumentException("Dimension value has invalid characters: " + value);
         }
     }
 }

powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/model/DimensionSetTest.java

@@ -152,16 +152,16 @@ void shouldReturnNullForNonExistentDimension() {
     @Test
     void shouldThrowExceptionWhenExceedingMaxDimensions() {
         // Given
-        // Create a map with 9 dimensions (9 is maximum)
-        Map<String, String> dimensions = Map.of(
-                "Key1", "Value1", "Key2", "Value2", "Key3", "Value3", "Key4", "Value4", "Key5", "Value5",
-                "Key6", "Value6", "Key7", "Value7", "Key8", "Value8", "Key9", "Value9");
-        DimensionSet dimensionSet = DimensionSet.of(dimensions);
+        // Create a dimension set with 30 dimensions (30 is maximum)
+        DimensionSet dimensionSet = new DimensionSet();
+        for (int i = 1; i <= 30; i++) {
+            dimensionSet.addDimension("Key" + i, "Value" + i);
+        }
 
         // When/Then
-        assertThatThrownBy(() -> dimensionSet.addDimension("Key10", "Value10"))
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key31", "Value31"))
                 .isInstanceOf(IllegalStateException.class)
-                .hasMessageContaining("Cannot exceed 9 dimensions per dimension set");
+                .hasMessageContaining("Cannot exceed 30 dimensions per dimension set");
     }
 
     @Test
@@ -194,6 +194,98 @@ void shouldThrowExceptionWhenValueIsNull() {
         // When/Then
         assertThatThrownBy(() -> dimensionSet.addDimension("Key", null))
                 .isInstanceOf(IllegalArgumentException.class)
-                .hasMessage("Dimension value cannot be null");
+                .hasMessage("Dimension value cannot be null or empty");
+    }
+
+    @Test
+    void shouldThrowExceptionWhenValueIsEmpty() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key", ""))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension value cannot be null or empty");
+    }
+
+    @Test
+    void shouldThrowExceptionWhenKeyContainsWhitespace() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key With Space", "Value"))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension key cannot contain whitespaces: Key With Space");
+    }
+
+    @Test
+    void shouldThrowExceptionWhenValueContainsWhitespace() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key", "Value With Space"))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension value cannot contain whitespaces: Value With Space");
+    }
+
+    @Test
+    void shouldThrowExceptionWhenKeyStartsWithColon() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension(":Key", "Value"))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension key cannot start with colon: :Key");
+    }
+
+    @Test
+    void shouldThrowExceptionWhenKeyExceedsMaxLength() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+        String longKey = "a".repeat(251); // MAX_DIMENSION_NAME_LENGTH + 1
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension(longKey, "Value"))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension name exceeds maximum length of 250: " + longKey);
+    }
+
+    @Test
+    void shouldThrowExceptionWhenValueExceedsMaxLength() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+        String longValue = "a".repeat(1025); // MAX_DIMENSION_VALUE_LENGTH + 1
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key", longValue))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension value exceeds maximum length of 1024: " + longValue);
+    }
+
+    @Test
+    void shouldThrowExceptionWhenKeyContainsNonAsciiCharacters() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+        String keyWithNonAscii = "Key\u0080"; // Non-ASCII character
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension(keyWithNonAscii, "Value"))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension name has invalid characters: " + keyWithNonAscii);
+    }
+
+    @Test
+    void shouldThrowExceptionWhenValueContainsNonAsciiCharacters() {
+        // Given
+        DimensionSet dimensionSet = DimensionSet.of(Collections.emptyMap());
+        String valueWithNonAscii = "Value\u0080"; // Non-ASCII character
+
+        // When/Then
+        assertThatThrownBy(() -> dimensionSet.addDimension("Key", valueWithNonAscii))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Dimension value has invalid characters: " + valueWithNonAscii);
     }
 }