Skip to content

Commit 43d36ed

Browse files
nicktindallnicktindall
authored
Remove high-cardinality metric attributes (elastic#116700)
Relates: ES-10027
1 parent 270d9d2 commit 43d36ed

File tree

8 files changed

+14
-57
lines changed

8 files changed

+14
-57
lines changed

‎modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryMetricsTests.java‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void testThrottleResponsesAreCountedInMetrics() throws IOException {
112112
blobContainer.blobExists(purpose, blobName);
113113

114114
// Correct metrics are recorded
115-
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).expectMetrics()
115+
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES).expectMetrics()
116116
.withRequests(numThrottles + 1)
117117
.withThrottles(numThrottles)
118118
.withExceptions(numThrottles)
@@ -137,7 +137,7 @@ public void testRangeNotSatisfiedAreCountedInMetrics() throws IOException {
137137
assertThrows(RequestedRangeNotSatisfiedException.class, () -> blobContainer.readBlob(purpose, blobName));
138138

139139
// Correct metrics are recorded
140-
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB, repository).expectMetrics()
140+
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB).expectMetrics()
141141
.withRequests(1)
142142
.withThrottles(0)
143143
.withExceptions(1)
@@ -170,7 +170,7 @@ public void testErrorResponsesAreCountedInMetrics() throws IOException {
170170
blobContainer.blobExists(purpose, blobName);
171171

172172
// Correct metrics are recorded
173-
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).expectMetrics()
173+
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES).expectMetrics()
174174
.withRequests(numErrors + 1)
175175
.withThrottles(throttles.get())
176176
.withExceptions(numErrors)
@@ -191,7 +191,7 @@ public void testRequestFailuresAreCountedInMetrics() {
191191
assertThrows(IOException.class, () -> blobContainer.listBlobs(purpose));
192192

193193
// Correct metrics are recorded
194-
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS, repository).expectMetrics()
194+
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS).expectMetrics()
195195
.withRequests(4)
196196
.withThrottles(0)
197197
.withExceptions(4)
@@ -322,20 +322,14 @@ private void clearMetrics(String discoveryNode) {
322322
.forEach(TestTelemetryPlugin::resetMeter);
323323
}
324324

325-
private MetricsAsserter metricsAsserter(
326-
String dataNodeName,
327-
OperationPurpose operationPurpose,
328-
AzureBlobStore.Operation operation,
329-
String repository
330-
) {
331-
return new MetricsAsserter(dataNodeName, operationPurpose, operation, repository);
325+
private MetricsAsserter metricsAsserter(String dataNodeName, OperationPurpose operationPurpose, AzureBlobStore.Operation operation) {
326+
return new MetricsAsserter(dataNodeName, operationPurpose, operation);
332327
}
333328

334329
private class MetricsAsserter {
335330
private final String dataNodeName;
336331
private final OperationPurpose purpose;
337332
private final AzureBlobStore.Operation operation;
338-
private final String repository;
339333

340334
enum Result {
341335
Success,
@@ -361,11 +355,10 @@ List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, Strin
361355
abstract List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, String name);
362356
}
363357

364-
private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation, String repository) {
358+
private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation) {
365359
this.dataNodeName = dataNodeName;
366360
this.purpose = purpose;
367361
this.operation = operation;
368-
this.repository = repository;
369362
}
370363

371364
private class Expectations {
@@ -458,7 +451,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa
458451
.filter(
459452
m -> m.attributes().get("operation").equals(operation.getKey())
460453
&& m.attributes().get("purpose").equals(purpose.getKey())
461-
&& m.attributes().get("repo_name").equals(repository)
462454
&& m.attributes().get("repo_type").equals("azure")
463455
)
464456
.findFirst()
@@ -470,8 +462,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa
470462
+ operation.getKey()
471463
+ " and purpose="
472464
+ purpose.getKey()
473-
+ " and repo_name="
474-
+ repository
475465
+ " in "
476466
+ measurements
477467
)

‎modules/repository-azure/src/internalClusterTest/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,7 @@ public void testMetrics() throws Exception {
402402
)
403403
);
404404
metrics.forEach(metric -> {
405-
assertThat(
406-
metric.attributes(),
407-
allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose"))
408-
);
405+
assertThat(metric.attributes(), allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("operation"), hasKey("purpose")));
409406
final AzureBlobStore.Operation operation = AzureBlobStore.Operation.fromKey((String) metric.attributes().get("operation"));
410407
final AzureBlobStore.StatsKey statsKey = new AzureBlobStore.StatsKey(
411408
operation,

‎modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,7 @@ public void testMetrics() throws Exception {
300300
)
301301
);
302302
metrics.forEach(metric -> {
303-
assertThat(
304-
metric.attributes(),
305-
allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose"))
306-
);
303+
assertThat(metric.attributes(), allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("operation"), hasKey("purpose")));
307304
final S3BlobStore.Operation operation = S3BlobStore.Operation.parse((String) metric.attributes().get("operation"));
308305
final S3BlobStore.StatsKey statsKey = new S3BlobStore.StatsKey(
309306
operation,

‎modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,6 @@ private Map<String, Object> metricAttributes(String action) {
327327
return Map.of(
328328
"repo_type",
329329
S3Repository.TYPE,
330-
"repo_name",
331-
blobStore.getRepositoryMetadata().name(),
332330
"operation",
333331
Operation.GET_OBJECT.getKey(),
334332
"purpose",

‎modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@ private List<Measurement> getRetryHistogramMeasurements() {
11061106
}
11071107

11081108
private Map<String, Object> metricAttributes(String action) {
1109-
return Map.of("repo_type", "s3", "repo_name", "repository", "operation", "GetObject", "purpose", "Indices", "action", action);
1109+
return Map.of("repo_type", "s3", "operation", "GetObject", "purpose", "Indices", "action", action);
11101110
}
11111111

11121112
/**

‎server/src/main/java/org/elasticsearch/repositories/RepositoriesMetrics.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,7 @@ public static Map<String, Object> createAttributesMap(
127127
OperationPurpose purpose,
128128
String operation
129129
) {
130-
return Map.of(
131-
"repo_type",
132-
repositoryMetadata.type(),
133-
"repo_name",
134-
repositoryMetadata.name(),
135-
"operation",
136-
operation,
137-
"purpose",
138-
purpose.getKey()
139-
);
130+
return Map.of("repo_type", repositoryMetadata.type(), "operation", operation, "purpose", purpose.getKey());
140131
}
141132

142133
}

‎x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheMetrics.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,24 +115,16 @@ public LongHistogram getCacheMissLoadTimes() {
115115
*
116116
* @param bytesCopied The number of bytes copied
117117
* @param copyTimeNanos The time taken to copy the bytes in nanoseconds
118-
* @param index The index being loaded
119-
* @param shardId The ID of the shard being loaded
120118
* @param cachePopulationReason The reason for the cache being populated
121119
* @param cachePopulationSource The source from which the data is being loaded
122120
*/
123121
public void recordCachePopulationMetrics(
124122
int bytesCopied,
125123
long copyTimeNanos,
126-
String index,
127-
int shardId,
128124
CachePopulationReason cachePopulationReason,
129125
CachePopulationSource cachePopulationSource
130126
) {
131127
Map<String, Object> metricAttributes = Map.of(
132-
INDEX_ATTRIBUTE_KEY,
133-
index,
134-
SHARD_ID_ATTRIBUTE_KEY,
135-
shardId,
136128
CACHE_POPULATION_REASON_ATTRIBUTE_KEY,
137129
cachePopulationReason.name(),
138130
CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY,

‎x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/BlobCacheMetricsTests.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,11 @@ public void createMetrics() {
3030
public void testRecordCachePopulationMetricsRecordsThroughput() {
3131
int mebiBytesSent = randomIntBetween(1, 4);
3232
int secondsTaken = randomIntBetween(1, 5);
33-
String indexName = randomIdentifier();
34-
int shardId = randomIntBetween(0, 10);
3533
BlobCacheMetrics.CachePopulationReason cachePopulationReason = randomFrom(BlobCacheMetrics.CachePopulationReason.values());
3634
CachePopulationSource cachePopulationSource = randomFrom(CachePopulationSource.values());
3735
metrics.recordCachePopulationMetrics(
3836
Math.toIntExact(ByteSizeValue.ofMb(mebiBytesSent).getBytes()),
3937
TimeUnit.SECONDS.toNanos(secondsTaken),
40-
indexName,
41-
shardId,
4238
cachePopulationReason,
4339
cachePopulationSource
4440
);
@@ -48,32 +44,28 @@ public void testRecordCachePopulationMetricsRecordsThroughput() {
4844
.getMeasurements(InstrumentType.DOUBLE_HISTOGRAM, "es.blob_cache.population.throughput.histogram")
4945
.get(0);
5046
assertEquals(throughputMeasurement.getDouble(), (double) mebiBytesSent / secondsTaken, 0.0);
51-
assertExpectedAttributesPresent(throughputMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
47+
assertExpectedAttributesPresent(throughputMeasurement, cachePopulationReason, cachePopulationSource);
5248

5349
// bytes counter
5450
Measurement totalBytesMeasurement = recordingMeterRegistry.getRecorder()
5551
.getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.bytes.total")
5652
.get(0);
5753
assertEquals(totalBytesMeasurement.getLong(), ByteSizeValue.ofMb(mebiBytesSent).getBytes());
58-
assertExpectedAttributesPresent(totalBytesMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
54+
assertExpectedAttributesPresent(totalBytesMeasurement, cachePopulationReason, cachePopulationSource);
5955

6056
// time counter
6157
Measurement totalTimeMeasurement = recordingMeterRegistry.getRecorder()
6258
.getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.time.total")
6359
.get(0);
6460
assertEquals(totalTimeMeasurement.getLong(), TimeUnit.SECONDS.toMillis(secondsTaken));
65-
assertExpectedAttributesPresent(totalTimeMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
61+
assertExpectedAttributesPresent(totalTimeMeasurement, cachePopulationReason, cachePopulationSource);
6662
}
6763

6864
private static void assertExpectedAttributesPresent(
6965
Measurement measurement,
70-
int shardId,
71-
String indexName,
7266
BlobCacheMetrics.CachePopulationReason cachePopulationReason,
7367
CachePopulationSource cachePopulationSource
7468
) {
75-
assertEquals(measurement.attributes().get(BlobCacheMetrics.SHARD_ID_ATTRIBUTE_KEY), shardId);
76-
assertEquals(measurement.attributes().get(BlobCacheMetrics.INDEX_ATTRIBUTE_KEY), indexName);
7769
assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_REASON_ATTRIBUTE_KEY), cachePopulationReason.name());
7870
assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY), cachePopulationSource.name());
7971
}

0 commit comments

Comments
 (0)