Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing incorrect RU metric reporting in micrometer metrics #31307

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos-spark_3-1_2-12/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Breaking Changes

#### Bugs Fixed
* Fixed incorrect RU metric reporting in micrometer metrics. - See [PR 31307](https://github.com/Azure/azure-sdk-for-java/pull/31307)

#### Other Changes

Expand Down
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos-spark_3-2_2-12/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Breaking Changes

#### Bugs Fixed
* Fixed incorrect RU metric reporting in micrometer metrics. - See [PR 31307](https://github.com/Azure/azure-sdk-for-java/pull/31307)

#### Other Changes

Expand Down
1 change: 1 addition & 0 deletions sdk/cosmos/azure-cosmos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Breaking Changes

#### Bugs Fixed
* Fixed incorrect RU metric reporting in micrometer metrics. - See [PR 31307](https://github.com/Azure/azure-sdk-for-java/pull/31307)

#### Other Changes
* Added SslHandshakeTimeout minimum duration config - See [PR 31298](https://github.com/Azure/azure-sdk-for-java/pull/31298)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ public void recordOperation(
.publishPercentileHistogram(true)
.tags(operationTags)
.register(compositeRegistry);
requestChargeMeter.record(Math.max(requestCharge, 10_000_000d));
requestChargeMeter.record(Math.min(requestCharge, 10_000_000d));

DistributionSummary regionsContactedMeter = DistributionSummary
.builder(nameOf("op.regionsContacted"))
Expand All @@ -320,7 +320,7 @@ public void recordOperation(
.tags(operationTags)
.register(compositeRegistry);
if (contactedRegions != null && contactedRegions.size() > 0) {
regionsContactedMeter.record(Math.max(contactedRegions.size(), 100d));
regionsContactedMeter.record(Math.min(contactedRegions.size(), 100d));
}

Timer latencyMeter = Timer
Expand Down Expand Up @@ -657,7 +657,7 @@ private void recordStoreResponseStatistics(
.publishPercentileHistogram(true)
.tags(requestTags)
.register(compositeRegistry);
requestChargeMeter.record(Math.max(requestCharge, 1_000_000d));
requestChargeMeter.record(Math.min(requestCharge, 1_000_000d));

Duration latency = responseStatistics.getDuration();
if (latency != null) {
Expand Down Expand Up @@ -739,7 +739,7 @@ private void recordGatewayStatistics(
.publishPercentileHistogram(true)
.tags(requestTags)
.register(compositeRegistry);
requestChargeMeter.record(Math.max(requestCharge, 1_000_000d));
requestChargeMeter.record(Math.min(requestCharge, 1_000_000d));

if (latency != null) {
Timer requestLatencyMeter = Timer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,14 @@ public void maxValueExceedingDefinedLimitStillWorksWithoutException() throws Exc
List<Measurement> measurements = new ArrayList<>();
requestLatencyMeter.measure().forEach(measurements::add);

assertThat(measurements.size() == 1);
assertThat(measurements.get(0).getValue() == 600);


assertThat(measurements.size()).isEqualTo(3);

assertThat(measurements.get(0).getStatistic().getTagValueRepresentation()).isEqualTo("count");
assertThat(measurements.get(0).getValue()).isEqualTo(1);
assertThat(measurements.get(1).getStatistic().getTagValueRepresentation()).isEqualTo("total");
assertThat(measurements.get(1).getValue()).isEqualTo(600 * 1000); // transform into milliseconds
assertThat(measurements.get(2).getStatistic().getTagValueRepresentation()).isEqualTo("max");
assertThat(measurements.get(2).getValue()).isEqualTo(600 * 1000); // transform into milliseconds
} finally {
this.afterTest();
}
Expand All @@ -165,14 +169,19 @@ public void createItem() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "201"),
Tag.of(TagName.RequestStatusCode.toString(), "201/0")
Tag.of(TagName.RequestStatusCode.toString(), "201/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Create"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Create")
Tag.of(TagName.RequestOperationType.toString(), "Document/Create"),
1,
100
);

} finally {
this.afterTest();
}
Expand All @@ -193,13 +202,17 @@ public void readItem() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
50
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Read"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Read")
Tag.of(TagName.RequestOperationType.toString(), "Document/Read"),
1,
50
);

Tag queryPlanTag = Tag.of(TagName.RequestOperationType.toString(), "DocumentCollection_QueryPlan");
Expand Down Expand Up @@ -231,13 +244,17 @@ public void replaceItem() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Replace"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Replace")
Tag.of(TagName.RequestOperationType.toString(), "Document/Replace"),
1,
100
);
} finally {
this.afterTest();
Expand All @@ -259,13 +276,17 @@ public void deleteItem() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "204"),
Tag.of(TagName.RequestStatusCode.toString(), "204/0")
Tag.of(TagName.RequestStatusCode.toString(), "204/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Delete"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Delete")
Tag.of(TagName.RequestOperationType.toString(), "Document/Delete"),
1,
100
);
} finally {
this.afterTest();
Expand All @@ -287,13 +308,17 @@ public void readAllItems() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/ReadFeed/readAllItems." + container.getId()),
Tag.of(TagName.RequestOperationType.toString(), "Document/Query")
Tag.of(TagName.RequestOperationType.toString(), "Document/Query"),
1,
100
);

this.validateItemCountMetrics(
Expand Down Expand Up @@ -337,13 +362,17 @@ public void queryItems() throws Exception {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
10000
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Query/queryItems." + container.getId()),
Tag.of(TagName.RequestOperationType.toString(), "Document/Query")
Tag.of(TagName.RequestOperationType.toString(), "Document/Query"),
1,
10000
);

this.validateItemCountMetrics(
Expand Down Expand Up @@ -411,13 +440,17 @@ public void itemPatchSuccess() {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Patch"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Patch")
Tag.of(TagName.RequestOperationType.toString(), "Document/Patch"),
1,
100
);
} finally {
this.afterTest();
Expand Down Expand Up @@ -461,13 +494,17 @@ public void createItem_withBulk() {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Batch"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Batch")
Tag.of(TagName.RequestOperationType.toString(), "Document/Batch"),
1,
100
);
} finally {
this.afterTest();
Expand Down Expand Up @@ -519,13 +556,17 @@ public void batchMultipleItemExecution() {

this.validateMetrics(
Tag.of(TagName.OperationStatusCode.toString(), "200"),
Tag.of(TagName.RequestStatusCode.toString(), "200/0")
Tag.of(TagName.RequestStatusCode.toString(), "200/0"),
1,
100
);

this.validateMetrics(
Tag.of(
TagName.Operation.toString(), "Document/Batch"),
Tag.of(TagName.RequestOperationType.toString(), "Document/Batch")
Tag.of(TagName.RequestOperationType.toString(), "Document/Batch"),
1,
100
);
} finally {
this.afterTest();
Expand All @@ -552,10 +593,11 @@ private void validateItemResponse(InternalObjectNode containerProperties,
.isEqualTo(containerProperties.getId());
}

private void validateMetrics() {
private void validateMetrics(int minRu, int maxRu) {
this.assertMetrics("cosmos.client.op.latency", true);
this.assertMetrics("cosmos.client.op.calls", true);
this.assertMetrics("cosmos.client.op.RUs", true);
Meter reportedOpRequestCharge = this.assertMetrics("cosmos.client.op.RUs", true);
validateReasonableRUs(reportedOpRequestCharge, minRu, maxRu);
this.assertMetrics("cosmos.client.op.regionsContacted", true);
this.assertMetrics(
"cosmos.client.op.regionsContacted",
Expand All @@ -570,7 +612,9 @@ private void validateMetrics() {
Tag.of(TagName.RegionName.toString(), this.preferredRegion));
this.assertMetrics("cosmos.client.req.rntbd.backendLatency", true);
this.assertMetrics("cosmos.client.req.rntbd.requests", true);
this.assertMetrics("cosmos.client.req.rntbd.RUs", true);
Meter reportedRntbdRequestCharge =
this.assertMetrics("cosmos.client.req.rntbd.RUs", true);
validateReasonableRUs(reportedRntbdRequestCharge, minRu, maxRu);
this.assertMetrics("cosmos.client.req.rntbd.timeline", true);
} else {
this.assertMetrics("cosmos.client.req.gw.latency", true);
Expand All @@ -580,7 +624,9 @@ private void validateMetrics() {
Tag.of(TagName.RegionName.toString(), this.preferredRegion));
this.assertMetrics("cosmos.client.req.gw.backendLatency", false);
this.assertMetrics("cosmos.client.req.gw.requests", true);
this.assertMetrics("cosmos.client.req.gw.RUs", true);
Meter reportedGatewayRequestCharge =
this.assertMetrics("cosmos.client.req.gw.RUs", true);
validateReasonableRUs(reportedGatewayRequestCharge, minRu, maxRu);
this.assertMetrics("cosmos.client.req.gw.timeline", true);
this.assertMetrics("cosmos.client.req.rntbd", false);
}
Expand All @@ -591,10 +637,23 @@ private void validateItemCountMetrics(Tag expectedOperationTag) {
this.assertMetrics("cosmos.client.op.actualItemCount", true, expectedOperationTag);
}

private void validateMetrics(Tag expectedOperationTag, Tag expectedRequestTag) {
private void validateReasonableRUs(Meter reportedRequestChargeMeter, int expectedMinRu, int expectedMaxRu) {
List<Measurement> measurements = new ArrayList<>();
reportedRequestChargeMeter.measure().forEach(measurements::add);

assertThat(measurements.size()).isGreaterThan(0);
for (int i = 0; i < measurements.size(); i++) {
assertThat(measurements.get(i).getValue()).isGreaterThanOrEqualTo(expectedMinRu);
assertThat(measurements.get(i).getValue()).isLessThanOrEqualTo(expectedMaxRu);
}
}
private void validateMetrics(Tag expectedOperationTag, Tag expectedRequestTag, int minRu, int maxRu) {
this.assertMetrics("cosmos.client.op.latency", true, expectedOperationTag);
this.assertMetrics("cosmos.client.op.calls", true, expectedOperationTag);
this.assertMetrics("cosmos.client.op.RUs", true, expectedOperationTag);
Meter reportedOpRequestCharge = this.assertMetrics(
"cosmos.client.op.RUs", true, expectedOperationTag);
validateReasonableRUs(reportedOpRequestCharge, minRu, maxRu);

this.assertMetrics("cosmos.client.op.regionsContacted", true, expectedOperationTag);

this.assertMetrics(
Expand All @@ -610,7 +669,9 @@ private void validateMetrics(Tag expectedOperationTag, Tag expectedRequestTag) {
Tag.of(TagName.RegionName.toString(), this.preferredRegion));
this.assertMetrics("cosmos.client.req.rntbd.backendLatency", true, expectedRequestTag);
this.assertMetrics("cosmos.client.req.rntbd.requests", true, expectedRequestTag);
this.assertMetrics("cosmos.client.req.rntbd.RUs", true, expectedRequestTag);
Meter reportedRntbdRequestCharge =
this.assertMetrics("cosmos.client.req.rntbd.RUs", true, expectedRequestTag);
validateReasonableRUs(reportedRntbdRequestCharge, minRu, maxRu);
this.assertMetrics("cosmos.client.req.rntbd.timeline", true, expectedRequestTag);
} else {
this.assertMetrics("cosmos.client.req.gw.latency", true, expectedRequestTag);
Expand All @@ -620,7 +681,9 @@ private void validateMetrics(Tag expectedOperationTag, Tag expectedRequestTag) {
Tag.of(TagName.RegionName.toString(), this.preferredRegion));
this.assertMetrics("cosmos.client.req.gw.backendLatency", false, expectedRequestTag);
this.assertMetrics("cosmos.client.req.gw.requests", true, expectedRequestTag);
this.assertMetrics("cosmos.client.req.gw.RUs", true, expectedRequestTag);
Meter reportedGatewayRequestCharge =
this.assertMetrics("cosmos.client.req.gw.RUs", true, expectedRequestTag);
validateReasonableRUs(reportedGatewayRequestCharge, minRu, maxRu);
this.assertMetrics("cosmos.client.req.gw.timeline", true, expectedRequestTag);
this.assertMetrics("cosmos.client.req.rntbd", false);
}
Expand Down