Merge pull request #420 from Aiven-Open/jeqo/fix-gcs-metrics

fix: capture GCS metrics when bucket contains hyphens

docker/kafka-jmx-exporter.yml

@@ -127,3 +127,15 @@ rules:
   - pattern: aiven.kafka.server.tieredstorage<type=remote-storage-manager-metrics><>(.+-total|.+-rate|.+-avg|.+-max)
     name: kafka_tiered_storage_rsm_$1
     type: GAUGE
+  - pattern: aiven.kafka.server.tieredstorage.cache<type=segment-manifest-cache><>(.+-total|.+-rate|.+-avg|.+-max)
+    name: kafka_tiered_storage_cache_segment_manifest_$1
+    type: GAUGE
+  - pattern: aiven.kafka.server.tieredstorage.cache<type=chunk-cache><>(.+-total|.+-rate|.+-avg|.+-max)
+    name: kafka_tiered_storage_cache_chunk_$1
+    type: GAUGE
+  - pattern: aiven.kafka.server.tieredstorage.s3<type=s3-metrics><>(.+-total|.+-rate|.+-avg|.+-max)
+    name: kafka_tiered_storage_backend_s3_$1
+    type: GAUGE
+  - pattern: aiven.kafka.server.tieredstorage.gcs<type=gcs-metrics><>(.+-total|.+-rate|.+-avg|.+-max)
+    name: kafka_tiered_storage_backend_gcs_$1
+    type: GAUGE

storage/gcs/src/main/java/io/aiven/kafka/tieredstorage/storage/gcs/GcsStorage.java

@@ -36,8 +36,6 @@
 import com.google.cloud.storage.StorageOptions;
 
 public class GcsStorage implements StorageBackend {
-    private final MetricCollector metricCollector = new MetricCollector();
-
     private Storage storage;
     private String bucketName;
     private Integer resumableUploadChunkSize;
@@ -48,7 +46,7 @@ public void configure(final Map<String, ?> configs) {
         this.bucketName = config.bucketName();
         final StorageOptions.Builder builder = StorageOptions.newBuilder()
             .setCredentials(config.credentials())
-            .setTransportOptions(metricCollector.httpTransportOptions());
+            .setTransportOptions(new MetricCollector().httpTransportOptions());
         if (config.endpointUrl() != null) {
             builder.setHost(config.endpointUrl());
         }

storage/gcs/src/main/java/io/aiven/kafka/tieredstorage/storage/gcs/MetricCollector.java

@@ -45,24 +45,24 @@ class MetricCollector {
      *
      * <p>That is, {@literal "/storage/v1/b/<bucket>/o/<object>"}.
      */
-    private static final Pattern OBJECT_METADATA_PATH_PATTERN =
-        Pattern.compile("^/storage/v1/b/\\w+/o/\\w+/?$");
+    static final Pattern OBJECT_METADATA_PATH_PATTERN =
+        Pattern.compile("^/storage/v1/b/([^/]+)/o/([^/]+)/?$");
 
     /**
      * The pattern for object download paths.
      *
      * <p>That is, {@literal "/download/storage/v1/b/<bucket>/o/<object>"}.
      */
-    private static final Pattern OBJECT_DOWNLOAD_PATH_PATTERN =
-        Pattern.compile("^/download/storage/v1/b/\\w+/o/\\w+/?$");
+    static final Pattern OBJECT_DOWNLOAD_PATH_PATTERN =
+        Pattern.compile("^/download/storage/v1/b/([^/]+)/o/([^/]+)/?$");
 
     /**
      * The pattern for object upload paths.
      *
      * <p>That is, {@literal "/upload/storage/v1/b/<bucket>/o"}.
      */
-    private static final Pattern OBJECT_UPLOAD_PATH_PATTERN =
-        Pattern.compile("^/upload/storage/v1/b/\\w+/o/?$");
+    static final Pattern OBJECT_UPLOAD_PATH_PATTERN =
+        Pattern.compile("^/upload/storage/v1/b/([^/]+)/o/?$");
 
     private static final String METRIC_GROUP = "gcs-metrics";
 

storage/gcs/src/test/java/io/aiven/kafka/tieredstorage/storage/gcs/MetricCollectorRegexTest.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2023 Aiven Oy
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.aiven.kafka.tieredstorage.storage.gcs;
+
+import org.junit.jupiter.api.Test;
+
+import static io.aiven.kafka.tieredstorage.storage.gcs.MetricCollector.OBJECT_DOWNLOAD_PATH_PATTERN;
+import static io.aiven.kafka.tieredstorage.storage.gcs.MetricCollector.OBJECT_METADATA_PATH_PATTERN;
+import static io.aiven.kafka.tieredstorage.storage.gcs.MetricCollector.OBJECT_UPLOAD_PATH_PATTERN;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class MetricCollectorRegexTest {
+
+    @Test
+    void validBucketName() {
+        final var bucketName = "test-bucket_with.all_chars123";
+        final var prefix = "tiered-storage-demo";
+        final var topic = "topic1";
+        final var topicId = "_dCovw9-QaebnUIeIsIJvg";
+        final var partition = "0";
+        assertThat(OBJECT_METADATA_PATH_PATTERN.matcher(
+            "/storage/v1/b/"
+                + bucketName
+                + "/o/"
+                + prefix + "%2F"
+                + topic + "-" + topicId + "%2F" + partition
+                + "%2F"
+                + "00000000000000023511-mhTGflMpQJyceHAOQja1sw.indexes"
+        )).matches();
+        assertThat(OBJECT_DOWNLOAD_PATH_PATTERN.matcher(
+            "/download/storage/v1/b/"
+                + bucketName
+                + "/o/"
+                + prefix + "%2F"
+                + topic + "-" + topicId + "%2F" + partition
+                + "%2F"
+                + "00000000000000023511-mhTGflMpQJyceHAOQja1sw.indexes"
+        )).matches();
+        assertThat(OBJECT_UPLOAD_PATH_PATTERN.matcher(
+            "/upload/storage/v1/b/"
+                + bucketName
+                + "/o"
+        )).matches();
+    }
+
+    @Test
+    void invalidBucketName() {
+        final var bucketName = "test/invalid";
+        final var prefix = "tiered-storage-demo";
+        final var topic = "topic1";
+        final var topicId = "_dCovw9-QaebnUIeIsIJvg";
+        final var partition = "0";
+        assertThat(OBJECT_METADATA_PATH_PATTERN.matcher(
+            "/storage/v1/b/"
+                + bucketName
+                + "/o/"
+                + prefix + "%2F"
+                + topic + "-" + topicId + "%2F" + partition
+                + "%2F"
+                + "00000000000000023511-mhTGflMpQJyceHAOQja1sw.indexes"
+        ).matches()).isFalse();
+        assertThat(OBJECT_DOWNLOAD_PATH_PATTERN.matcher(
+            "/download/storage/v1/b/"
+                + bucketName
+                + "/o/"
+                + prefix + "%2F"
+                + topic + "-" + topicId + "%2F" + partition
+                + "%2F"
+                + "00000000000000023511-mhTGflMpQJyceHAOQja1sw.indexes"
+        ).matches()).isFalse();
+        assertThat(OBJECT_UPLOAD_PATH_PATTERN.matcher(
+            "/upload/storage/v1/b/"
+                + bucketName
+                + "/o"
+        ).matches()).isFalse();
+    }
+}