Skip to content

Commit 3738c85

Browse files
dougqhclaude
andcommitted
Eliminate MetricKey: inline its fields onto AggregateEntry
MetricKey existed for two reasons -- the prior LRUCache key role (now handled by AggregateTable's Hashtable.Entry mechanics) and as the labels argument to MetricWriter.add. The first is gone; the second is the only thing keeping MetricKey alive. Fold its UTF8-encoded label fields onto AggregateEntry, change MetricWriter.add to take AggregateEntry directly, and delete MetricKey + MetricKeys. What AggregateEntry now holds ----------------------------- - 10 UTF8BytesString label fields (resource, service, operationName, serviceSource, type, spanKind, httpMethod, httpEndpoint, grpcStatusCode, and a List<UTF8BytesString> peerTags for serialization). - 3 primitives (httpStatusCode, synthetic, traceRoot). - AggregateMetric (the value being accumulated). - The raw String[] peerTagPairs is retained alongside the encoded peerTags -- matches() compares it positionally against the snapshot's pairs; the encoded form is only consumed by the writer. matches(SpanSnapshot) compares the entry's UTF8 forms to the snapshot's raw String / CharSequence fields via content-equality (UTF8BytesString.toString() returns the underlying String in O(1)). This closes a latent bug in the prior raw-vs-raw matches(): if one snapshot delivered a tag value as String and a later snapshot delivered the same content as UTF8BytesString, the old Objects.equals would return false and the table would split into two entries. Content-equality matching collapses them into one. Consolidated caches ------------------- The static UTF8 caches that used to live partly on MetricKey (RESOURCE_CACHE, OPERATION_CACHE, SERVICE_SOURCE_CACHE, TYPE_CACHE, KIND_CACHE, HTTP_METHOD_CACHE, HTTP_ENDPOINT_CACHE, GRPC_STATUS_CODE_CACHE, SERVICE_CACHE) and partly on ConflatingMetricsAggregator (SERVICE_NAMES, SPAN_KINDS, PEER_TAGS_CACHE) are all now on AggregateEntry. The split was duplicating work -- SERVICE_NAMES and SERVICE_CACHE both cached service-name to UTF8BytesString. One cache per field now. API change: MetricWriter.add ---------------------------- Was: add(MetricKey key, AggregateMetric aggregate) Now: add(AggregateEntry entry) The aggregate lives on the entry. Single-arg. SerializingMetricWriter reads the same UTF8 fields off AggregateEntry that it previously read off MetricKey; the wire format is byte-identical. Test impact ----------- AggregateEntry.of(...) takes the same 13 positional args new MetricKey(...) took, so test diffs are mostly mechanical: new MetricKey(args) -> AggregateEntry.of(args) writer.add(key, _) -> writer.add(entry) ValidatingSink in SerializingMetricWriterTest now iterates List<AggregateEntry> directly. ConflatingMetricAggregatorTest's Spock matchers (~36 sites) rely on AggregateEntry.equals comparing the 13 label fields (not the aggregate) so the mock matches by labels regardless of the aggregate state at call time; post-invocation closures verify aggregate state. Benchmarks (2 forks x 5 iter x 15s) ----------------------------------- The change is consumer-thread only; producer publish() is unchanged. SimpleSpan bench: 3.123 +- 0.025 us/op (prior: 3.119 +- 0.018) DDSpan bench: 2.412 +- 0.022 us/op (prior: 2.463 +- 0.041) Both within noise -- the win is structural (one less class, one less allocation per miss, one fewer cache layer) rather than benchmarked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f1b030a commit 3738c85

13 files changed

Lines changed: 609 additions & 713 deletions

File tree

Lines changed: 315 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,186 @@
11
package datadog.trace.common.metrics;
22

3+
import static datadog.trace.api.Functions.UTF8_ENCODE;
4+
import static datadog.trace.bootstrap.instrumentation.api.UTF8BytesString.EMPTY;
5+
6+
import datadog.trace.api.Pair;
7+
import datadog.trace.api.cache.DDCache;
8+
import datadog.trace.api.cache.DDCaches;
9+
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
310
import datadog.trace.util.Hashtable;
411
import datadog.trace.util.LongHashingUtils;
12+
import java.util.ArrayList;
513
import java.util.Arrays;
6-
import java.util.Objects;
14+
import java.util.Collections;
15+
import java.util.List;
16+
import java.util.function.Function;
717

818
/**
9-
* Hashtable entry pairing the raw {@link SpanSnapshot} key fields with their canonical {@link
10-
* MetricKey} (built once on miss) and the mutable {@link AggregateMetric}.
19+
* Hashtable entry for the consumer-side aggregator. Holds the UTF8-encoded label fields (the data
20+
* {@link SerializingMetricWriter} writes to the wire) plus the mutable {@link AggregateMetric}.
21+
*
22+
* <p>{@link #matches(SpanSnapshot)} compares the entry's stored UTF8 forms against the snapshot's
23+
* raw {@code CharSequence}/{@code String}/{@code String[]} fields via content-equality, so {@code
24+
* String} vs {@code UTF8BytesString} mixing on the same logical key collapses into one entry
25+
* instead of splitting.
1126
*
12-
* <p>Lookups compare the snapshot's raw fields against the entry's stored copies, so the consumer
13-
* never has to build a {@link MetricKey} just to do a HashMap lookup. The {@code MetricKey} field
14-
* is retained because the serializer ({@link MetricWriter#add}) needs it at report time.
27+
* <p>The static UTF8 caches that used to live on {@code MetricKey} and {@code
28+
* ConflatingMetricsAggregator} are consolidated here.
1529
*/
1630
final class AggregateEntry extends Hashtable.Entry {
17-
final MetricKey key;
18-
final AggregateMetric aggregate;
1931

20-
// Raw snapshot fields, used by matches(SpanSnapshot). Stored as captured at insert time;
21-
// the canonical MetricKey above holds the UTF8BytesString-encoded forms.
22-
private final CharSequence resourceName;
23-
private final String serviceName;
24-
private final CharSequence operationName;
25-
private final CharSequence serviceNameSource;
26-
private final CharSequence spanType;
32+
// UTF8 caches consolidated from the previous MetricKey + ConflatingMetricsAggregator split.
33+
private static final DDCache<String, UTF8BytesString> RESOURCE_CACHE =
34+
DDCaches.newFixedSizeCache(32);
35+
private static final DDCache<String, UTF8BytesString> SERVICE_CACHE =
36+
DDCaches.newFixedSizeCache(32);
37+
private static final DDCache<String, UTF8BytesString> OPERATION_CACHE =
38+
DDCaches.newFixedSizeCache(64);
39+
private static final DDCache<String, UTF8BytesString> SERVICE_SOURCE_CACHE =
40+
DDCaches.newFixedSizeCache(16);
41+
private static final DDCache<String, UTF8BytesString> TYPE_CACHE = DDCaches.newFixedSizeCache(8);
42+
private static final DDCache<String, UTF8BytesString> SPAN_KIND_CACHE =
43+
DDCaches.newFixedSizeCache(16);
44+
private static final DDCache<String, UTF8BytesString> HTTP_METHOD_CACHE =
45+
DDCaches.newFixedSizeCache(8);
46+
private static final DDCache<String, UTF8BytesString> HTTP_ENDPOINT_CACHE =
47+
DDCaches.newFixedSizeCache(32);
48+
private static final DDCache<String, UTF8BytesString> GRPC_STATUS_CODE_CACHE =
49+
DDCaches.newFixedSizeCache(32);
50+
51+
/**
52+
* Outer cache keyed by peer-tag name, with an inner per-name cache keyed by value. The inner
53+
* cache produces the "name:value" encoded form the serializer writes.
54+
*/
55+
private static final DDCache<
56+
String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>>
57+
PEER_TAGS_CACHE = DDCaches.newFixedSizeCache(64);
58+
59+
private static final Function<
60+
String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>>
61+
PEER_TAGS_CACHE_ADDER =
62+
key ->
63+
Pair.of(
64+
DDCaches.newFixedSizeCache(512),
65+
value -> UTF8BytesString.create(key + ":" + value));
66+
67+
private final UTF8BytesString resource;
68+
private final UTF8BytesString service;
69+
private final UTF8BytesString operationName;
70+
private final UTF8BytesString serviceSource; // nullable
71+
private final UTF8BytesString type;
72+
private final UTF8BytesString spanKind;
73+
private final UTF8BytesString httpMethod; // nullable
74+
private final UTF8BytesString httpEndpoint; // nullable
75+
private final UTF8BytesString grpcStatusCode; // nullable
2776
private final short httpStatusCode;
2877
private final boolean synthetic;
2978
private final boolean traceRoot;
30-
private final String spanKind;
31-
private final String[] peerTagPairs;
32-
private final String httpMethod;
33-
private final String httpEndpoint;
34-
private final String grpcStatusCode;
35-
36-
AggregateEntry(MetricKey key, SpanSnapshot s, AggregateMetric aggregate) {
37-
super(hashOf(s));
38-
this.key = key;
39-
this.aggregate = aggregate;
40-
this.resourceName = s.resourceName;
41-
this.serviceName = s.serviceName;
42-
this.operationName = s.operationName;
43-
this.serviceNameSource = s.serviceNameSource;
44-
this.spanType = s.spanType;
79+
80+
// Peer tags carried in two forms: raw String[] for matches() against the snapshot's pairs,
81+
// and pre-encoded List<UTF8BytesString> ("name:value") for the serializer.
82+
private final String[] peerTagPairsRaw;
83+
private final List<UTF8BytesString> peerTags;
84+
85+
final AggregateMetric aggregate;
86+
87+
/** Hot-path constructor for the producer/consumer flow. Builds UTF8 fields via the caches. */
88+
private AggregateEntry(SpanSnapshot s, long keyHash, AggregateMetric aggregate) {
89+
super(keyHash);
90+
this.resource = canonicalize(RESOURCE_CACHE, s.resourceName);
91+
this.service = SERVICE_CACHE.computeIfAbsent(s.serviceName, UTF8_ENCODE);
92+
this.operationName = canonicalize(OPERATION_CACHE, s.operationName);
93+
this.serviceSource =
94+
s.serviceNameSource == null
95+
? null
96+
: canonicalize(SERVICE_SOURCE_CACHE, s.serviceNameSource);
97+
this.type = canonicalize(TYPE_CACHE, s.spanType);
98+
this.spanKind = SPAN_KIND_CACHE.computeIfAbsent(s.spanKind, UTF8BytesString::create);
99+
this.httpMethod =
100+
s.httpMethod == null
101+
? null
102+
: HTTP_METHOD_CACHE.computeIfAbsent(s.httpMethod, UTF8BytesString::create);
103+
this.httpEndpoint =
104+
s.httpEndpoint == null
105+
? null
106+
: HTTP_ENDPOINT_CACHE.computeIfAbsent(s.httpEndpoint, UTF8BytesString::create);
107+
this.grpcStatusCode =
108+
s.grpcStatusCode == null
109+
? null
110+
: GRPC_STATUS_CODE_CACHE.computeIfAbsent(s.grpcStatusCode, UTF8BytesString::create);
45111
this.httpStatusCode = s.httpStatusCode;
46112
this.synthetic = s.synthetic;
47113
this.traceRoot = s.traceRoot;
48-
this.spanKind = s.spanKind;
49-
this.peerTagPairs = s.peerTagPairs;
50-
this.httpMethod = s.httpMethod;
51-
this.httpEndpoint = s.httpEndpoint;
52-
this.grpcStatusCode = s.grpcStatusCode;
114+
this.peerTagPairsRaw = s.peerTagPairs;
115+
this.peerTags = materializePeerTags(s.peerTagPairs);
116+
this.aggregate = aggregate;
117+
}
118+
119+
/** Test-friendly factory mirroring the prior {@code new MetricKey(...)} positional args. */
120+
static AggregateEntry of(
121+
CharSequence resource,
122+
CharSequence service,
123+
CharSequence operationName,
124+
CharSequence serviceSource,
125+
CharSequence type,
126+
int httpStatusCode,
127+
boolean synthetic,
128+
boolean traceRoot,
129+
CharSequence spanKind,
130+
List<UTF8BytesString> peerTags,
131+
CharSequence httpMethod,
132+
CharSequence httpEndpoint,
133+
CharSequence grpcStatusCode) {
134+
String[] rawPairs = peerTagsToRawPairs(peerTags);
135+
SpanSnapshot synthetic_snapshot =
136+
new SpanSnapshot(
137+
resource,
138+
service == null ? null : service.toString(),
139+
operationName,
140+
serviceSource,
141+
type,
142+
(short) httpStatusCode,
143+
synthetic,
144+
traceRoot,
145+
spanKind == null ? null : spanKind.toString(),
146+
rawPairs,
147+
httpMethod == null ? null : httpMethod.toString(),
148+
httpEndpoint == null ? null : httpEndpoint.toString(),
149+
grpcStatusCode == null ? null : grpcStatusCode.toString(),
150+
0L);
151+
return new AggregateEntry(
152+
synthetic_snapshot, hashOf(synthetic_snapshot), new AggregateMetric());
153+
}
154+
155+
/** Construct from a snapshot at consumer-thread miss time. */
156+
static AggregateEntry forSnapshot(SpanSnapshot s, AggregateMetric aggregate) {
157+
return new AggregateEntry(s, hashOf(s), aggregate);
53158
}
54159

55160
boolean matches(SpanSnapshot s) {
56161
return httpStatusCode == s.httpStatusCode
57162
&& synthetic == s.synthetic
58163
&& traceRoot == s.traceRoot
59-
&& Objects.equals(resourceName, s.resourceName)
60-
&& Objects.equals(serviceName, s.serviceName)
61-
&& Objects.equals(operationName, s.operationName)
62-
&& Objects.equals(serviceNameSource, s.serviceNameSource)
63-
&& Objects.equals(spanType, s.spanType)
64-
&& Objects.equals(spanKind, s.spanKind)
65-
&& Arrays.equals(peerTagPairs, s.peerTagPairs)
66-
&& Objects.equals(httpMethod, s.httpMethod)
67-
&& Objects.equals(httpEndpoint, s.httpEndpoint)
68-
&& Objects.equals(grpcStatusCode, s.grpcStatusCode);
164+
&& contentEquals(resource, s.resourceName)
165+
&& stringContentEquals(service, s.serviceName)
166+
&& contentEquals(operationName, s.operationName)
167+
&& contentEquals(serviceSource, s.serviceNameSource)
168+
&& contentEquals(type, s.spanType)
169+
&& stringContentEquals(spanKind, s.spanKind)
170+
&& Arrays.equals(peerTagPairsRaw, s.peerTagPairs)
171+
&& stringContentEquals(httpMethod, s.httpMethod)
172+
&& stringContentEquals(httpEndpoint, s.httpEndpoint)
173+
&& stringContentEquals(grpcStatusCode, s.grpcStatusCode);
69174
}
70175

71176
/**
72177
* Computes the 64-bit lookup hash for a {@link SpanSnapshot}. Chained per-field calls -- no
73178
* varargs / Object[] allocation, no autoboxing on primitive overloads. The constructor's
74179
* super({@code hashOf(s)}) call uses the same function so an entry built from a snapshot hashes
75180
* to the same bucket the snapshot itself looks up.
181+
*
182+
* <p>Hashes are content-stable across {@code String} / {@code UTF8BytesString}: {@link
183+
* UTF8BytesString#hashCode()} returns the underlying {@code String}'s hash.
76184
*/
77185
static long hashOf(SpanSnapshot s) {
78186
long h = 0;
@@ -95,4 +203,166 @@ static long hashOf(SpanSnapshot s) {
95203
h = LongHashingUtils.addToHash(h, s.grpcStatusCode);
96204
return h;
97205
}
206+
207+
// Accessors for SerializingMetricWriter.
208+
UTF8BytesString getResource() {
209+
return resource;
210+
}
211+
212+
UTF8BytesString getService() {
213+
return service;
214+
}
215+
216+
UTF8BytesString getOperationName() {
217+
return operationName;
218+
}
219+
220+
UTF8BytesString getServiceSource() {
221+
return serviceSource;
222+
}
223+
224+
UTF8BytesString getType() {
225+
return type;
226+
}
227+
228+
UTF8BytesString getSpanKind() {
229+
return spanKind;
230+
}
231+
232+
UTF8BytesString getHttpMethod() {
233+
return httpMethod;
234+
}
235+
236+
UTF8BytesString getHttpEndpoint() {
237+
return httpEndpoint;
238+
}
239+
240+
UTF8BytesString getGrpcStatusCode() {
241+
return grpcStatusCode;
242+
}
243+
244+
int getHttpStatusCode() {
245+
return httpStatusCode;
246+
}
247+
248+
boolean isSynthetics() {
249+
return synthetic;
250+
}
251+
252+
boolean isTraceRoot() {
253+
return traceRoot;
254+
}
255+
256+
List<UTF8BytesString> getPeerTags() {
257+
return peerTags;
258+
}
259+
260+
/**
261+
* Equality on the 13 label fields (not on the aggregate). Used only by test mock matchers; the
262+
* {@link Hashtable} does its own bucketing via {@link #keyHash} + {@link #matches(SpanSnapshot)}
263+
* and never calls {@code equals}.
264+
*/
265+
@Override
266+
public boolean equals(Object o) {
267+
if (this == o) return true;
268+
if (!(o instanceof AggregateEntry)) return false;
269+
AggregateEntry that = (AggregateEntry) o;
270+
return httpStatusCode == that.httpStatusCode
271+
&& synthetic == that.synthetic
272+
&& traceRoot == that.traceRoot
273+
&& java.util.Objects.equals(resource, that.resource)
274+
&& java.util.Objects.equals(service, that.service)
275+
&& java.util.Objects.equals(operationName, that.operationName)
276+
&& java.util.Objects.equals(serviceSource, that.serviceSource)
277+
&& java.util.Objects.equals(type, that.type)
278+
&& java.util.Objects.equals(spanKind, that.spanKind)
279+
&& peerTags.equals(that.peerTags)
280+
&& java.util.Objects.equals(httpMethod, that.httpMethod)
281+
&& java.util.Objects.equals(httpEndpoint, that.httpEndpoint)
282+
&& java.util.Objects.equals(grpcStatusCode, that.grpcStatusCode);
283+
}
284+
285+
@Override
286+
public int hashCode() {
287+
return (int) keyHash;
288+
}
289+
290+
// ----- helpers -----
291+
292+
private static UTF8BytesString canonicalize(
293+
DDCache<String, UTF8BytesString> cache, CharSequence charSeq) {
294+
if (charSeq == null) {
295+
return EMPTY;
296+
}
297+
if (charSeq instanceof UTF8BytesString) {
298+
return (UTF8BytesString) charSeq;
299+
}
300+
return cache.computeIfAbsent(charSeq.toString(), UTF8BytesString::create);
301+
}
302+
303+
/** UTF8 vs raw CharSequence content-equality, no allocation in the common (String) case. */
304+
private static boolean contentEquals(UTF8BytesString a, CharSequence b) {
305+
if (a == null) {
306+
return b == null;
307+
}
308+
if (b == null) {
309+
return false;
310+
}
311+
// UTF8BytesString.toString() returns the underlying String -- O(1), no allocation.
312+
String aStr = a.toString();
313+
if (b instanceof String) {
314+
return aStr.equals(b);
315+
}
316+
if (b instanceof UTF8BytesString) {
317+
return aStr.equals(b.toString());
318+
}
319+
return aStr.contentEquals(b);
320+
}
321+
322+
private static boolean stringContentEquals(UTF8BytesString a, String b) {
323+
if (a == null) {
324+
return b == null;
325+
}
326+
return b != null && a.toString().equals(b);
327+
}
328+
329+
private static List<UTF8BytesString> materializePeerTags(String[] pairs) {
330+
if (pairs == null || pairs.length == 0) {
331+
return Collections.emptyList();
332+
}
333+
if (pairs.length == 2) {
334+
return Collections.singletonList(encodePeerTag(pairs[0], pairs[1]));
335+
}
336+
List<UTF8BytesString> tags = new ArrayList<>(pairs.length / 2);
337+
for (int i = 0; i < pairs.length; i += 2) {
338+
tags.add(encodePeerTag(pairs[i], pairs[i + 1]));
339+
}
340+
return tags;
341+
}
342+
343+
private static UTF8BytesString encodePeerTag(String name, String value) {
344+
final Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>
345+
cacheAndCreator = PEER_TAGS_CACHE.computeIfAbsent(name, PEER_TAGS_CACHE_ADDER);
346+
return cacheAndCreator.getLeft().computeIfAbsent(value, cacheAndCreator.getRight());
347+
}
348+
349+
/**
350+
* Inverse of {@link #materializePeerTags}: takes pre-encoded UTF8 peer tags and recovers the raw
351+
* {@code [name0, value0, name1, value1, ...]} pairs. Used by the test factory {@link #of}, not by
352+
* the hot path.
353+
*/
354+
private static String[] peerTagsToRawPairs(List<UTF8BytesString> peerTags) {
355+
if (peerTags == null || peerTags.isEmpty()) {
356+
return null;
357+
}
358+
String[] pairs = new String[peerTags.size() * 2];
359+
int i = 0;
360+
for (UTF8BytesString peerTag : peerTags) {
361+
String s = peerTag.toString();
362+
int colon = s.indexOf(':');
363+
pairs[i++] = colon < 0 ? s : s.substring(0, colon);
364+
pairs[i++] = colon < 0 ? "" : s.substring(colon + 1);
365+
}
366+
return pairs;
367+
}
98368
}

0 commit comments

Comments
 (0)