Skip to content

Commit c53e0d2

Browse files
programmatixprogrammatix
committed
JVMCBC-1671: Transactions metrics not reporting all required attributes
FIT has caught the transactions metrics exported by the SDK do not include various attributes that are expected by the RFC, such as service name. DRYing attribute tags across core, transactions and Protostellar to both fix this, and move a significant chunk of logic out of Core. Change-Id: I3b909a4f773a7dcf52b398beee38454dea807a8f Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/230733 Tested-by: Graham Pople <[email protected]> Reviewed-by: Michael Reiche <[email protected]>
1 parent 7edbdc8 commit c53e0d2

File tree

6 files changed

+205
-182
lines changed

6 files changed

+205
-182
lines changed

‎core-io/src/main/java/com/couchbase/client/core/Core.java‎

Lines changed: 3 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.couchbase.client.core.cnc.events.core.WatchdogRunFailedEvent;
5757
import com.couchbase.client.core.cnc.events.transaction.TransactionsStartedEvent;
5858
import com.couchbase.client.core.cnc.metrics.LoggingMeter;
59+
import com.couchbase.client.core.cnc.metrics.ResponseMetricIdentifier;
5960
import com.couchbase.client.core.config.ClusterConfig;
6061
import com.couchbase.client.core.config.ConfigurationProvider;
6162
import com.couchbase.client.core.config.DefaultConfigurationProvider;
@@ -75,17 +76,13 @@
7576
import com.couchbase.client.core.error.RequestCanceledException;
7677
import com.couchbase.client.core.error.UnambiguousTimeoutException;
7778
import com.couchbase.client.core.error.UnsupportedConfigMechanismException;
78-
import com.couchbase.client.core.io.CollectionIdentifier;
7979
import com.couchbase.client.core.manager.CoreBucketManagerOps;
8080
import com.couchbase.client.core.manager.CoreCollectionManager;
8181
import com.couchbase.client.core.msg.CancellationReason;
8282
import com.couchbase.client.core.msg.Request;
8383
import com.couchbase.client.core.msg.RequestContext;
8484
import com.couchbase.client.core.msg.RequestTarget;
8585
import com.couchbase.client.core.msg.Response;
86-
import com.couchbase.client.core.msg.kv.KeyValueRequest;
87-
import com.couchbase.client.core.msg.query.QueryRequest;
88-
import com.couchbase.client.core.msg.search.ServerSearchRequest;
8986
import com.couchbase.client.core.node.AnalyticsLocator;
9087
import com.couchbase.client.core.node.KeyValueLocator;
9188
import com.couchbase.client.core.node.Locator;
@@ -118,11 +115,9 @@
118115
import java.time.Duration;
119116
import java.util.ArrayList;
120117
import java.util.Collection;
121-
import java.util.HashMap;
122118
import java.util.List;
123119
import java.util.Map;
124120
import java.util.NoSuchElementException;
125-
import java.util.Objects;
126121
import java.util.Optional;
127122
import java.util.Set;
128123
import java.util.concurrent.CompletableFuture;
@@ -642,42 +637,10 @@ public ValueRecorder responseMetric(final Request<?> request, @Nullable Throwabl
642637
exceptionSimpleName = err.getClass().getSimpleName().replace("Exception", "");
643638
}
644639
}
645-
final String finalExceptionSimpleName = exceptionSimpleName;
646640
final ClusterIdentifier clusterIdent = ClusterIdentifierUtil.fromConfig(currentConfig);
647641

648-
return responseMetrics.computeIfAbsent(new ResponseMetricIdentifier(request, exceptionSimpleName, clusterIdent), key -> {
649-
Map<String, String> tags = new HashMap<>(9);
650-
tags.put(TracingIdentifiers.ATTR_SERVICE, key.serviceTracingId);
651-
tags.put(TracingIdentifiers.ATTR_OPERATION, key.requestName);
652-
653-
// The LoggingMeter only uses the service and operation labels, so optimise this hot-path by skipping
654-
// assigning other labels.
655-
if (!isDefaultLoggingMeter) {
656-
// Crucial note for Micrometer:
657-
// If we are ever going to output an attribute from a given JVM run then we must always
658-
// output that attribute in this run. Specifying null as an attribute value allows the OTel backend to strip it, and
659-
// the Micrometer backend to provide a default value.
660-
// See (internal to Couchbase) discussion here for full details:
661-
// https://issues.couchbase.com/browse/CBSE-17070?focusedId=779820&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-779820
662-
// If this rule is not followed, then Micrometer will silently discard some metrics. Micrometer requires that
663-
// every value output under a given metric has the same set of attributes.
664-
665-
tags.put(TracingIdentifiers.ATTR_NAME, key.bucketName);
666-
tags.put(TracingIdentifiers.ATTR_SCOPE, key.scopeName);
667-
tags.put(TracingIdentifiers.ATTR_COLLECTION, key.collectionName);
668-
669-
tags.put(TracingIdentifiers.ATTR_CLUSTER_UUID, key.clusterUuid);
670-
tags.put(TracingIdentifiers.ATTR_CLUSTER_NAME, key.clusterName);
671-
672-
if (finalExceptionSimpleName != null) {
673-
tags.put(TracingIdentifiers.ATTR_OUTCOME, finalExceptionSimpleName);
674-
} else {
675-
tags.put(TracingIdentifiers.ATTR_OUTCOME, "Success");
676-
}
677-
}
678-
679-
return coreContext.environment().meter().valueRecorder(TracingIdentifiers.METER_OPERATIONS, tags);
680-
});
642+
return responseMetrics.computeIfAbsent(ResponseMetricIdentifier.fromRequest(request, exceptionSimpleName, clusterIdent, isDefaultLoggingMeter),key ->
643+
coreContext.environment().meter().valueRecorder(TracingIdentifiers.METER_OPERATIONS, key.tags()));
681644
}
682645

683646
/**
@@ -1033,91 +996,6 @@ public CompletableFuture<Void> waitUntilReady(
1033996
return WaitUntilReadyHelper.waitUntilReady(this, serviceTypes, timeout, desiredState, Optional.ofNullable(bucketName));
1034997
}
1035998

1036-
@Stability.Internal
1037-
public static class ResponseMetricIdentifier {
1038-
1039-
private final String serviceTracingId;
1040-
private final String requestName;
1041-
private final @Nullable String bucketName;
1042-
private final @Nullable String scopeName;
1043-
private final @Nullable String collectionName;
1044-
private final @Nullable String exceptionSimpleName;
1045-
private final @Nullable String clusterName;
1046-
private final @Nullable String clusterUuid;
1047-
1048-
ResponseMetricIdentifier(final Request<?> request, @Nullable String exceptionSimpleName, @Nullable ClusterIdentifier clusterIdent) {
1049-
this.exceptionSimpleName = exceptionSimpleName;
1050-
this.serviceTracingId = request.serviceTracingId();
1051-
this.requestName = request.name();
1052-
this.clusterName = clusterIdent == null ? null : clusterIdent.clusterName();
1053-
this.clusterUuid = clusterIdent == null ? null : clusterIdent.clusterUuid();
1054-
if (request instanceof KeyValueRequest) {
1055-
KeyValueRequest<?> kv = (KeyValueRequest<?>) request;
1056-
bucketName = request.bucket();
1057-
scopeName = kv.collectionIdentifier().scope().orElse(CollectionIdentifier.DEFAULT_SCOPE);
1058-
collectionName = kv.collectionIdentifier().collection().orElse(CollectionIdentifier.DEFAULT_SCOPE);
1059-
} else if (request instanceof QueryRequest) {
1060-
QueryRequest query = (QueryRequest) request;
1061-
bucketName = request.bucket();
1062-
scopeName = query.scope();
1063-
collectionName = null;
1064-
} else if (request instanceof ServerSearchRequest) {
1065-
ServerSearchRequest search = (ServerSearchRequest) request;
1066-
if (search.scope() != null) {
1067-
bucketName = search.scope().bucketName();
1068-
scopeName = search.scope().scopeName();
1069-
} else {
1070-
bucketName = null;
1071-
scopeName = null;
1072-
}
1073-
collectionName = null;
1074-
} else {
1075-
bucketName = null;
1076-
scopeName = null;
1077-
collectionName = null;
1078-
}
1079-
}
1080-
1081-
public ResponseMetricIdentifier(final String serviceTracingId, final String requestName) {
1082-
this.serviceTracingId = serviceTracingId;
1083-
this.requestName = requestName;
1084-
this.bucketName = null;
1085-
this.scopeName = null;
1086-
this.collectionName = null;
1087-
this.exceptionSimpleName = null;
1088-
this.clusterName = null;
1089-
this.clusterUuid = null;
1090-
}
1091-
1092-
public String serviceTracingId() {
1093-
return serviceTracingId;
1094-
}
1095-
1096-
public String requestName() {
1097-
return requestName;
1098-
}
1099-
1100-
@Override
1101-
public boolean equals(Object o) {
1102-
if (this == o) return true;
1103-
if (o == null || getClass() != o.getClass()) return false;
1104-
ResponseMetricIdentifier that = (ResponseMetricIdentifier) o;
1105-
return serviceTracingId.equals(that.serviceTracingId)
1106-
&& Objects.equals(requestName, that.requestName)
1107-
&& Objects.equals(bucketName, that.bucketName)
1108-
&& Objects.equals(scopeName, that.scopeName)
1109-
&& Objects.equals(collectionName, that.collectionName)
1110-
&& Objects.equals(exceptionSimpleName, that.exceptionSimpleName)
1111-
&& Objects.equals(clusterName, that.clusterName)
1112-
&& Objects.equals(clusterUuid, that.clusterUuid);
1113-
}
1114-
1115-
@Override
1116-
public int hashCode() {
1117-
return Objects.hash(serviceTracingId, requestName, bucketName, scopeName, collectionName, exceptionSimpleName, clusterName, clusterUuid);
1118-
}
1119-
}
1120-
1121999
/**
11221000
* Watchdog responsible for checking for potentially invalid states and initiating corrective action.
11231001
* <p>

‎core-io/src/main/java/com/couchbase/client/core/CoreProtostellar.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.couchbase.client.core.cnc.ValueRecorder;
3131
import com.couchbase.client.core.cnc.events.core.ShutdownCompletedEvent;
3232
import com.couchbase.client.core.cnc.events.core.ShutdownInitiatedEvent;
33+
import com.couchbase.client.core.cnc.metrics.ResponseMetricIdentifier;
3334
import com.couchbase.client.core.diagnostics.ClusterState;
3435
import com.couchbase.client.core.endpoint.ProtostellarEndpoint;
3536
import com.couchbase.client.core.endpoint.ProtostellarPool;
@@ -152,16 +153,12 @@ public ProtostellarPool pool() {
152153
return pool;
153154
}
154155

155-
private final Map<Core.ResponseMetricIdentifier, ValueRecorder> responseMetrics = new ConcurrentHashMap<>();
156+
private final Map<ResponseMetricIdentifier, ValueRecorder> responseMetrics = new ConcurrentHashMap<>();
156157

157158
@Stability.Internal
158-
public ValueRecorder responseMetric(final Core.ResponseMetricIdentifier rmi) {
159-
return responseMetrics.computeIfAbsent(rmi, key -> {
160-
Map<String, String> tags = new HashMap<>(4);
161-
tags.put(TracingIdentifiers.ATTR_SERVICE, key.serviceTracingId());
162-
tags.put(TracingIdentifiers.ATTR_OPERATION, key.requestName());
163-
return ctx.environment().meter().valueRecorder(TracingIdentifiers.METER_OPERATIONS, tags);
164-
});
159+
public ValueRecorder responseMetric(final ResponseMetricIdentifier rmi) {
160+
return responseMetrics.computeIfAbsent(rmi, key ->
161+
ctx.environment().meter().valueRecorder(TracingIdentifiers.METER_OPERATIONS, key.tags()));
165162
}
166163

167164
@Override

‎core-io/src/main/java/com/couchbase/client/core/cnc/metrics/ResponseMetricIdentifier.java‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/*
2+
* Copyright (c) 2025 Couchbase, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.couchbase.client.core.cnc.metrics;
18+
19+
import com.couchbase.client.core.annotation.Stability;
20+
import com.couchbase.client.core.cnc.TracingIdentifiers;
21+
import com.couchbase.client.core.io.CollectionIdentifier;
22+
import com.couchbase.client.core.msg.Request;
23+
import com.couchbase.client.core.msg.kv.KeyValueRequest;
24+
import com.couchbase.client.core.msg.query.QueryRequest;
25+
import com.couchbase.client.core.msg.search.ServerSearchRequest;
26+
import com.couchbase.client.core.topology.ClusterIdentifier;
27+
import org.jspecify.annotations.Nullable;
28+
29+
import java.util.HashMap;
30+
import java.util.Map;
31+
import java.util.Objects;
32+
33+
@Stability.Internal
34+
public class ResponseMetricIdentifier {
35+
36+
private final String serviceTracingId;
37+
private final String requestName;
38+
private final @Nullable String bucketName;
39+
private final @Nullable String scopeName;
40+
private final @Nullable String collectionName;
41+
private final @Nullable String exceptionSimpleName;
42+
private final @Nullable ClusterIdentifier clusterIdent;
43+
private final boolean isDefaultLoggingMeter;
44+
45+
public static ResponseMetricIdentifier fromRequest(final Request<?> request,
46+
@Nullable String exceptionSimpleName,
47+
@Nullable ClusterIdentifier clusterIdent,
48+
boolean isDefaultLoggingMeter) {
49+
50+
String bucketName = null;
51+
String scopeName = null;
52+
String collectionName = null;
53+
54+
if (request instanceof KeyValueRequest) {
55+
KeyValueRequest<?> kv = (KeyValueRequest<?>) request;
56+
bucketName = request.bucket();
57+
scopeName = kv.collectionIdentifier().scope().orElse(CollectionIdentifier.DEFAULT_SCOPE);
58+
collectionName = kv.collectionIdentifier().collection().orElse(CollectionIdentifier.DEFAULT_SCOPE);
59+
} else if (request instanceof QueryRequest) {
60+
QueryRequest query = (QueryRequest) request;
61+
bucketName = request.bucket();
62+
scopeName = query.scope();
63+
} else if (request instanceof ServerSearchRequest) {
64+
ServerSearchRequest search = (ServerSearchRequest) request;
65+
if (search.scope() != null) {
66+
bucketName = search.scope().bucketName();
67+
scopeName = search.scope().scopeName();
68+
}
69+
}
70+
71+
return new ResponseMetricIdentifier(request.serviceTracingId(),
72+
request.name(),
73+
bucketName,
74+
scopeName,
75+
collectionName,
76+
exceptionSimpleName,
77+
clusterIdent,
78+
isDefaultLoggingMeter);
79+
}
80+
81+
public ResponseMetricIdentifier(String serviceTracingId,
82+
String requestName,
83+
@Nullable String bucketName,
84+
@Nullable String scopeName,
85+
@Nullable String collectionName,
86+
@Nullable String exceptionSimpleName,
87+
@Nullable ClusterIdentifier clusterIdent,
88+
boolean isDefaultLoggingMeter) {
89+
this.serviceTracingId = serviceTracingId;
90+
this.requestName = requestName;
91+
this.bucketName = bucketName;
92+
this.scopeName = scopeName;
93+
this.collectionName = collectionName;
94+
this.exceptionSimpleName = exceptionSimpleName;
95+
this.clusterIdent = clusterIdent;
96+
this.isDefaultLoggingMeter = isDefaultLoggingMeter;
97+
}
98+
99+
@Override
100+
public boolean equals(Object o) {
101+
if (this == o) return true;
102+
if (o == null || getClass() != o.getClass()) return false;
103+
ResponseMetricIdentifier that = (ResponseMetricIdentifier) o;
104+
return serviceTracingId.equals(that.serviceTracingId)
105+
&& Objects.equals(requestName, that.requestName)
106+
&& Objects.equals(bucketName, that.bucketName)
107+
&& Objects.equals(scopeName, that.scopeName)
108+
&& Objects.equals(collectionName, that.collectionName)
109+
&& Objects.equals(exceptionSimpleName, that.exceptionSimpleName)
110+
&& Objects.equals(clusterIdent, that.clusterIdent);
111+
}
112+
113+
@Override
114+
public int hashCode() {
115+
return Objects.hash(serviceTracingId, requestName, bucketName, scopeName, collectionName, exceptionSimpleName, clusterIdent);
116+
}
117+
118+
public Map<String, String> tags() {
119+
// N.b. remember the pattern is that ResponseMetricIdentifier is created many times (on every request),
120+
// while this method will only be called if we don't already have a corresponding ResponseMetricIdentifier
121+
// entry in the map. E.g. it would be a false optimisation to move this tags() logic into the ctor.
122+
Map<String, String> tags = new HashMap<>(9);
123+
tags.put(TracingIdentifiers.ATTR_SERVICE, serviceTracingId);
124+
tags.put(TracingIdentifiers.ATTR_OPERATION, requestName);
125+
126+
// The LoggingMeter only uses the service and operation labels, so optimise this hot-path by skipping
127+
// assigning other labels.
128+
if (!isDefaultLoggingMeter) {
129+
// Crucial note for Micrometer:
130+
// If we are ever going to output an attribute from a given JVM run then we must always
131+
// output that attribute in this run. Specifying null as an attribute value allows the OTel backend to strip it, and
132+
// the Micrometer backend to provide a default value.
133+
// See (internal to Couchbase) discussion here for full details:
134+
// https://issues.couchbase.com/browse/CBSE-17070?focusedId=779820&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-779820
135+
// If this rule is not followed, then Micrometer will silently discard some metrics. Micrometer requires that
136+
// every value output under a given metric has the same set of attributes.
137+
138+
tags.put(TracingIdentifiers.ATTR_NAME, bucketName);
139+
tags.put(TracingIdentifiers.ATTR_SCOPE, scopeName);
140+
tags.put(TracingIdentifiers.ATTR_COLLECTION, collectionName);
141+
142+
tags.put(TracingIdentifiers.ATTR_CLUSTER_UUID, clusterIdent == null ? null : clusterIdent.clusterUuid());
143+
tags.put(TracingIdentifiers.ATTR_CLUSTER_NAME, clusterIdent == null ? null : clusterIdent.clusterName());
144+
145+
if (exceptionSimpleName != null) {
146+
tags.put(TracingIdentifiers.ATTR_OUTCOME, exceptionSimpleName);
147+
} else {
148+
tags.put(TracingIdentifiers.ATTR_OUTCOME, "Success");
149+
}
150+
}
151+
152+
return tags;
153+
}
154+
}

‎core-io/src/main/java/com/couchbase/client/core/protostellar/ProtostellarRequest.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import com.couchbase.client.core.cnc.CbTracing;
2222
import com.couchbase.client.core.cnc.RequestSpan;
2323
import com.couchbase.client.core.cnc.TracingIdentifiers;
24+
import com.couchbase.client.core.cnc.metrics.LoggingMeter;
2425
import com.couchbase.client.core.cnc.metrics.NoopMeter;
26+
import com.couchbase.client.core.cnc.metrics.ResponseMetricIdentifier;
2527
import com.couchbase.client.core.deps.io.grpc.Deadline;
2628
import com.couchbase.client.core.error.RequestCanceledException;
2729
import com.couchbase.client.core.error.context.CancellationErrorContext;
@@ -152,8 +154,16 @@ public void raisedResponseToUser(@Nullable Throwable err) {
152154

153155
if (!(core.context().environment().meter() instanceof NoopMeter)) {
154156
long latency = logicalRequestLatency();
157+
boolean isDefaultLoggingMeter = core.context().environment().meter() instanceof LoggingMeter;
155158
if (latency > 0) {
156-
Core.ResponseMetricIdentifier rmi = new Core.ResponseMetricIdentifier(serviceType.id(), requestName);
159+
ResponseMetricIdentifier rmi = new ResponseMetricIdentifier(serviceType.id(),
160+
requestName,
161+
// These are not currently supported for Protostellar
162+
null, null, null,
163+
null,
164+
// This is not available on Protostellar
165+
null,
166+
isDefaultLoggingMeter);
157167
try {
158168
core.responseMetric(rmi).recordValue(latency);
159169
} catch (Exception e) {

0 commit comments

Comments
 (0)