Skip to content

Commit

Permalink
Remove high-cardinality metric attributes (elastic#116700)
Browse files Browse the repository at this point in the history
Relates: ES-10027
  • Loading branch information
nicktindall authored Nov 14, 2024
1 parent 270d9d2 commit 43d36ed
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void testThrottleResponsesAreCountedInMetrics() throws IOException {
blobContainer.blobExists(purpose, blobName);

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

// Correct metrics are recorded
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB, repository).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB).expectMetrics()
.withRequests(1)
.withThrottles(0)
.withExceptions(1)
Expand Down Expand Up @@ -170,7 +170,7 @@ public void testErrorResponsesAreCountedInMetrics() throws IOException {
blobContainer.blobExists(purpose, blobName);

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

// Correct metrics are recorded
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS, repository).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS).expectMetrics()
.withRequests(4)
.withThrottles(0)
.withExceptions(4)
Expand Down Expand Up @@ -322,20 +322,14 @@ private void clearMetrics(String discoveryNode) {
.forEach(TestTelemetryPlugin::resetMeter);
}

private MetricsAsserter metricsAsserter(
String dataNodeName,
OperationPurpose operationPurpose,
AzureBlobStore.Operation operation,
String repository
) {
return new MetricsAsserter(dataNodeName, operationPurpose, operation, repository);
private MetricsAsserter metricsAsserter(String dataNodeName, OperationPurpose operationPurpose, AzureBlobStore.Operation operation) {
return new MetricsAsserter(dataNodeName, operationPurpose, operation);
}

private class MetricsAsserter {
private final String dataNodeName;
private final OperationPurpose purpose;
private final AzureBlobStore.Operation operation;
private final String repository;

enum Result {
Success,
Expand All @@ -361,11 +355,10 @@ List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, Strin
abstract List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, String name);
}

private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation, String repository) {
private MetricsAsserter(String dataNodeName, OperationPurpose purpose, AzureBlobStore.Operation operation) {
this.dataNodeName = dataNodeName;
this.purpose = purpose;
this.operation = operation;
this.repository = repository;
}

private class Expectations {
Expand Down Expand Up @@ -458,7 +451,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa
.filter(
m -> m.attributes().get("operation").equals(operation.getKey())
&& m.attributes().get("purpose").equals(purpose.getKey())
&& m.attributes().get("repo_name").equals(repository)
&& m.attributes().get("repo_type").equals("azure")
)
.findFirst()
Expand All @@ -470,8 +462,6 @@ private void assertMatchingMetricRecorded(MetricType metricType, String metricNa
+ operation.getKey()
+ " and purpose="
+ purpose.getKey()
+ " and repo_name="
+ repository
+ " in "
+ measurements
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,10 +402,7 @@ public void testMetrics() throws Exception {
)
);
metrics.forEach(metric -> {
assertThat(
metric.attributes(),
allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose"))
);
assertThat(metric.attributes(), allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("operation"), hasKey("purpose")));
final AzureBlobStore.Operation operation = AzureBlobStore.Operation.fromKey((String) metric.attributes().get("operation"));
final AzureBlobStore.StatsKey statsKey = new AzureBlobStore.StatsKey(
operation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,7 @@ public void testMetrics() throws Exception {
)
);
metrics.forEach(metric -> {
assertThat(
metric.attributes(),
allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("repo_name"), hasKey("operation"), hasKey("purpose"))
);
assertThat(metric.attributes(), allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("operation"), hasKey("purpose")));
final S3BlobStore.Operation operation = S3BlobStore.Operation.parse((String) metric.attributes().get("operation"));
final S3BlobStore.StatsKey statsKey = new S3BlobStore.StatsKey(
operation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,6 @@ private Map<String, Object> metricAttributes(String action) {
return Map.of(
"repo_type",
S3Repository.TYPE,
"repo_name",
blobStore.getRepositoryMetadata().name(),
"operation",
Operation.GET_OBJECT.getKey(),
"purpose",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ private List<Measurement> getRetryHistogramMeasurements() {
}

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

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,7 @@ public static Map<String, Object> createAttributesMap(
OperationPurpose purpose,
String operation
) {
return Map.of(
"repo_type",
repositoryMetadata.type(),
"repo_name",
repositoryMetadata.name(),
"operation",
operation,
"purpose",
purpose.getKey()
);
return Map.of("repo_type", repositoryMetadata.type(), "operation", operation, "purpose", purpose.getKey());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -115,24 +115,16 @@ public LongHistogram getCacheMissLoadTimes() {
*
* @param bytesCopied The number of bytes copied
* @param copyTimeNanos The time taken to copy the bytes in nanoseconds
* @param index The index being loaded
* @param shardId The ID of the shard being loaded
* @param cachePopulationReason The reason for the cache being populated
* @param cachePopulationSource The source from which the data is being loaded
*/
public void recordCachePopulationMetrics(
int bytesCopied,
long copyTimeNanos,
String index,
int shardId,
CachePopulationReason cachePopulationReason,
CachePopulationSource cachePopulationSource
) {
Map<String, Object> metricAttributes = Map.of(
INDEX_ATTRIBUTE_KEY,
index,
SHARD_ID_ATTRIBUTE_KEY,
shardId,
CACHE_POPULATION_REASON_ATTRIBUTE_KEY,
cachePopulationReason.name(),
CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,11 @@ public void createMetrics() {
public void testRecordCachePopulationMetricsRecordsThroughput() {
int mebiBytesSent = randomIntBetween(1, 4);
int secondsTaken = randomIntBetween(1, 5);
String indexName = randomIdentifier();
int shardId = randomIntBetween(0, 10);
BlobCacheMetrics.CachePopulationReason cachePopulationReason = randomFrom(BlobCacheMetrics.CachePopulationReason.values());
CachePopulationSource cachePopulationSource = randomFrom(CachePopulationSource.values());
metrics.recordCachePopulationMetrics(
Math.toIntExact(ByteSizeValue.ofMb(mebiBytesSent).getBytes()),
TimeUnit.SECONDS.toNanos(secondsTaken),
indexName,
shardId,
cachePopulationReason,
cachePopulationSource
);
Expand All @@ -48,32 +44,28 @@ public void testRecordCachePopulationMetricsRecordsThroughput() {
.getMeasurements(InstrumentType.DOUBLE_HISTOGRAM, "es.blob_cache.population.throughput.histogram")
.get(0);
assertEquals(throughputMeasurement.getDouble(), (double) mebiBytesSent / secondsTaken, 0.0);
assertExpectedAttributesPresent(throughputMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
assertExpectedAttributesPresent(throughputMeasurement, cachePopulationReason, cachePopulationSource);

// bytes counter
Measurement totalBytesMeasurement = recordingMeterRegistry.getRecorder()
.getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.bytes.total")
.get(0);
assertEquals(totalBytesMeasurement.getLong(), ByteSizeValue.ofMb(mebiBytesSent).getBytes());
assertExpectedAttributesPresent(totalBytesMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
assertExpectedAttributesPresent(totalBytesMeasurement, cachePopulationReason, cachePopulationSource);

// time counter
Measurement totalTimeMeasurement = recordingMeterRegistry.getRecorder()
.getMeasurements(InstrumentType.LONG_COUNTER, "es.blob_cache.population.time.total")
.get(0);
assertEquals(totalTimeMeasurement.getLong(), TimeUnit.SECONDS.toMillis(secondsTaken));
assertExpectedAttributesPresent(totalTimeMeasurement, shardId, indexName, cachePopulationReason, cachePopulationSource);
assertExpectedAttributesPresent(totalTimeMeasurement, cachePopulationReason, cachePopulationSource);
}

private static void assertExpectedAttributesPresent(
Measurement measurement,
int shardId,
String indexName,
BlobCacheMetrics.CachePopulationReason cachePopulationReason,
CachePopulationSource cachePopulationSource
) {
assertEquals(measurement.attributes().get(BlobCacheMetrics.SHARD_ID_ATTRIBUTE_KEY), shardId);
assertEquals(measurement.attributes().get(BlobCacheMetrics.INDEX_ATTRIBUTE_KEY), indexName);
assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_REASON_ATTRIBUTE_KEY), cachePopulationReason.name());
assertEquals(measurement.attributes().get(BlobCacheMetrics.CACHE_POPULATION_SOURCE_ATTRIBUTE_KEY), cachePopulationSource.name());
}
Expand Down

0 comments on commit 43d36ed

Please sign in to comment.