Skip to content

Commit ec7ebc7

Browse files
daniel-mohedanodaniel-mohedano
authored
Improve tag propagation between test event levels (#9278)
1 parent 82e4ecc commit ec7ebc7

File tree

13 files changed

+278
-78
lines changed

13 files changed

+278
-78
lines changed

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public abstract class AbstractTestModule {
3030
protected final Codeowners codeowners;
3131
protected final LinesResolver linesResolver;
3232
private final Consumer<AgentSpan> onSpanFinish;
33+
protected final SpanTagsPropagator tagsPropagator;
3334

3435
public AbstractTestModule(
3536
AgentSpanContext sessionSpanContext,
@@ -63,6 +64,7 @@ public AbstractTestModule(
6364
}
6465

6566
span = spanBuilder.start();
67+
tagsPropagator = new SpanTagsPropagator(span);
6668

6769
span.setSpanType(InternalSpanTypes.TEST_MODULE_END);
6870
span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_MODULE);

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public abstract class AbstractTestSession {
4646
protected final SourcePathResolver sourcePathResolver;
4747
protected final Codeowners codeowners;
4848
protected final LinesResolver linesResolver;
49+
protected final SpanTagsPropagator tagPropagator;
4950

5051
public AbstractTestSession(
5152
String projectName,
@@ -93,6 +94,7 @@ public AbstractTestSession(
9394
}
9495

9596
span = spanBuilder.start();
97+
tagPropagator = new SpanTagsPropagator(span);
9698

9799
span.setSpanType(InternalSpanTypes.TEST_SESSION_END);
98100
span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SESSION);

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java‎ renamed to ‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/SpanTagsPropagator.java‎

Lines changed: 64 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package datadog.trace.civisibility.utils;
1+
package datadog.trace.civisibility.domain;
22

33
import datadog.trace.api.civisibility.execution.TestStatus;
44
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
@@ -14,28 +14,51 @@
1414
import java.util.function.BinaryOperator;
1515
import java.util.function.Consumer;
1616

17-
public class SpanUtils {
18-
public static final Consumer<AgentSpan> DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS = span -> {};
17+
public class SpanTagsPropagator {
18+
public static final Consumer<AgentSpan> NOOP_PROPAGATOR = span -> {};
1919

20-
public static Consumer<AgentSpan> propagateCiVisibilityTagsTo(AgentSpan parentSpan) {
21-
return childSpan -> propagateCiVisibilityTags(parentSpan, childSpan);
20+
private final AgentSpan parentSpan;
21+
private final Object tagPropagationLock = new Object();
22+
23+
public SpanTagsPropagator(AgentSpan parentSpan) {
24+
this.parentSpan = parentSpan;
25+
}
26+
27+
public void propagateCiVisibilityTags(AgentSpan childSpan) {
28+
mergeTestFrameworks(getFrameworks(childSpan));
29+
propagateStatus(childSpan);
30+
}
31+
32+
public void propagateStatus(AgentSpan childSpan) {
33+
synchronized (tagPropagationLock) {
34+
unsafePropagateStatus(childSpan);
35+
}
36+
}
37+
38+
public void mergeTestFrameworks(Collection<TestFramework> testFrameworks) {
39+
synchronized (tagPropagationLock) {
40+
unsafeMergeTestFrameworks(testFrameworks);
41+
}
2242
}
2343

24-
public static void propagateCiVisibilityTags(AgentSpan parentSpan, AgentSpan childSpan) {
25-
mergeTestFrameworks(parentSpan, getFrameworks(childSpan));
26-
propagateStatus(parentSpan, childSpan);
44+
public void propagateTags(AgentSpan childSpan, TagMergeSpec<?>... specs) {
45+
synchronized (tagPropagationLock) {
46+
for (TagMergeSpec<?> spec : specs) {
47+
unsafePropagateTag(childSpan, spec);
48+
}
49+
}
2750
}
2851

29-
public static void mergeTestFrameworks(AgentSpan span, Collection<TestFramework> testFrameworks) {
30-
Collection<TestFramework> spanFrameworks = getFrameworks(span);
31-
Collection<TestFramework> merged = merge(spanFrameworks, testFrameworks);
32-
setFrameworks(span, merged);
52+
private void unsafeMergeTestFrameworks(Collection<TestFramework> childFrameworks) {
53+
Collection<TestFramework> parentFrameworks = getFrameworks(parentSpan);
54+
Collection<TestFramework> merged = merge(parentFrameworks, childFrameworks);
55+
setFrameworks(merged);
3356
}
3457

35-
private static Collection<TestFramework> getFrameworks(AgentSpan span) {
58+
static Collection<TestFramework> getFrameworks(AgentSpan span) {
3659
Object nameTag = span.getTag(Tags.TEST_FRAMEWORK);
3760
Object versionTag = span.getTag(Tags.TEST_FRAMEWORK_VERSION);
38-
if (nameTag == null && versionTag == null) {
61+
if (nameTag == null) {
3962
return Collections.emptyList();
4063
}
4164

@@ -45,9 +68,11 @@ private static Collection<TestFramework> getFrameworks(AgentSpan span) {
4568

4669
} else if (nameTag instanceof Collection) {
4770
Iterator<String> names = ((Collection<String>) nameTag).iterator();
48-
Iterator<String> versions = ((Collection<String>) versionTag).iterator();
71+
Iterator<String> versions =
72+
versionTag != null ? ((Collection<String>) versionTag).iterator() : null;
4973
while (names.hasNext()) {
50-
frameworks.add(new TestFramework(names.next(), versions.next()));
74+
String version = (versions != null && versions.hasNext()) ? versions.next() : null;
75+
frameworks.add(new TestFramework(names.next(), version));
5176
}
5277

5378
} else {
@@ -79,7 +104,7 @@ private static Collection<TestFramework> merge(
79104
return merged;
80105
}
81106

82-
private static void setFrameworks(AgentSpan span, Collection<TestFramework> frameworks) {
107+
private void setFrameworks(Collection<TestFramework> frameworks) {
83108
if (frameworks.isEmpty()) {
84109
return;
85110
}
@@ -90,7 +115,7 @@ private static void setFrameworks(AgentSpan span, Collection<TestFramework> fram
90115
if (framework.getVersion() != null) {
91116
tags.put(Tags.TEST_FRAMEWORK_VERSION, framework.getVersion());
92117
}
93-
span.setAllTags(tags);
118+
parentSpan.setAllTags(tags);
94119
return;
95120
}
96121
Collection<String> names = new ArrayList<>(frameworks.size());
@@ -102,10 +127,10 @@ private static void setFrameworks(AgentSpan span, Collection<TestFramework> fram
102127
Map<String, Collection<String>> tags = new HashMap<>();
103128
tags.put(Tags.TEST_FRAMEWORK, names);
104129
tags.put(Tags.TEST_FRAMEWORK_VERSION, versions);
105-
span.setAllTags(tags);
130+
parentSpan.setAllTags(tags);
106131
}
107132

108-
private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) {
133+
private void unsafePropagateStatus(AgentSpan childSpan) {
109134
TestStatus childStatus = (TestStatus) childSpan.getTag(Tags.TEST_STATUS);
110135
if (childStatus == null) {
111136
return;
@@ -131,25 +156,32 @@ private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) {
131156
}
132157
}
133158

134-
public static void propagateTags(AgentSpan parentSpan, AgentSpan childSpan, String... tagNames) {
135-
for (String tagName : tagNames) {
136-
parentSpan.setTag(tagName, childSpan.getTag(tagName));
159+
public static class TagMergeSpec<T> {
160+
private final String tagKey;
161+
private final BinaryOperator<T> mergeFunction;
162+
163+
TagMergeSpec(String tagKey, BinaryOperator<T> mergeFunction) {
164+
this.tagKey = tagKey;
165+
this.mergeFunction = mergeFunction;
166+
}
167+
168+
public static <T> TagMergeSpec<T> of(String key, BinaryOperator<T> mergeFunction) {
169+
return new TagMergeSpec<>(key, mergeFunction);
137170
}
138-
}
139171

140-
public static <T> void propagateTag(AgentSpan parentSpan, AgentSpan childSpan, String tagName) {
141-
propagateTag(parentSpan, childSpan, tagName, (p, c) -> c);
172+
public static TagMergeSpec<Object> of(String tagKey) {
173+
return new TagMergeSpec<>(tagKey, (parent, child) -> child);
174+
}
142175
}
143176

144-
public static <T> void propagateTag(
145-
AgentSpan parentSpan, AgentSpan childSpan, String tagName, BinaryOperator<T> mergeStrategy) {
146-
T childTag = (T) childSpan.getTag(tagName);
177+
private <T> void unsafePropagateTag(AgentSpan childSpan, TagMergeSpec<T> spec) {
178+
T childTag = (T) childSpan.getTag(spec.tagKey);
147179
if (childTag != null) {
148-
T parentTag = (T) parentSpan.getTag(tagName);
180+
T parentTag = (T) parentSpan.getTag(spec.tagKey);
149181
if (parentTag == null) {
150-
parentSpan.setTag(tagName, childTag);
182+
parentSpan.setTag(spec.tagKey, childTag);
151183
} else {
152-
parentSpan.setTag(tagName, mergeStrategy.apply(parentTag, childTag));
184+
parentSpan.setTag(spec.tagKey, spec.mergeFunction.apply(parentTag, childTag));
153185
}
154186
}
155187
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import datadog.trace.civisibility.source.SourcePathResolver;
2525
import datadog.trace.civisibility.source.SourceResolutionException;
2626
import datadog.trace.civisibility.test.ExecutionResults;
27-
import datadog.trace.civisibility.utils.SpanUtils;
2827
import java.lang.reflect.Method;
2928
import java.util.Collection;
3029
import java.util.function.Consumer;
@@ -56,6 +55,7 @@ public class TestSuiteImpl implements DDTestSuite {
5655
private final boolean parallelized;
5756
private final Collection<LibraryCapability> capabilities;
5857
private final Consumer<AgentSpan> onSpanFinish;
58+
private final SpanTagsPropagator tagsPropagator;
5959

6060
public TestSuiteImpl(
6161
AgentSpanContext moduleSpanContext,
@@ -106,6 +106,7 @@ public TestSuiteImpl(
106106
}
107107

108108
span = spanBuilder.start();
109+
tagsPropagator = new SpanTagsPropagator(span);
109110

110111
span.setSpanType(InternalSpanTypes.TEST_SUITE_END);
111112
span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SUITE);
@@ -264,6 +265,6 @@ public TestImpl testStart(
264265
coverageStoreFactory,
265266
executionResults,
266267
capabilities,
267-
SpanUtils.propagateCiVisibilityTagsTo(span));
268+
tagsPropagator::propagateStatus);
268269
}
269270
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import datadog.trace.civisibility.ipc.SignalType;
3131
import datadog.trace.civisibility.source.LinesResolver;
3232
import datadog.trace.civisibility.source.SourcePathResolver;
33-
import datadog.trace.civisibility.utils.SpanUtils;
3433
import datadog.trace.util.Strings;
3534
import java.net.InetSocketAddress;
3635
import java.nio.file.Path;
@@ -292,7 +291,7 @@ private SignalResponse onModuleExecutionResultReceived(ModuleExecutionResult res
292291

293292
testsSkipped.add(result.getTestsSkippedTotal());
294293

295-
SpanUtils.mergeTestFrameworks(span, result.getTestFrameworks());
294+
tagsPropagator.mergeTestFrameworks(result.getTestFrameworks());
296295

297296
return AckResponse.INSTANCE;
298297
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.civisibility.domain.buildsystem;
22

33
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
4+
import static datadog.trace.civisibility.domain.SpanTagsPropagator.TagMergeSpec;
45

56
import datadog.trace.api.Config;
67
import datadog.trace.api.DDTags;
@@ -35,7 +36,6 @@
3536
import datadog.trace.civisibility.source.SourcePathResolver;
3637
import datadog.trace.civisibility.source.index.RepoIndex;
3738
import datadog.trace.civisibility.source.index.RepoIndexProvider;
38-
import datadog.trace.civisibility.utils.SpanUtils;
3939
import java.nio.file.Path;
4040
import java.util.ArrayList;
4141
import java.util.Collection;
@@ -54,7 +54,6 @@ public class BuildSystemSessionImpl<T extends CoverageCalculator> extends Abstra
5454
private final CoverageCalculator.Factory<T> coverageCalculatorFactory;
5555
private final T coverageCalculator;
5656
private final BuildSessionSettings settings;
57-
private final Object tagPropagationLock = new Object();
5857

5958
public BuildSystemSessionImpl(
6059
String projectName,
@@ -183,22 +182,17 @@ public AgentSpan testTaskStart(String taskName) {
183182

184183
private void onModuleFinish(AgentSpan moduleSpan) {
185184
// multiple modules can finish in parallel
186-
synchronized (tagPropagationLock) {
187-
SpanUtils.propagateCiVisibilityTags(span, moduleSpan);
188-
189-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ENABLED, Boolean::logicalOr);
190-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ABORT_REASON);
191-
192-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_CODE_COVERAGE_ENABLED, Boolean::logicalOr);
193-
SpanUtils.propagateTag(
194-
span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, Boolean::logicalOr);
195-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_TYPE);
196-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_COUNT, Long::sum);
197-
SpanUtils.propagateTag(span, moduleSpan, DDTags.CI_ITR_TESTS_SKIPPED, Boolean::logicalOr);
198-
199-
SpanUtils.propagateTag(
200-
span, moduleSpan, Tags.TEST_TEST_MANAGEMENT_ENABLED, Boolean::logicalOr);
201-
}
185+
tagPropagator.propagateCiVisibilityTags(moduleSpan);
186+
tagPropagator.propagateTags(
187+
moduleSpan,
188+
TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ENABLED, Boolean::logicalOr),
189+
TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ABORT_REASON),
190+
TagMergeSpec.of(Tags.TEST_CODE_COVERAGE_ENABLED, Boolean::logicalOr),
191+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, Boolean::logicalOr),
192+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_TYPE),
193+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_COUNT, Long::sum),
194+
TagMergeSpec.of(DDTags.CI_ITR_TESTS_SKIPPED, Boolean::logicalOr),
195+
TagMergeSpec.of(Tags.TEST_TEST_MANAGEMENT_ENABLED, Boolean::logicalOr));
202196
}
203197

204198
@Override

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import datadog.trace.civisibility.source.SourcePathResolver;
2828
import datadog.trace.civisibility.test.ExecutionResults;
2929
import datadog.trace.civisibility.test.ExecutionStrategy;
30-
import datadog.trace.civisibility.utils.SpanUtils;
3130
import java.util.Collection;
3231
import java.util.function.Consumer;
3332
import javax.annotation.Nonnull;
@@ -183,6 +182,6 @@ public TestSuiteImpl testSuiteStart(
183182
coverageStoreFactory,
184183
executionResults,
185184
capabilities,
186-
SpanUtils.propagateCiVisibilityTagsTo(span));
185+
tagsPropagator::propagateCiVisibilityTags);
187186
}
188187
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package datadog.trace.civisibility.domain.headless;
22

3+
import static datadog.trace.civisibility.domain.SpanTagsPropagator.TagMergeSpec;
4+
35
import datadog.trace.api.Config;
46
import datadog.trace.api.DDTags;
57
import datadog.trace.api.civisibility.config.LibraryCapability;
@@ -19,7 +21,6 @@
1921
import datadog.trace.civisibility.source.LinesResolver;
2022
import datadog.trace.civisibility.source.SourcePathResolver;
2123
import datadog.trace.civisibility.test.ExecutionStrategy;
22-
import datadog.trace.civisibility.utils.SpanUtils;
2324
import java.util.Collection;
2425
import java.util.Collections;
2526
import javax.annotation.Nonnull;
@@ -82,22 +83,21 @@ public HeadlessTestModule testModuleStart(String moduleName, @Nullable Long star
8283
coverageStoreFactory,
8384
executionStrategy,
8485
capabilities,
85-
this::propagateModuleTags);
86+
this::onModuleFinish);
8687
}
8788

88-
private void propagateModuleTags(AgentSpan moduleSpan) {
89-
SpanUtils.propagateCiVisibilityTags(span, moduleSpan);
90-
SpanUtils.propagateTags(
91-
span,
89+
private void onModuleFinish(AgentSpan moduleSpan) {
90+
tagPropagator.propagateCiVisibilityTags(moduleSpan);
91+
tagPropagator.propagateTags(
9292
moduleSpan,
93-
Tags.TEST_CODE_COVERAGE_ENABLED,
94-
Tags.TEST_ITR_TESTS_SKIPPING_ENABLED,
95-
Tags.TEST_ITR_TESTS_SKIPPING_TYPE,
96-
Tags.TEST_ITR_TESTS_SKIPPING_COUNT,
97-
Tags.TEST_EARLY_FLAKE_ENABLED,
98-
Tags.TEST_EARLY_FLAKE_ABORT_REASON,
99-
DDTags.CI_ITR_TESTS_SKIPPED,
100-
Tags.TEST_TEST_MANAGEMENT_ENABLED);
93+
TagMergeSpec.of(Tags.TEST_CODE_COVERAGE_ENABLED),
94+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED),
95+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_TYPE),
96+
TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_COUNT),
97+
TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ENABLED),
98+
TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ABORT_REASON),
99+
TagMergeSpec.of(DDTags.CI_ITR_TESTS_SKIPPED),
100+
TagMergeSpec.of(Tags.TEST_TEST_MANAGEMENT_ENABLED));
101101
}
102102

103103
@Override

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import datadog.trace.civisibility.source.LinesResolver;
1616
import datadog.trace.civisibility.source.SourcePathResolver;
1717
import datadog.trace.civisibility.test.ExecutionResults;
18-
import datadog.trace.civisibility.utils.SpanUtils;
1918
import java.util.Collections;
2019
import java.util.function.Consumer;
2120
import javax.annotation.Nullable;
@@ -81,6 +80,6 @@ public TestSuiteImpl testSuiteStart(
8180
coverageStoreFactory,
8281
executionResults,
8382
Collections.emptyList(),
84-
SpanUtils.propagateCiVisibilityTagsTo(span));
83+
tagsPropagator::propagateCiVisibilityTags);
8584
}
8685
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import datadog.trace.civisibility.domain.InstrumentationType;
1212
import datadog.trace.civisibility.source.LinesResolver;
1313
import datadog.trace.civisibility.source.SourcePathResolver;
14-
import datadog.trace.civisibility.utils.SpanUtils;
1514
import javax.annotation.Nullable;
1615

1716
/**
@@ -60,6 +59,6 @@ public ManualApiTestModule testModuleStart(String moduleName, @Nullable Long sta
6059
codeowners,
6160
linesResolver,
6261
coverageStoreFactory,
63-
SpanUtils.propagateCiVisibilityTagsTo(span));
62+
tagPropagator::propagateCiVisibilityTags);
6463
}
6564
}

0 commit comments

Comments
 (0)