Skip to content

Commit 9cd40b6

Browse files
jpbempeljpbempel
committed
address comments
1 parent c71437c commit 9cd40b6

4 files changed

Lines changed: 34 additions & 47 deletions

File tree

‎dd-trace-core/src/test/java/datadog/trace/common/writer/DDAgentWriterTest.java‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static datadog.trace.common.writer.ddagent.PrioritizationStrategy.PublishResult.ENQUEUED_FOR_SERIALIZATION;
44
import static datadog.trace.common.writer.ddagent.PrioritizationStrategy.PublishResult.ENQUEUED_FOR_SINGLE_SPAN_SAMPLING;
5+
import static java.util.concurrent.TimeUnit.SECONDS;
56
import static org.junit.jupiter.api.Assertions.assertNotNull;
67
import static org.mockito.ArgumentMatchers.any;
78
import static org.mockito.ArgumentMatchers.anyInt;
@@ -23,11 +24,10 @@
2324
import datadog.trace.core.DDSpan;
2425
import datadog.trace.core.monitor.HealthMetrics;
2526
import datadog.trace.core.propagation.PropagationTags;
27+
import java.util.Arrays;
2628
import java.util.Collections;
2729
import java.util.List;
28-
import java.util.concurrent.TimeUnit;
2930
import org.junit.jupiter.api.AfterEach;
30-
import org.junit.jupiter.api.BeforeEach;
3131
import org.junit.jupiter.api.Test;
3232
import org.tabletest.junit.TableTest;
3333

@@ -37,25 +37,18 @@ class DDAgentWriterTest extends DDCoreJavaSpecification {
3737
TraceProcessingWorker worker = mock(TraceProcessingWorker.class);
3838
DDAgentFeaturesDiscovery discovery = mock(DDAgentFeaturesDiscovery.class);
3939
DDAgentApi api = mock(DDAgentApi.class);
40-
MonitoringImpl monitoring = new MonitoringImpl(StatsDClient.NO_OP, 1, TimeUnit.SECONDS);
40+
MonitoringImpl monitoring = new MonitoringImpl(StatsDClient.NO_OP, 1, SECONDS);
4141
PayloadDispatcherImpl dispatcher =
4242
new PayloadDispatcherImpl(new DDAgentMapperDiscovery(discovery), api, monitor, monitoring);
43-
DDAgentWriter writer = new DDAgentWriter(worker, dispatcher, monitor, 1, TimeUnit.SECONDS, false);
43+
DDAgentWriter writer = new DDAgentWriter(worker, dispatcher, monitor, 1, SECONDS, false);
4444

4545
// Only used to create spans
46-
CoreTracer dummyTracer;
47-
48-
@BeforeEach
49-
void setup() {
50-
dummyTracer = tracerBuilder().writer(new ListWriter()).build();
51-
}
46+
CoreTracer dummyTracer = tracerBuilder().writer(new ListWriter()).build();
5247

5348
@AfterEach
5449
void cleanup() {
5550
writer.close();
56-
if (dummyTracer != null) {
57-
dummyTracer.close();
58-
}
51+
dummyTracer.close();
5952
}
6053

6154
@Test
@@ -91,13 +84,13 @@ void testWriterStartClosed() {
9184

9285
@Test
9386
void testWriterFlush() {
94-
when(worker.flush(1, TimeUnit.SECONDS)).thenReturn(true, false);
87+
when(worker.flush(1, SECONDS)).thenReturn(true, false);
9588

9689
// first flush succeeds
9790
writer.flush();
9891

9992
// monitor is notified
100-
verify(worker).flush(1, TimeUnit.SECONDS);
93+
verify(worker).flush(1, SECONDS);
10194
verify(monitor).onFlush(false);
10295
verifyNoMoreInteractions(monitor, worker, discovery, api);
10396

@@ -107,7 +100,7 @@ void testWriterFlush() {
107100
writer.flush();
108101

109102
// no additional monitor notifications
110-
verify(worker).flush(1, TimeUnit.SECONDS);
103+
verify(worker).flush(1, SECONDS);
111104
verifyNoMoreInteractions(monitor, worker, discovery, api);
112105
}
113106

@@ -207,10 +200,9 @@ void testDroppedTraceIsCounted(PublishResult publishResult) {
207200
HealthMetrics localMonitor = mock(HealthMetrics.class);
208201
PayloadDispatcherImpl localDispatcher = mock(PayloadDispatcherImpl.class);
209202
DDAgentWriter localWriter =
210-
new DDAgentWriter(localWorker, localDispatcher, localMonitor, 1, TimeUnit.SECONDS, false);
203+
new DDAgentWriter(localWorker, localDispatcher, localMonitor, 1, SECONDS, false);
211204

212-
DDSpan p0 = newSpan();
213-
List<DDSpan> trace = java.util.Arrays.asList(p0, newSpan());
205+
List<DDSpan> trace = Arrays.asList(newSpan(), newSpan());
214206

215207
when(localWorker.publish(eq(trace.get(0)), anyInt(), eq(trace))).thenReturn(publishResult);
216208
localWriter.write(trace);

‎dd-trace-core/src/test/java/datadog/trace/common/writer/SerializationTest.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,14 @@ class SerializationTest extends DDJavaSpecification {
1919

2020
@Test
2121
void testJsonMapperSerialization() throws Exception {
22-
// setup
2322
ObjectMapper mapper = new ObjectMapper();
2423
Map<String, String> map = singletonMap("key1", "val1");
2524
byte[] serializedMap = mapper.writeValueAsBytes(map);
2625
byte[] serializedList = ("[" + new String(serializedMap) + "]").getBytes();
2726

28-
// when
2927
List<Map<String, String>> result =
3028
mapper.readValue(serializedList, new TypeReference<List<Map<String, String>>>() {});
3129

32-
// then
3330
assertEquals(Collections.singletonList(map), result);
3431
assertEquals("[{\"key1\":\"val1\"}]", new String(serializedList));
3532
}
@@ -55,11 +52,9 @@ void testMsgpackMapperSerialization() throws Exception {
5552
}
5653
byte[] serializedList = packer.toByteArray();
5754

58-
// when
5955
List<Map<String, String>> result =
6056
mapper.readValue(serializedList, new TypeReference<List<Map<String, String>>>() {});
6157

62-
// then
6358
assertEquals(input, result);
6459
}
6560
}

‎dd-trace-core/src/test/java/datadog/trace/common/writer/ddagent/TraceMapperV04PayloadTest.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,6 @@ public void close() {}
442442
}
443443

444444
private static void assertEqualsWithNullAsEmpty(CharSequence expected, CharSequence actual) {
445-
if (null == expected) {
446-
assertEquals("", actual);
447-
} else {
448-
assertEquals(expected.toString(), actual.toString());
449-
}
445+
assertEquals(expected == null ? "" : expected.toString(), actual.toString());
450446
}
451447
}

‎dd-trace-core/src/test/java/datadog/trace/common/writer/ddagent/TraceMapperV05PayloadTest.java‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ void resetProcessTags() {
5353
@Test
5454
void testBodyOverflowCausesFlush() {
5555
// disable process tags since they are only on the first span of the chunk otherwise the
56-
// calculation woes
57-
// 4x 36 ASCII characters and 2 bytes of msgpack string prefix
56+
// calculation woes 4x 36 ASCII characters and 2 bytes of msgpack string prefix
5857
int dictionarySpacePerTrace = 4 * (36 + 2);
5958
// enough space for two traces with distinct string values, plus the header
6059
int dictionarySize = dictionarySpacePerTrace * 2 + 5;
@@ -263,22 +262,27 @@ public void accept(int messageCount, ByteBuffer buffer) {
263262
meta.put(dictionary[unpacker.unpackInt()], dictionary[unpacker.unpackInt()]);
264263
}
265264
for (Map.Entry<String, String> entry : meta.entrySet()) {
266-
if (Tags.HTTP_STATUS.equals(entry.getKey())) {
267-
assertEquals(String.valueOf(expectedSpan.getHttpStatusCode()), entry.getValue());
268-
} else if (DDTags.ORIGIN_KEY.equals(entry.getKey())) {
269-
assertEquals(expectedSpan.getOrigin(), entry.getValue());
270-
} else if (DDTags.PROCESS_TAGS.equals(entry.getKey())) {
271-
processTagsCount++;
272-
assertTrue(Config.get().isExperimentalPropagateProcessTagsEnabled());
273-
assertEquals(0, k);
274-
assertEquals(ProcessTags.getTagsForSerialization().toString(), entry.getValue());
275-
} else {
276-
Object tag = expectedSpan.getTag(entry.getKey());
277-
if (null != tag) {
278-
assertEquals(String.valueOf(tag), entry.getValue());
279-
} else {
280-
assertEquals(expectedSpan.getBaggage().get(entry.getKey()), entry.getValue());
281-
}
265+
switch (entry.getKey()) {
266+
case Tags.HTTP_STATUS:
267+
assertEquals(String.valueOf(expectedSpan.getHttpStatusCode()), entry.getValue());
268+
break;
269+
case DDTags.ORIGIN_KEY:
270+
assertEquals(expectedSpan.getOrigin(), entry.getValue());
271+
break;
272+
case DDTags.PROCESS_TAGS:
273+
processTagsCount++;
274+
assertTrue(Config.get().isExperimentalPropagateProcessTagsEnabled());
275+
assertEquals(0, k);
276+
assertEquals(ProcessTags.getTagsForSerialization().toString(), entry.getValue());
277+
break;
278+
default:
279+
Object tag = expectedSpan.getTag(entry.getKey());
280+
if (null != tag) {
281+
assertEquals(String.valueOf(tag), entry.getValue());
282+
} else {
283+
assertEquals(expectedSpan.getBaggage().get(entry.getKey()), entry.getValue());
284+
}
285+
break;
282286
}
283287
}
284288
int metricsSize = unpacker.unpackMapHeader();

0 commit comments

Comments
 (0)