Skip to content

Commit 5c3b967

Browse files
daniel-mohedanodaniel-mohedano
committed
feat: refactor SpanUtils into SpanTagsPropagator
1 parent b1103c5 commit 5c3b967

File tree

14 files changed

+287
-125
lines changed

14 files changed

+287
-125
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +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 Object tagPropagationLock = new Object();
33+
protected final SpanTagsPropagator tagsPropagator;
3434

3535
public AbstractTestModule(
3636
AgentSpanContext sessionSpanContext,
@@ -64,6 +64,7 @@ public AbstractTestModule(
6464
}
6565

6666
span = spanBuilder.start();
67+
tagsPropagator = new SpanTagsPropagator(span);
6768

6869
span.setSpanType(InternalSpanTypes.TEST_MODULE_END);
6970
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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public abstract class AbstractTestSession {
4646
protected final SourcePathResolver sourcePathResolver;
4747
protected final Codeowners codeowners;
4848
protected final LinesResolver linesResolver;
49-
protected final Object tagPropagationLock = new Object();
49+
protected final SpanTagsPropagator tagPropagator;
5050

5151
public AbstractTestSession(
5252
String projectName,
@@ -94,6 +94,7 @@ public AbstractTestSession(
9494
}
9595

9696
span = spanBuilder.start();
97+
tagPropagator = new SpanTagsPropagator(span);
9798

9899
span.setSpanType(InternalSpanTypes.TEST_SESSION_END);
99100
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: 58 additions & 43 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,38 +14,45 @@
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(
21-
AgentSpan parentSpan, Object lock, String... additionalTags) {
22-
return childSpan -> {
23-
synchronized (lock) {
24-
propagateCiVisibilityTags(parentSpan, childSpan);
25-
if (additionalTags != null) {
26-
propagateTags(parentSpan, childSpan, additionalTags);
27-
}
28-
}
29-
};
20+
private final AgentSpan parentSpan;
21+
private final Object tagPropagationLock = new Object();
22+
23+
public SpanTagsPropagator(AgentSpan parentSpan) {
24+
this.parentSpan = parentSpan;
3025
}
3126

32-
public static Consumer<AgentSpan> propagateStatusTo(AgentSpan parentSpan, Object lock) {
33-
return childSpan -> {
34-
synchronized (lock) {
35-
propagateStatus(parentSpan, childSpan);
36-
}
37-
};
27+
public void propagateCiVisibilityTags(AgentSpan childSpan) {
28+
mergeTestFrameworks(getFrameworks(childSpan));
29+
propagateStatus(childSpan);
3830
}
3931

40-
public static void propagateCiVisibilityTags(AgentSpan parentSpan, AgentSpan childSpan) {
41-
mergeTestFrameworks(parentSpan, getFrameworks(childSpan));
42-
propagateStatus(parentSpan, childSpan);
32+
public void propagateStatus(AgentSpan childSpan) {
33+
synchronized (tagPropagationLock) {
34+
unsafePropagateStatus(childSpan);
35+
}
4336
}
4437

45-
public static void mergeTestFrameworks(AgentSpan span, Collection<TestFramework> testFrameworks) {
46-
Collection<TestFramework> spanFrameworks = getFrameworks(span);
47-
Collection<TestFramework> merged = merge(spanFrameworks, testFrameworks);
48-
setFrameworks(span, merged);
38+
public void mergeTestFrameworks(Collection<TestFramework> testFrameworks) {
39+
synchronized (tagPropagationLock) {
40+
unsafeMergeTestFrameworks(testFrameworks);
41+
}
42+
}
43+
44+
public void propagateTags(AgentSpan childSpan, TagMergeSpec<?>... specs) {
45+
synchronized (tagPropagationLock) {
46+
for (TagMergeSpec<?> spec : specs) {
47+
unsafePropagateTag(childSpan, spec);
48+
}
49+
}
50+
}
51+
52+
private void unsafeMergeTestFrameworks(Collection<TestFramework> childFrameworks) {
53+
Collection<TestFramework> parentFrameworks = getFrameworks(parentSpan);
54+
Collection<TestFramework> merged = merge(parentFrameworks, childFrameworks);
55+
setFrameworks(merged);
4956
}
5057

5158
static Collection<TestFramework> getFrameworks(AgentSpan span) {
@@ -64,7 +71,8 @@ static Collection<TestFramework> getFrameworks(AgentSpan span) {
6471
Iterator<String> versions =
6572
versionTag != null ? ((Collection<String>) versionTag).iterator() : null;
6673
while (names.hasNext()) {
67-
frameworks.add(new TestFramework(names.next(), versions != null ? versions.next() : null));
74+
String version = (versions != null && versions.hasNext()) ? versions.next() : null;
75+
frameworks.add(new TestFramework(names.next(), version));
6876
}
6977

7078
} else {
@@ -96,7 +104,7 @@ private static Collection<TestFramework> merge(
96104
return merged;
97105
}
98106

99-
private static void setFrameworks(AgentSpan span, Collection<TestFramework> frameworks) {
107+
private void setFrameworks(Collection<TestFramework> frameworks) {
100108
if (frameworks.isEmpty()) {
101109
return;
102110
}
@@ -107,7 +115,7 @@ private static void setFrameworks(AgentSpan span, Collection<TestFramework> fram
107115
if (framework.getVersion() != null) {
108116
tags.put(Tags.TEST_FRAMEWORK_VERSION, framework.getVersion());
109117
}
110-
span.setAllTags(tags);
118+
parentSpan.setAllTags(tags);
111119
return;
112120
}
113121
Collection<String> names = new ArrayList<>(frameworks.size());
@@ -119,10 +127,10 @@ private static void setFrameworks(AgentSpan span, Collection<TestFramework> fram
119127
Map<String, Collection<String>> tags = new HashMap<>();
120128
tags.put(Tags.TEST_FRAMEWORK, names);
121129
tags.put(Tags.TEST_FRAMEWORK_VERSION, versions);
122-
span.setAllTags(tags);
130+
parentSpan.setAllTags(tags);
123131
}
124132

125-
private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) {
133+
private void unsafePropagateStatus(AgentSpan childSpan) {
126134
TestStatus childStatus = (TestStatus) childSpan.getTag(Tags.TEST_STATUS);
127135
if (childStatus == null) {
128136
return;
@@ -148,25 +156,32 @@ private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) {
148156
}
149157
}
150158

151-
public static void propagateTags(AgentSpan parentSpan, AgentSpan childSpan, String... tagNames) {
152-
for (String tagName : tagNames) {
153-
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);
154170
}
155-
}
156171

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

161-
public static <T> void propagateTag(
162-
AgentSpan parentSpan, AgentSpan childSpan, String tagName, BinaryOperator<T> mergeStrategy) {
163-
T childTag = (T) childSpan.getTag(tagName);
177+
private <T> void unsafePropagateTag(AgentSpan childSpan, TagMergeSpec<T> spec) {
178+
T childTag = (T) childSpan.getTag(spec.tagKey);
164179
if (childTag != null) {
165-
T parentTag = (T) parentSpan.getTag(tagName);
180+
T parentTag = (T) parentSpan.getTag(spec.tagKey);
166181
if (parentTag == null) {
167-
parentSpan.setTag(tagName, childTag);
182+
parentSpan.setTag(spec.tagKey, childTag);
168183
} else {
169-
parentSpan.setTag(tagName, mergeStrategy.apply(parentTag, childTag));
184+
parentSpan.setTag(spec.tagKey, spec.mergeFunction.apply(parentTag, childTag));
170185
}
171186
}
172187
}

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

Lines changed: 3 additions & 3 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,7 +55,7 @@ public class TestSuiteImpl implements DDTestSuite {
5655
private final boolean parallelized;
5756
private final Collection<LibraryCapability> capabilities;
5857
private final Consumer<AgentSpan> onSpanFinish;
59-
private final Object tagPropagationLock = new Object();
58+
private final SpanTagsPropagator tagsPropagator;
6059

6160
public TestSuiteImpl(
6261
AgentSpanContext moduleSpanContext,
@@ -107,6 +106,7 @@ public TestSuiteImpl(
107106
}
108107

109108
span = spanBuilder.start();
109+
tagsPropagator = new SpanTagsPropagator(span);
110110

111111
span.setSpanType(InternalSpanTypes.TEST_SUITE_END);
112112
span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SUITE);
@@ -265,6 +265,6 @@ public TestImpl testStart(
265265
coverageStoreFactory,
266266
executionResults,
267267
capabilities,
268-
SpanUtils.propagateStatusTo(span, tagPropagationLock));
268+
tagsPropagator::propagateStatus);
269269
}
270270
}

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

Lines changed: 1 addition & 5 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;
@@ -289,10 +288,7 @@ private SignalResponse onModuleExecutionResultReceived(ModuleExecutionResult res
289288

290289
testsSkipped.add(result.getTestsSkippedTotal());
291290

292-
synchronized (tagPropagationLock) {
293-
// avoids desync between read and merging
294-
SpanUtils.mergeTestFrameworks(span, result.getTestFrameworks());
295-
}
291+
tagsPropagator.mergeTestFrameworks(result.getTestFrameworks());
296292

297293
return AckResponse.INSTANCE;
298294
}

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

Lines changed: 12 additions & 17 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;
@@ -182,22 +182,17 @@ public AgentSpan testTaskStart(String taskName) {
182182

183183
private void onModuleFinish(AgentSpan moduleSpan) {
184184
// multiple modules can finish in parallel
185-
synchronized (tagPropagationLock) {
186-
SpanUtils.propagateCiVisibilityTags(span, moduleSpan);
187-
188-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ENABLED, Boolean::logicalOr);
189-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ABORT_REASON);
190-
191-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_CODE_COVERAGE_ENABLED, Boolean::logicalOr);
192-
SpanUtils.propagateTag(
193-
span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, Boolean::logicalOr);
194-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_TYPE);
195-
SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_COUNT, Long::sum);
196-
SpanUtils.propagateTag(span, moduleSpan, DDTags.CI_ITR_TESTS_SKIPPED, Boolean::logicalOr);
197-
198-
SpanUtils.propagateTag(
199-
span, moduleSpan, Tags.TEST_TEST_MANAGEMENT_ENABLED, Boolean::logicalOr);
200-
}
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));
201196
}
202197

203198
@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, tagPropagationLock));
185+
tagsPropagator::propagateCiVisibilityTags);
187186
}
188187
}

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

Lines changed: 18 additions & 12 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;
@@ -8,6 +10,7 @@
810
import datadog.trace.api.civisibility.telemetry.TagValue;
911
import datadog.trace.api.civisibility.telemetry.tag.EarlyFlakeDetectionAbortReason;
1012
import datadog.trace.api.civisibility.telemetry.tag.Provider;
13+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1114
import datadog.trace.bootstrap.instrumentation.api.Tags;
1215
import datadog.trace.civisibility.Constants;
1316
import datadog.trace.civisibility.codeowners.Codeowners;
@@ -18,7 +21,6 @@
1821
import datadog.trace.civisibility.source.LinesResolver;
1922
import datadog.trace.civisibility.source.SourcePathResolver;
2023
import datadog.trace.civisibility.test.ExecutionStrategy;
21-
import datadog.trace.civisibility.utils.SpanUtils;
2224
import java.util.Collection;
2325
import java.util.Collections;
2426
import javax.annotation.Nonnull;
@@ -81,17 +83,21 @@ public HeadlessTestModule testModuleStart(String moduleName, @Nullable Long star
8183
coverageStoreFactory,
8284
executionStrategy,
8385
capabilities,
84-
SpanUtils.propagateCiVisibilityTagsTo(
85-
span,
86-
tagPropagationLock,
87-
Tags.TEST_CODE_COVERAGE_ENABLED,
88-
Tags.TEST_ITR_TESTS_SKIPPING_ENABLED,
89-
Tags.TEST_ITR_TESTS_SKIPPING_TYPE,
90-
Tags.TEST_ITR_TESTS_SKIPPING_COUNT,
91-
Tags.TEST_EARLY_FLAKE_ENABLED,
92-
Tags.TEST_EARLY_FLAKE_ABORT_REASON,
93-
DDTags.CI_ITR_TESTS_SKIPPED,
94-
Tags.TEST_TEST_MANAGEMENT_ENABLED));
86+
this::onModuleFinish);
87+
}
88+
89+
private void onModuleFinish(AgentSpan moduleSpan) {
90+
tagPropagator.propagateCiVisibilityTags(moduleSpan);
91+
tagPropagator.propagateTags(
92+
moduleSpan,
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));
95101
}
96102

97103
@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, tagPropagationLock));
83+
tagsPropagator::propagateCiVisibilityTags);
8584
}
8685
}

0 commit comments

Comments
 (0)