Skip to content

Commit b53a739

Browse files
bric3bric3
committed
chore(css): Use a map to create the metric key
Also delays the creation of the Utf8ByteString at serialization time. Note that `writer.writeUTF8` emits a header corresponding to the length of the value being written, calling this method again will count as an entry in the array. A possible idea was to use a cache to store the computation of the peerTags, however `Utf8ByteString` is not concatenable / appendable, which is necessary to have the proper encoding. Creating a new "temporary" `Utf8ByteString`, was replaced by a direct call to `String: :getBytes`.
1 parent b24f2e1 commit b53a739

File tree

8 files changed

+57
-46
lines changed

8 files changed

+57
-46
lines changed

‎dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@
2424
import datadog.trace.core.CoreSpan;
2525
import datadog.trace.core.DDTraceCoreInfo;
2626
import datadog.trace.util.AgentTaskScheduler;
27-
import datadog.trace.util.TraceUtils;
28-
import java.util.ArrayList;
2927
import java.util.Collections;
28+
import java.util.HashMap;
3029
import java.util.List;
3130
import java.util.Map;
3231
import java.util.Queue;
@@ -269,7 +268,7 @@ private boolean publish(CoreSpan<?> span, boolean isTopLevel) {
269268
// returning false means that either the batch can't take any
270269
// more data, or it has already been consumed
271270
if (batch.add(tag, durationNanos)) {
272-
// added to a pending batch prior to consumption
271+
// added to a pending batch prior to consumption,
273272
// so skip publishing to the queue (we also know
274273
// the key isn't rare enough to override the sampler)
275274
return false;
@@ -289,12 +288,12 @@ private boolean publish(CoreSpan<?> span, boolean isTopLevel) {
289288
return isNewKey || span.getError() > 0;
290289
}
291290

292-
private List<CharSequence> getPeerTags(CoreSpan<?> span) {
293-
List<CharSequence> peerTags = new ArrayList<>();
291+
private Map<String, String> getPeerTags(CoreSpan<?> span) {
292+
Map<String, String> peerTags = new HashMap<>();
294293
for (String peerTag : features.peerTags()) {
295294
Object value = span.getTag(peerTag);
296295
if (value != null) {
297-
peerTags.add(peerTag + ":" + TraceUtils.normalizeTag(value.toString()));
296+
peerTags.put(peerTag, value.toString());
298297
}
299298
}
300299
return peerTags;

‎dd-trace-core/src/main/java/datadog/trace/common/metrics/MetricKey.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
import static datadog.trace.bootstrap.instrumentation.api.UTF8BytesString.EMPTY;
44

55
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
6-
import java.util.Arrays;
7-
import java.util.List;
6+
import java.util.Collections;
7+
import java.util.Map;
88

99
/** The aggregation key for tracked metrics. */
1010
public final class MetricKey {
@@ -17,7 +17,7 @@ public final class MetricKey {
1717
private final int hash;
1818
private final boolean isTraceRoot;
1919
private final UTF8BytesString spanKind;
20-
private final UTF8BytesString[] peerTags;
20+
private final Map<String, String> peerTags;
2121

2222
public MetricKey(
2323
CharSequence resource,
@@ -28,7 +28,7 @@ public MetricKey(
2828
boolean synthetics,
2929
boolean isTraceRoot,
3030
CharSequence spanKind,
31-
List<CharSequence> peerTags) {
31+
Map<String, String> peerTags) {
3232
this.resource = null == resource ? EMPTY : UTF8BytesString.create(resource);
3333
this.service = null == service ? EMPTY : UTF8BytesString.create(service);
3434
this.operationName = null == operationName ? EMPTY : UTF8BytesString.create(operationName);
@@ -37,21 +37,19 @@ public MetricKey(
3737
this.synthetics = synthetics;
3838
this.isTraceRoot = isTraceRoot;
3939
this.spanKind = null == spanKind ? EMPTY : UTF8BytesString.create(spanKind);
40-
this.peerTags = new UTF8BytesString[peerTags.size()];
41-
for (int i = 0; i < peerTags.size(); i++) {
42-
this.peerTags[i] = UTF8BytesString.create(peerTags.get(i));
43-
}
40+
this.peerTags = peerTags == null ? Collections.emptyMap() : peerTags;
4441

4542
// Unrolled polynomial hashcode to avoid varargs allocation
4643
// and eliminate data dependency between iterations as in Arrays.hashCode.
4744
// Coefficient constants are powers of 31, with integer overflow (hence negative numbers).
4845
// See
4946
// https://richardstartin.github.io/posts/collecting-rocks-and-benchmarks
5047
// https://richardstartin.github.io/posts/still-true-in-java-9-handwritten-hash-codes-are-faster
48+
5149
this.hash =
5250
-196513505 * Boolean.hashCode(this.isTraceRoot)
5351
+ -1807454463 * this.spanKind.hashCode()
54-
+ 887_503_681 * Arrays.hashCode(this.peerTags)
52+
+ 887_503_681 * this.peerTags.hashCode() // possibly unroll here has well.
5553
+ 28_629_151 * this.resource.hashCode()
5654
+ 923_521 * this.service.hashCode()
5755
+ 29791 * this.operationName.hashCode()
@@ -92,7 +90,7 @@ public UTF8BytesString getSpanKind() {
9290
return spanKind;
9391
}
9492

95-
public UTF8BytesString[] getPeerTags() {
93+
public Map<String, String> getPeerTags() {
9694
return peerTags;
9795
}
9896

@@ -112,7 +110,7 @@ public boolean equals(Object o) {
112110
&& type.equals(metricKey.type)
113111
&& isTraceRoot == metricKey.isTraceRoot
114112
&& spanKind.equals(metricKey.spanKind)
115-
&& Arrays.equals(peerTags, metricKey.peerTags);
113+
&& peerTags.equals(metricKey.peerTags);
116114
}
117115
return false;
118116
}

‎dd-trace-core/src/main/java/datadog/trace/common/metrics/SerializingMetricWriter.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package datadog.trace.common.metrics;
22

33
import static java.nio.charset.StandardCharsets.ISO_8859_1;
4+
import static java.nio.charset.StandardCharsets.UTF_8;
45

56
import datadog.communication.serialization.GrowableBuffer;
67
import datadog.communication.serialization.WritableFormatter;
78
import datadog.communication.serialization.msgpack.MsgPackWriter;
89
import datadog.trace.api.ProcessTags;
910
import datadog.trace.api.WellKnownTags;
1011
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
12+
import datadog.trace.util.TraceUtils;
13+
import java.util.Map;
1114

1215
public final class SerializingMetricWriter implements MetricWriter {
1316

@@ -123,10 +126,19 @@ public void add(MetricKey key, AggregateMetric aggregate) {
123126
writer.writeUTF8(key.getSpanKind());
124127

125128
writer.writeUTF8(PEER_TAGS);
126-
UTF8BytesString[] peerTags = key.getPeerTags();
127-
writer.startArray(peerTags.length);
128-
for (UTF8BytesString peerTag : peerTags) {
129-
writer.writeUTF8(peerTag);
129+
Map<String, String> peerTags = key.getPeerTags();
130+
writer.startArray(peerTags.size());
131+
132+
StringBuilder peerTagBuilder = new StringBuilder();
133+
for (Map.Entry<String, String> peerTag : peerTags.entrySet()) {
134+
peerTagBuilder.setLength(0);
135+
String toWrite =
136+
peerTagBuilder
137+
.append(peerTag.getKey())
138+
.append(':')
139+
.append(TraceUtils.normalizeTag(peerTag.getValue()))
140+
.toString();
141+
writer.writeUTF8(toWrite.getBytes(UTF_8));
130142
}
131143

132144
writer.writeUTF8(HITS);

‎dd-trace-core/src/test/groovy/datadog/trace/common/metrics/AggregateMetricTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class AggregateMetricTest extends DDSpecification {
5151
given:
5252
AggregateMetric aggregate = new AggregateMetric().recordDurations(3, new AtomicLongArray(0L, 0L, 0L | ERROR_TAG | TOP_LEVEL_TAG))
5353

54-
Batch batch = new Batch().reset(new MetricKey("foo", "bar", "qux", "type", 0, false, true, "corge", ["grault"]))
54+
Batch batch = new Batch().reset(new MetricKey("foo", "bar", "qux", "type", 0, false, true, "corge", ["grault":"quux"]))
5555
batch.add(0L, 10)
5656
batch.add(0L, 10)
5757
batch.add(0L, 10)
@@ -126,7 +126,7 @@ class AggregateMetricTest extends DDSpecification {
126126
def "consistent under concurrent attempts to read and write"() {
127127
given:
128128
AggregateMetric aggregate = new AggregateMetric()
129-
MetricKey key = new MetricKey("foo", "bar", "qux", "type", 0, false, true, "corge", ["grault"])
129+
MetricKey key = new MetricKey("foo", "bar", "qux", "type", 0, false, true, "corge", ["grault":"quux"])
130130
BlockingDeque<Batch> queue = new LinkedBlockingDeque<>(1000)
131131
ExecutorService reader = Executors.newSingleThreadExecutor()
132132
int writerCount = 10

‎dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
125125
false,
126126
true,
127127
"baz",
128-
[]
128+
[:]
129129
), _) >> { MetricKey key, AggregateMetric value ->
130130
value.getHitCount() == 1 && value.getTopLevelCount() == 1 && value.getDuration() == 100
131131
}
@@ -168,7 +168,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
168168
false,
169169
false,
170170
kind,
171-
[]
171+
[:]
172172
), { AggregateMetric aggregateMetric ->
173173
aggregateMetric.getHitCount() == 1 && aggregateMetric.getTopLevelCount() == 0 && aggregateMetric.getDuration() == 100
174174
})
@@ -221,7 +221,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
221221
false,
222222
false,
223223
"grault",
224-
["country:france"]
224+
["country":"france"]
225225
), { AggregateMetric aggregateMetric ->
226226
aggregateMetric.getHitCount() == 1 && aggregateMetric.getTopLevelCount() == 0 && aggregateMetric.getDuration() == 100
227227
})
@@ -235,7 +235,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
235235
false,
236236
false,
237237
"grault",
238-
["country:france", "georegion:europe"]
238+
["country":"france", "georegion":"europe"]
239239
), { AggregateMetric aggregateMetric ->
240240
aggregateMetric.getHitCount() == 1 && aggregateMetric.getTopLevelCount() == 0 && aggregateMetric.getDuration() == 100
241241
})
@@ -277,7 +277,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
277277
false,
278278
topLevel,
279279
"baz",
280-
[]
280+
[:]
281281
), { AggregateMetric value ->
282282
value.getHitCount() == 1 && value.getTopLevelCount() == topLevelCount && value.getDuration() == 100
283283
})
@@ -333,7 +333,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
333333
false,
334334
false,
335335
"baz",
336-
[]
336+
[:]
337337
), { AggregateMetric value ->
338338
value.getHitCount() == count && value.getDuration() == count * duration
339339
})
@@ -346,7 +346,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
346346
false,
347347
false,
348348
"baz",
349-
[]
349+
[:]
350350
), { AggregateMetric value ->
351351
value.getHitCount() == count && value.getDuration() == count * duration * 2
352352
})
@@ -396,7 +396,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
396396
false,
397397
true,
398398
"baz",
399-
[]
399+
[:]
400400
), _) >> { MetricKey key, AggregateMetric value ->
401401
value.getHitCount() == 1 && value.getDuration() == duration
402402
}
@@ -410,7 +410,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
410410
false,
411411
true,
412412
"baz",
413-
[]
413+
[:]
414414
), _)
415415
1 * writer.finishBucket() >> { latch.countDown() }
416416

@@ -456,7 +456,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
456456
false,
457457
true,
458458
"baz",
459-
[]
459+
[:]
460460
), { AggregateMetric value ->
461461
value.getHitCount() == 1 && value.getDuration() == duration
462462
})
@@ -487,7 +487,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
487487
false,
488488
true,
489489
"baz",
490-
[]
490+
[:]
491491
),{ AggregateMetric value ->
492492
value.getHitCount() == 1 && value.getDuration() == duration
493493
})
@@ -501,7 +501,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
501501
false,
502502
true,
503503
"baz",
504-
[]
504+
[:]
505505
), _)
506506
1 * writer.finishBucket() >> { latch.countDown() }
507507

@@ -547,7 +547,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
547547
false,
548548
true,
549549
"quux",
550-
[]
550+
[:]
551551
), { AggregateMetric value ->
552552
value.getHitCount() == 1 && value.getDuration() == duration
553553
})
@@ -603,7 +603,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
603603
false,
604604
true,
605605
"garply",
606-
[]
606+
[:]
607607
), { AggregateMetric value ->
608608
value.getHitCount() == 1 && value.getDuration() == duration
609609
})

‎dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ class FootprintForkedTest extends DDSpecification {
3838
1000,
3939
1000,
4040
100,
41-
SECONDS
42-
)
41+
SECONDS)
4342
// Removing the 'features' as it's a mock, and mocks are heavyweight, e.g. around 22MiB
4443
def baseline = footprint(aggregator, features)
4544
aggregator.start()

‎dd-trace-core/src/test/groovy/datadog/trace/common/metrics/SerializingMetricWriterTest.groovy‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class SerializingMetricWriterTest extends DDSpecification {
5353
false,
5454
false,
5555
"client",
56-
["country:canada", "georegion:amer", "peer.service:remote-service"]
56+
["country":"canada", "georegion":"amer", "peer.service":"remote-service"]
5757
),
5858
new AggregateMetric().recordDurations(10, new AtomicLongArray(1L))
5959
),
@@ -67,7 +67,7 @@ class SerializingMetricWriterTest extends DDSpecification {
6767
true,
6868
false,
6969
"producer",
70-
["country:canada", "georegion:amer", "peer.service:remote-service"]
70+
["country":"canada", "georegion":"amer", "peer.service":"remote-service"]
7171
),
7272
new AggregateMetric().recordDurations(9, new AtomicLongArray(1L))
7373
)
@@ -83,7 +83,7 @@ class SerializingMetricWriterTest extends DDSpecification {
8383
false,
8484
false,
8585
"producer",
86-
["messaging.destination" + i]
86+
["messaging.destination" : "dest" + i]
8787
),
8888
new AggregateMetric().recordDurations(10, new AtomicLongArray(1L))
8989
)
@@ -175,9 +175,12 @@ class SerializingMetricWriterTest extends DDSpecification {
175175
++elementCount
176176
assert unpacker.unpackString() == "PeerTags"
177177
int peerTagsLength = unpacker.unpackArrayHeader()
178-
assert peerTagsLength == key.getPeerTags().length
178+
assert peerTagsLength == key.getPeerTags().size()
179179
for (int i = 0; i < peerTagsLength; i++) {
180-
assert unpacker.unpackString() == key.getPeerTags()[i] as String
180+
def string = unpacker.unpackString()
181+
def separatorPos = string.indexOf(':')
182+
def tagVal = key.getPeerTags()[string.substring(0, separatorPos)]
183+
assert tagVal == string.substring(separatorPos + 1)
181184
}
182185
++elementCount
183186
assert unpacker.unpackString() == "Hits"

‎dd-trace-core/src/traceAgentTest/groovy/MetricsIntegrationTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ class MetricsIntegrationTest extends AbstractTraceAgentTest {
3434
)
3535
writer.startBucket(2, System.nanoTime(), SECONDS.toNanos(10))
3636
writer.add(
37-
new MetricKey("resource1", "service1", "operation1", "sql", 0, false, true, "xyzzy", ["grault"]),
37+
new MetricKey("resource1", "service1", "operation1", "sql", 0, false, true, "xyzzy", ["grault":"quux"]),
3838
new AggregateMetric().recordDurations(5, new AtomicLongArray(2, 1, 2, 250, 4, 5))
3939
)
4040
writer.add(
41-
new MetricKey("resource2", "service2", "operation2", "web", 200, false, true, "xyzzy", ["grault"]),
41+
new MetricKey("resource2", "service2", "operation2", "web", 200, false, true, "xyzzy", ["grault":"quux"]),
4242
new AggregateMetric().recordDurations(10, new AtomicLongArray(1, 1, 200, 2, 3, 4, 5, 6, 7, 8, 9))
4343
)
4444
writer.finishBucket()

0 commit comments

Comments
 (0)