Skip to content

Commit

Permalink
Restore repo_name label to repository metrics (#117114)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicktindall authored Nov 22, 2024
1 parent eff0c42 commit d87a1a2
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 11 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).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).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).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB, repository).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).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.GET_BLOB_PROPERTIES, repository).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).expectMetrics()
metricsAsserter(dataNodeName, purpose, AzureBlobStore.Operation.LIST_BLOBS, repository).expectMetrics()
.withRequests(4)
.withThrottles(0)
.withExceptions(4)
Expand Down Expand Up @@ -322,14 +322,20 @@ private void clearMetrics(String discoveryNode) {
.forEach(TestTelemetryPlugin::resetMeter);
}

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

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 @@ -355,10 +361,11 @@ List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, Strin
abstract List<Measurement> getMeasurements(TestTelemetryPlugin testTelemetryPlugin, String name);
}

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

private class Expectations {
Expand Down Expand Up @@ -451,6 +458,7 @@ 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 @@ -462,6 +470,8 @@ 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,7 +402,10 @@ public void testMetrics() throws Exception {
)
);
metrics.forEach(metric -> {
assertThat(metric.attributes(), allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("operation"), hasKey("purpose")));
assertThat(
metric.attributes(),
allOf(hasEntry("repo_type", AzureRepository.TYPE), hasKey("repo_name"), 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,7 +300,10 @@ public void testMetrics() throws Exception {
)
);
metrics.forEach(metric -> {
assertThat(metric.attributes(), allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("operation"), hasKey("purpose")));
assertThat(
metric.attributes(),
allOf(hasEntry("repo_type", S3Repository.TYPE), hasKey("repo_name"), 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,6 +327,8 @@ 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", "operation", "GetObject", "purpose", "Indices", "action", action);
return Map.of("repo_type", "s3", "repo_name", "repository", "operation", "GetObject", "purpose", "Indices", "action", action);
}

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

}

0 comments on commit d87a1a2

Please sign in to comment.