From b32212bf9b40be8e482f204176e778fff7fcabc5 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 31 Mar 2025 12:16:58 +0100 Subject: [PATCH] Rename `IgnoreNoResponseMetricsCollector` Originally this metrics collector was just there to ignore API calls that didn't make it all the way to S3, but (a) it doesn't really do that because it also apparently ignores 4xx responses and (b) it also does a bunch of other metrics collection too. `IgnoreNoResponseMetricsCollector` is definitely the wrong name these days so this commit renames it to something more general. --- .../s3/S3BlobStoreRepositoryTests.java | 2 +- .../repositories/s3/S3BlobStore.java | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index 6874ebdf3b5c0..bb25b85f3763b 100644 --- a/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -285,7 +285,7 @@ public void testMetrics() throws Exception { final BlobStore blobStore = blobStoreRepository.blobStore(); final BlobStore delegateBlobStore = ((BlobStoreWrapper) blobStore).delegate(); final S3BlobStore s3BlobStore = (S3BlobStore) delegateBlobStore; - final Map statsCollectors = s3BlobStore + final Map statsCollectors = s3BlobStore .getStatsCollectors().collectors; final var plugins = internalCluster().getInstance(PluginsService.class, nodeName) diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java index 3b5c1e317b135..b96a7c009dc7b 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java @@ -143,16 +143,17 @@ public TimeValue getCompareAndExchangeAntiContentionDelay() { return service.compareAndExchangeAntiContentionDelay; } - // metrics collector that ignores null responses that we interpret as the request not reaching the S3 endpoint due to a network - // issue - class IgnoreNoResponseMetricsCollector extends RequestMetricCollector { + /** + * A {@link RequestMetricCollector} that processes the metrics related to each API invocation attempt according to Elasticsearch's needs + */ + class ElasticsearchS3MetricsCollector extends RequestMetricCollector { final LongAdder requests = new LongAdder(); final LongAdder operations = new LongAdder(); private final Operation operation; private final Map attributes; - private IgnoreNoResponseMetricsCollector(Operation operation, OperationPurpose purpose) { + private ElasticsearchS3MetricsCollector(Operation operation, OperationPurpose purpose) { this.operation = operation; this.attributes = RepositoriesMetrics.createAttributesMap(repositoryMetadata, purpose, operation.getKey()); } @@ -582,7 +583,7 @@ public String toString() { } class StatsCollectors { - final Map collectors = new ConcurrentHashMap<>(); + final Map collectors = new ConcurrentHashMap<>(); RequestMetricCollector getMetricCollector(Operation operation, OperationPurpose purpose) { return collectors.computeIfAbsent(new StatsKey(operation, purpose), k -> buildMetricCollector(k.operation(), k.purpose())); @@ -605,8 +606,8 @@ Map statsMap(boolean isStateless) { } } - IgnoreNoResponseMetricsCollector buildMetricCollector(Operation operation, OperationPurpose purpose) { - return new IgnoreNoResponseMetricsCollector(operation, purpose); + ElasticsearchS3MetricsCollector buildMetricCollector(Operation operation, OperationPurpose purpose) { + return new ElasticsearchS3MetricsCollector(operation, purpose); } }