From 24c15750e87e504933421178db7465c8a8282cce Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 30 Jul 2025 16:17:48 +0200 Subject: [PATCH 1/6] fix: ignore framework if name tag is null in span --- .../trace/civisibility/utils/SpanUtils.java | 9 ++--- .../civisibility/utils/SpanUtilsTest.groovy | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java index 64d154292b3..46634a0ec42 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java @@ -32,10 +32,10 @@ public static void mergeTestFrameworks(AgentSpan span, Collection setFrameworks(span, merged); } - private static Collection getFrameworks(AgentSpan span) { + static Collection getFrameworks(AgentSpan span) { Object nameTag = span.getTag(Tags.TEST_FRAMEWORK); Object versionTag = span.getTag(Tags.TEST_FRAMEWORK_VERSION); - if (nameTag == null && versionTag == null) { + if (nameTag == null) { return Collections.emptyList(); } @@ -45,9 +45,10 @@ private static Collection getFrameworks(AgentSpan span) { } else if (nameTag instanceof Collection) { Iterator names = ((Collection) nameTag).iterator(); - Iterator versions = ((Collection) versionTag).iterator(); + Iterator versions = + versionTag != null ? ((Collection) versionTag).iterator() : null; while (names.hasNext()) { - frameworks.add(new TestFramework(names.next(), versions.next())); + frameworks.add(new TestFramework(names.next(), versions != null ? versions.next() : null)); } } else { diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy new file mode 100644 index 00000000000..18508743281 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy @@ -0,0 +1,33 @@ +package datadog.trace.civisibility.utils + +import datadog.trace.bootstrap.instrumentation.api.Tags +import datadog.trace.civisibility.ipc.TestFramework +import datadog.trace.core.DDSpan +import spock.lang.Specification + +class SpanUtilsTest extends Specification { + def "test getFrameworks"() { + when: + def span = createSpanWithFrameworks(frameworkTag, frameworkVersionTag) + def frameworks = SpanUtils.getFrameworks(span) + + then: + frameworks == expected + + where: + frameworkTag | frameworkVersionTag | expected + "name" | "version" | [new TestFramework("name", "version")] + "name" | null | [new TestFramework("name", null)] + null | "version" | [] + ["nameA", "nameB"] | ["versionA", "versionB"] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", "versionB")] + ["nameA", "nameB"] | null | [new TestFramework("nameA", null), new TestFramework("nameB", null)] + ["nameA", "nameB"] | ["versionA", null] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", null)] + } + + DDSpan createSpanWithFrameworks(Object frameworkTag, Object frameworkVersionTag) { + def span = Stub(DDSpan) + span.getTag(Tags.TEST_FRAMEWORK) >> frameworkTag + span.getTag(Tags.TEST_FRAMEWORK_VERSION) >> frameworkVersionTag + return span + } +} From 6f06fdaf274281ec350b1c941744937bde30f281 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Fri, 1 Aug 2025 11:47:50 +0200 Subject: [PATCH 2/6] feat: synchronize on span propagation --- .../domain/AbstractTestModule.java | 1 + .../domain/AbstractTestSession.java | 1 + .../civisibility/domain/TestSuiteImpl.java | 3 ++- .../buildsystem/BuildSystemModuleImpl.java | 5 +++- .../buildsystem/BuildSystemSessionImpl.java | 1 - .../domain/headless/HeadlessTestModule.java | 2 +- .../domain/headless/HeadlessTestSession.java | 27 ++++++++----------- .../domain/manualapi/ManualApiTestModule.java | 2 +- .../manualapi/ManualApiTestSession.java | 2 +- .../trace/civisibility/utils/SpanUtils.java | 19 +++++++++++-- 10 files changed, 39 insertions(+), 24 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java index e87807d0ed0..841d919167c 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java @@ -30,6 +30,7 @@ public abstract class AbstractTestModule { protected final Codeowners codeowners; protected final LinesResolver linesResolver; private final Consumer onSpanFinish; + protected final Object tagPropagationLock = new Object(); public AbstractTestModule( AgentSpanContext sessionSpanContext, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java index 4338a9a7ba8..7278af1cac7 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java @@ -46,6 +46,7 @@ public abstract class AbstractTestSession { protected final SourcePathResolver sourcePathResolver; protected final Codeowners codeowners; protected final LinesResolver linesResolver; + protected final Object tagPropagationLock = new Object(); public AbstractTestSession( String projectName, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index 24319318968..a3c7acc1daa 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -56,6 +56,7 @@ public class TestSuiteImpl implements DDTestSuite { private final boolean parallelized; private final Collection capabilities; private final Consumer onSpanFinish; + private final Object tagPropagationLock = new Object(); public TestSuiteImpl( AgentSpanContext moduleSpanContext, @@ -264,6 +265,6 @@ public TestImpl testStart( coverageStoreFactory, executionResults, capabilities, - SpanUtils.propagateCiVisibilityTagsTo(span)); + SpanUtils.propagateStatusTo(span, tagPropagationLock)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java index 4fc0c27f59a..2922f665b8a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java @@ -289,7 +289,10 @@ private SignalResponse onModuleExecutionResultReceived(ModuleExecutionResult res testsSkipped.add(result.getTestsSkippedTotal()); - SpanUtils.mergeTestFrameworks(span, result.getTestFrameworks()); + synchronized (tagPropagationLock) { + // avoids desync between read and merging + SpanUtils.mergeTestFrameworks(span, result.getTestFrameworks()); + } return AckResponse.INSTANCE; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java index c8a289caa7a..d6902aaec30 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java @@ -54,7 +54,6 @@ public class BuildSystemSessionImpl extends Abstra private final CoverageCalculator.Factory coverageCalculatorFactory; private final T coverageCalculator; private final BuildSessionSettings settings; - private final Object tagPropagationLock = new Object(); public BuildSystemSessionImpl( String projectName, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java index 145a5a25ff8..1d5b7dda8be 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java @@ -183,6 +183,6 @@ public TestSuiteImpl testSuiteStart( coverageStoreFactory, executionResults, capabilities, - SpanUtils.propagateCiVisibilityTagsTo(span)); + SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java index ccafd04a41d..4fb61547b58 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java @@ -82,22 +82,17 @@ public HeadlessTestModule testModuleStart(String moduleName, @Nullable Long star coverageStoreFactory, executionStrategy, capabilities, - this::propagateModuleTags); - } - - private void propagateModuleTags(AgentSpan moduleSpan) { - SpanUtils.propagateCiVisibilityTags(span, moduleSpan); - SpanUtils.propagateTags( - span, - moduleSpan, - Tags.TEST_CODE_COVERAGE_ENABLED, - Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, - Tags.TEST_ITR_TESTS_SKIPPING_TYPE, - Tags.TEST_ITR_TESTS_SKIPPING_COUNT, - Tags.TEST_EARLY_FLAKE_ENABLED, - Tags.TEST_EARLY_FLAKE_ABORT_REASON, - DDTags.CI_ITR_TESTS_SKIPPED, - Tags.TEST_TEST_MANAGEMENT_ENABLED); + SpanUtils.propagateCiVisibilityTagsTo( + span, + tagPropagationLock, + Tags.TEST_CODE_COVERAGE_ENABLED, + Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, + Tags.TEST_ITR_TESTS_SKIPPING_TYPE, + Tags.TEST_ITR_TESTS_SKIPPING_COUNT, + Tags.TEST_EARLY_FLAKE_ENABLED, + Tags.TEST_EARLY_FLAKE_ABORT_REASON, + DDTags.CI_ITR_TESTS_SKIPPED, + Tags.TEST_TEST_MANAGEMENT_ENABLED)); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java index 217409521c7..a89f20345fe 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java @@ -81,6 +81,6 @@ public TestSuiteImpl testSuiteStart( coverageStoreFactory, executionResults, Collections.emptyList(), - SpanUtils.propagateCiVisibilityTagsTo(span)); + SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java index 283e13f513b..3abc8747a34 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java @@ -60,6 +60,6 @@ public ManualApiTestModule testModuleStart(String moduleName, @Nullable Long sta codeowners, linesResolver, coverageStoreFactory, - SpanUtils.propagateCiVisibilityTagsTo(span)); + SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java index 46634a0ec42..b95194d2b13 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java @@ -17,8 +17,23 @@ public class SpanUtils { public static final Consumer DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS = span -> {}; - public static Consumer propagateCiVisibilityTagsTo(AgentSpan parentSpan) { - return childSpan -> propagateCiVisibilityTags(parentSpan, childSpan); + public static Consumer propagateCiVisibilityTagsTo(AgentSpan parentSpan, Object lock, String... additionalTags) { + return childSpan -> { + synchronized (lock) { + propagateCiVisibilityTags(parentSpan, childSpan); + if (additionalTags != null) { + propagateTags(parentSpan, childSpan, additionalTags); + } + } + }; + } + + public static Consumer propagateStatusTo(AgentSpan parentSpan, Object lock) { + return childSpan -> { + synchronized (lock) { + propagateStatus(parentSpan, childSpan); + } + }; } public static void propagateCiVisibilityTags(AgentSpan parentSpan, AgentSpan childSpan) { From 76ee33fa822ced9d48226cc9c3cf39aaacc6118d Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Fri, 1 Aug 2025 16:00:57 +0200 Subject: [PATCH 3/6] style: spotless --- .../civisibility/domain/headless/HeadlessTestSession.java | 1 - .../main/java/datadog/trace/civisibility/utils/SpanUtils.java | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java index 4fb61547b58..5300f3cf320 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java @@ -8,7 +8,6 @@ import datadog.trace.api.civisibility.telemetry.TagValue; import datadog.trace.api.civisibility.telemetry.tag.EarlyFlakeDetectionAbortReason; import datadog.trace.api.civisibility.telemetry.tag.Provider; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.civisibility.Constants; import datadog.trace.civisibility.codeowners.Codeowners; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java index b95194d2b13..bd4abead72f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java @@ -17,7 +17,8 @@ public class SpanUtils { public static final Consumer DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS = span -> {}; - public static Consumer propagateCiVisibilityTagsTo(AgentSpan parentSpan, Object lock, String... additionalTags) { + public static Consumer propagateCiVisibilityTagsTo( + AgentSpan parentSpan, Object lock, String... additionalTags) { return childSpan -> { synchronized (lock) { propagateCiVisibilityTags(parentSpan, childSpan); From 5c3b9676f81fe19017112c88116f6ab6e8096e4d Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 4 Aug 2025 16:35:58 +0200 Subject: [PATCH 4/6] feat: refactor SpanUtils into SpanTagsPropagator --- .../domain/AbstractTestModule.java | 3 +- .../domain/AbstractTestSession.java | 3 +- .../SpanTagsPropagator.java} | 101 ++++++---- .../civisibility/domain/TestSuiteImpl.java | 6 +- .../buildsystem/BuildSystemModuleImpl.java | 6 +- .../buildsystem/BuildSystemSessionImpl.java | 29 ++- .../domain/headless/HeadlessTestModule.java | 3 +- .../domain/headless/HeadlessTestSession.java | 30 +-- .../domain/manualapi/ManualApiTestModule.java | 3 +- .../manualapi/ManualApiTestSession.java | 3 +- .../domain/SpanTagsPropagatorTest.groovy | 186 ++++++++++++++++++ .../civisibility/domain/TestImplTest.groovy | 3 +- .../domain/TestSuiteImplTest.groovy | 3 +- .../civisibility/utils/SpanUtilsTest.groovy | 33 ---- 14 files changed, 287 insertions(+), 125 deletions(-) rename dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/{utils/SpanUtils.java => domain/SpanTagsPropagator.java} (59%) create mode 100644 dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy delete mode 100644 dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java index 841d919167c..6dfc5137337 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java @@ -30,7 +30,7 @@ public abstract class AbstractTestModule { protected final Codeowners codeowners; protected final LinesResolver linesResolver; private final Consumer onSpanFinish; - protected final Object tagPropagationLock = new Object(); + protected final SpanTagsPropagator tagsPropagator; public AbstractTestModule( AgentSpanContext sessionSpanContext, @@ -64,6 +64,7 @@ public AbstractTestModule( } span = spanBuilder.start(); + tagsPropagator = new SpanTagsPropagator(span); span.setSpanType(InternalSpanTypes.TEST_MODULE_END); span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_MODULE); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java index 7278af1cac7..15c3979104f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java @@ -46,7 +46,7 @@ public abstract class AbstractTestSession { protected final SourcePathResolver sourcePathResolver; protected final Codeowners codeowners; protected final LinesResolver linesResolver; - protected final Object tagPropagationLock = new Object(); + protected final SpanTagsPropagator tagPropagator; public AbstractTestSession( String projectName, @@ -94,6 +94,7 @@ public AbstractTestSession( } span = spanBuilder.start(); + tagPropagator = new SpanTagsPropagator(span); span.setSpanType(InternalSpanTypes.TEST_SESSION_END); span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SESSION); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/SpanTagsPropagator.java similarity index 59% rename from dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java rename to dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/SpanTagsPropagator.java index bd4abead72f..d5c4c002aed 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/SpanTagsPropagator.java @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.utils; +package datadog.trace.civisibility.domain; import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -14,38 +14,45 @@ import java.util.function.BinaryOperator; import java.util.function.Consumer; -public class SpanUtils { - public static final Consumer DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS = span -> {}; +public class SpanTagsPropagator { + public static final Consumer NOOP_PROPAGATOR = span -> {}; - public static Consumer propagateCiVisibilityTagsTo( - AgentSpan parentSpan, Object lock, String... additionalTags) { - return childSpan -> { - synchronized (lock) { - propagateCiVisibilityTags(parentSpan, childSpan); - if (additionalTags != null) { - propagateTags(parentSpan, childSpan, additionalTags); - } - } - }; + private final AgentSpan parentSpan; + private final Object tagPropagationLock = new Object(); + + public SpanTagsPropagator(AgentSpan parentSpan) { + this.parentSpan = parentSpan; } - public static Consumer propagateStatusTo(AgentSpan parentSpan, Object lock) { - return childSpan -> { - synchronized (lock) { - propagateStatus(parentSpan, childSpan); - } - }; + public void propagateCiVisibilityTags(AgentSpan childSpan) { + mergeTestFrameworks(getFrameworks(childSpan)); + propagateStatus(childSpan); } - public static void propagateCiVisibilityTags(AgentSpan parentSpan, AgentSpan childSpan) { - mergeTestFrameworks(parentSpan, getFrameworks(childSpan)); - propagateStatus(parentSpan, childSpan); + public void propagateStatus(AgentSpan childSpan) { + synchronized (tagPropagationLock) { + unsafePropagateStatus(childSpan); + } } - public static void mergeTestFrameworks(AgentSpan span, Collection testFrameworks) { - Collection spanFrameworks = getFrameworks(span); - Collection merged = merge(spanFrameworks, testFrameworks); - setFrameworks(span, merged); + public void mergeTestFrameworks(Collection testFrameworks) { + synchronized (tagPropagationLock) { + unsafeMergeTestFrameworks(testFrameworks); + } + } + + public void propagateTags(AgentSpan childSpan, TagMergeSpec... specs) { + synchronized (tagPropagationLock) { + for (TagMergeSpec spec : specs) { + unsafePropagateTag(childSpan, spec); + } + } + } + + private void unsafeMergeTestFrameworks(Collection childFrameworks) { + Collection parentFrameworks = getFrameworks(parentSpan); + Collection merged = merge(parentFrameworks, childFrameworks); + setFrameworks(merged); } static Collection getFrameworks(AgentSpan span) { @@ -64,7 +71,8 @@ static Collection getFrameworks(AgentSpan span) { Iterator versions = versionTag != null ? ((Collection) versionTag).iterator() : null; while (names.hasNext()) { - frameworks.add(new TestFramework(names.next(), versions != null ? versions.next() : null)); + String version = (versions != null && versions.hasNext()) ? versions.next() : null; + frameworks.add(new TestFramework(names.next(), version)); } } else { @@ -96,7 +104,7 @@ private static Collection merge( return merged; } - private static void setFrameworks(AgentSpan span, Collection frameworks) { + private void setFrameworks(Collection frameworks) { if (frameworks.isEmpty()) { return; } @@ -107,7 +115,7 @@ private static void setFrameworks(AgentSpan span, Collection fram if (framework.getVersion() != null) { tags.put(Tags.TEST_FRAMEWORK_VERSION, framework.getVersion()); } - span.setAllTags(tags); + parentSpan.setAllTags(tags); return; } Collection names = new ArrayList<>(frameworks.size()); @@ -119,10 +127,10 @@ private static void setFrameworks(AgentSpan span, Collection fram Map> tags = new HashMap<>(); tags.put(Tags.TEST_FRAMEWORK, names); tags.put(Tags.TEST_FRAMEWORK_VERSION, versions); - span.setAllTags(tags); + parentSpan.setAllTags(tags); } - private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) { + private void unsafePropagateStatus(AgentSpan childSpan) { TestStatus childStatus = (TestStatus) childSpan.getTag(Tags.TEST_STATUS); if (childStatus == null) { return; @@ -148,25 +156,32 @@ private static void propagateStatus(AgentSpan parentSpan, AgentSpan childSpan) { } } - public static void propagateTags(AgentSpan parentSpan, AgentSpan childSpan, String... tagNames) { - for (String tagName : tagNames) { - parentSpan.setTag(tagName, childSpan.getTag(tagName)); + public static class TagMergeSpec { + private final String tagKey; + private final BinaryOperator mergeFunction; + + TagMergeSpec(String tagKey, BinaryOperator mergeFunction) { + this.tagKey = tagKey; + this.mergeFunction = mergeFunction; + } + + public static TagMergeSpec of(String key, BinaryOperator mergeFunction) { + return new TagMergeSpec<>(key, mergeFunction); } - } - public static void propagateTag(AgentSpan parentSpan, AgentSpan childSpan, String tagName) { - propagateTag(parentSpan, childSpan, tagName, (p, c) -> c); + public static TagMergeSpec of(String tagKey) { + return new TagMergeSpec<>(tagKey, (parent, child) -> child); + } } - public static void propagateTag( - AgentSpan parentSpan, AgentSpan childSpan, String tagName, BinaryOperator mergeStrategy) { - T childTag = (T) childSpan.getTag(tagName); + private void unsafePropagateTag(AgentSpan childSpan, TagMergeSpec spec) { + T childTag = (T) childSpan.getTag(spec.tagKey); if (childTag != null) { - T parentTag = (T) parentSpan.getTag(tagName); + T parentTag = (T) parentSpan.getTag(spec.tagKey); if (parentTag == null) { - parentSpan.setTag(tagName, childTag); + parentSpan.setTag(spec.tagKey, childTag); } else { - parentSpan.setTag(tagName, mergeStrategy.apply(parentTag, childTag)); + parentSpan.setTag(spec.tagKey, spec.mergeFunction.apply(parentTag, childTag)); } } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index a3c7acc1daa..c29fa175229 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -24,7 +24,6 @@ import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.test.ExecutionResults; -import datadog.trace.civisibility.utils.SpanUtils; import java.lang.reflect.Method; import java.util.Collection; import java.util.function.Consumer; @@ -56,7 +55,7 @@ public class TestSuiteImpl implements DDTestSuite { private final boolean parallelized; private final Collection capabilities; private final Consumer onSpanFinish; - private final Object tagPropagationLock = new Object(); + private final SpanTagsPropagator tagsPropagator; public TestSuiteImpl( AgentSpanContext moduleSpanContext, @@ -107,6 +106,7 @@ public TestSuiteImpl( } span = spanBuilder.start(); + tagsPropagator = new SpanTagsPropagator(span); span.setSpanType(InternalSpanTypes.TEST_SUITE_END); span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST_SUITE); @@ -265,6 +265,6 @@ public TestImpl testStart( coverageStoreFactory, executionResults, capabilities, - SpanUtils.propagateStatusTo(span, tagPropagationLock)); + tagsPropagator::propagateStatus); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java index 2922f665b8a..525f82baf50 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemModuleImpl.java @@ -30,7 +30,6 @@ import datadog.trace.civisibility.ipc.SignalType; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.utils.SpanUtils; import datadog.trace.util.Strings; import java.net.InetSocketAddress; import java.nio.file.Path; @@ -289,10 +288,7 @@ private SignalResponse onModuleExecutionResultReceived(ModuleExecutionResult res testsSkipped.add(result.getTestsSkippedTotal()); - synchronized (tagPropagationLock) { - // avoids desync between read and merging - SpanUtils.mergeTestFrameworks(span, result.getTestFrameworks()); - } + tagsPropagator.mergeTestFrameworks(result.getTestFrameworks()); return AckResponse.INSTANCE; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java index d6902aaec30..330d2aa8167 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/BuildSystemSessionImpl.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.domain.buildsystem; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.civisibility.domain.SpanTagsPropagator.TagMergeSpec; import datadog.trace.api.Config; import datadog.trace.api.DDTags; @@ -35,7 +36,6 @@ import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.source.index.RepoIndex; import datadog.trace.civisibility.source.index.RepoIndexProvider; -import datadog.trace.civisibility.utils.SpanUtils; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; @@ -182,22 +182,17 @@ public AgentSpan testTaskStart(String taskName) { private void onModuleFinish(AgentSpan moduleSpan) { // multiple modules can finish in parallel - synchronized (tagPropagationLock) { - SpanUtils.propagateCiVisibilityTags(span, moduleSpan); - - SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ENABLED, Boolean::logicalOr); - SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_EARLY_FLAKE_ABORT_REASON); - - SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_CODE_COVERAGE_ENABLED, Boolean::logicalOr); - SpanUtils.propagateTag( - span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, Boolean::logicalOr); - SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_TYPE); - SpanUtils.propagateTag(span, moduleSpan, Tags.TEST_ITR_TESTS_SKIPPING_COUNT, Long::sum); - SpanUtils.propagateTag(span, moduleSpan, DDTags.CI_ITR_TESTS_SKIPPED, Boolean::logicalOr); - - SpanUtils.propagateTag( - span, moduleSpan, Tags.TEST_TEST_MANAGEMENT_ENABLED, Boolean::logicalOr); - } + tagPropagator.propagateCiVisibilityTags(moduleSpan); + tagPropagator.propagateTags( + moduleSpan, + TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ENABLED, Boolean::logicalOr), + TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ABORT_REASON), + TagMergeSpec.of(Tags.TEST_CODE_COVERAGE_ENABLED, Boolean::logicalOr), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, Boolean::logicalOr), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_TYPE), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_COUNT, Long::sum), + TagMergeSpec.of(DDTags.CI_ITR_TESTS_SKIPPED, Boolean::logicalOr), + TagMergeSpec.of(Tags.TEST_TEST_MANAGEMENT_ENABLED, Boolean::logicalOr)); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java index 1d5b7dda8be..a338c36f0af 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java @@ -27,7 +27,6 @@ import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.test.ExecutionResults; import datadog.trace.civisibility.test.ExecutionStrategy; -import datadog.trace.civisibility.utils.SpanUtils; import java.util.Collection; import java.util.function.Consumer; import javax.annotation.Nonnull; @@ -183,6 +182,6 @@ public TestSuiteImpl testSuiteStart( coverageStoreFactory, executionResults, capabilities, - SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); + tagsPropagator::propagateCiVisibilityTags); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java index 5300f3cf320..926fdbba4e9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestSession.java @@ -1,5 +1,7 @@ package datadog.trace.civisibility.domain.headless; +import static datadog.trace.civisibility.domain.SpanTagsPropagator.TagMergeSpec; + import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.api.civisibility.config.LibraryCapability; @@ -8,6 +10,7 @@ import datadog.trace.api.civisibility.telemetry.TagValue; import datadog.trace.api.civisibility.telemetry.tag.EarlyFlakeDetectionAbortReason; import datadog.trace.api.civisibility.telemetry.tag.Provider; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.civisibility.Constants; import datadog.trace.civisibility.codeowners.Codeowners; @@ -18,7 +21,6 @@ import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.test.ExecutionStrategy; -import datadog.trace.civisibility.utils.SpanUtils; import java.util.Collection; import java.util.Collections; import javax.annotation.Nonnull; @@ -81,17 +83,21 @@ public HeadlessTestModule testModuleStart(String moduleName, @Nullable Long star coverageStoreFactory, executionStrategy, capabilities, - SpanUtils.propagateCiVisibilityTagsTo( - span, - tagPropagationLock, - Tags.TEST_CODE_COVERAGE_ENABLED, - Tags.TEST_ITR_TESTS_SKIPPING_ENABLED, - Tags.TEST_ITR_TESTS_SKIPPING_TYPE, - Tags.TEST_ITR_TESTS_SKIPPING_COUNT, - Tags.TEST_EARLY_FLAKE_ENABLED, - Tags.TEST_EARLY_FLAKE_ABORT_REASON, - DDTags.CI_ITR_TESTS_SKIPPED, - Tags.TEST_TEST_MANAGEMENT_ENABLED)); + this::onModuleFinish); + } + + private void onModuleFinish(AgentSpan moduleSpan) { + tagPropagator.propagateCiVisibilityTags(moduleSpan); + tagPropagator.propagateTags( + moduleSpan, + TagMergeSpec.of(Tags.TEST_CODE_COVERAGE_ENABLED), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_ENABLED), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_TYPE), + TagMergeSpec.of(Tags.TEST_ITR_TESTS_SKIPPING_COUNT), + TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ENABLED), + TagMergeSpec.of(Tags.TEST_EARLY_FLAKE_ABORT_REASON), + TagMergeSpec.of(DDTags.CI_ITR_TESTS_SKIPPED), + TagMergeSpec.of(Tags.TEST_TEST_MANAGEMENT_ENABLED)); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java index a89f20345fe..22dbe30994a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestModule.java @@ -15,7 +15,6 @@ import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.test.ExecutionResults; -import datadog.trace.civisibility.utils.SpanUtils; import java.util.Collections; import java.util.function.Consumer; import javax.annotation.Nullable; @@ -81,6 +80,6 @@ public TestSuiteImpl testSuiteStart( coverageStoreFactory, executionResults, Collections.emptyList(), - SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); + tagsPropagator::propagateCiVisibilityTags); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java index 3abc8747a34..917bf9ddbc0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/manualapi/ManualApiTestSession.java @@ -11,7 +11,6 @@ import datadog.trace.civisibility.domain.InstrumentationType; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.utils.SpanUtils; import javax.annotation.Nullable; /** @@ -60,6 +59,6 @@ public ManualApiTestModule testModuleStart(String moduleName, @Nullable Long sta codeowners, linesResolver, coverageStoreFactory, - SpanUtils.propagateCiVisibilityTagsTo(span, tagPropagationLock)); + tagPropagator::propagateCiVisibilityTags); } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy new file mode 100644 index 00000000000..8532c374b47 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy @@ -0,0 +1,186 @@ +package datadog.trace.civisibility.domain + +import static datadog.trace.civisibility.domain.SpanTagsPropagator.TagMergeSpec + +import datadog.trace.api.civisibility.execution.TestStatus +import datadog.trace.bootstrap.instrumentation.api.Tags +import datadog.trace.civisibility.ipc.TestFramework +import datadog.trace.core.DDSpan +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import spock.lang.Specification + +class SpanTagsPropagatorTest extends Specification { + def "test getFrameworks"() { + when: + def span = createSpanWithFrameworks(frameworkTag, frameworkVersionTag) + def frameworks = SpanTagsPropagator.getFrameworks(span) + + then: + frameworks == expected + + where: + frameworkTag | frameworkVersionTag | expected + "name" | "version" | [new TestFramework("name", "version")] + "name" | null | [new TestFramework("name", null)] + null | "version" | [] + ["nameA", "nameB"] | ["versionA", "versionB"] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", "versionB")] + ["nameA", "nameB"] | null | [new TestFramework("nameA", null), new TestFramework("nameB", null)] + ["nameA", "nameB"] | ["versionA", null] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", null)] + } + + DDSpan createSpanWithFrameworks(Object frameworkTag, Object frameworkVersionTag) { + def span = Stub(DDSpan) + span.getTag(Tags.TEST_FRAMEWORK) >> frameworkTag + span.getTag(Tags.TEST_FRAMEWORK_VERSION) >> frameworkVersionTag + return span + } + + def "test status propagation: #childStatus to #parentStatus"() { + given: + def parentSpan = Mock(DDSpan) + parentSpan.getTag(Tags.TEST_STATUS) >> parentStatus + + def childSpan = Mock(DDSpan) + childSpan.getTag(Tags.TEST_STATUS) >> childStatus + + def propagator = new SpanTagsPropagator(parentSpan) + + when: + propagator.propagateStatus(childSpan) + + then: + if (expectedStatus != null) { + 1 * parentSpan.setTag(Tags.TEST_STATUS, expectedStatus) + } else { + 0 * parentSpan.setTag(Tags.TEST_STATUS, _) + } + + where: + childStatus | parentStatus | expectedStatus + TestStatus.pass | null | TestStatus.pass + TestStatus.pass | TestStatus.skip | TestStatus.pass + TestStatus.pass | TestStatus.pass | null // no change + TestStatus.pass | TestStatus.fail | null // no change + TestStatus.fail | null | TestStatus.fail + TestStatus.fail | TestStatus.pass | TestStatus.fail + TestStatus.fail | TestStatus.skip | TestStatus.fail + TestStatus.fail | TestStatus.fail | TestStatus.fail + TestStatus.skip | null | TestStatus.skip + TestStatus.skip | TestStatus.pass | null // no change + TestStatus.skip | TestStatus.fail | null // no change + TestStatus.skip | TestStatus.skip | null // no change + null | TestStatus.pass | null // no change + } + + def "test framework merging: #childFrameworks and #parentFrameworks"() { + given: + def parentSpan = Mock(DDSpan) + parentSpan.getTag(Tags.TEST_FRAMEWORK) >> parentFrameworks.collect(it -> it.getName()) + parentSpan.getTag(Tags.TEST_FRAMEWORK_VERSION) >> parentFrameworks.collect(it -> it.getVersion()) + + def propagator = new SpanTagsPropagator(parentSpan) + + def expectedNames = expectedFrameworks.collect(it -> it.getName()) + def expectedVersions = expectedFrameworks.collect(it -> it.getVersion()) + + when: + propagator.mergeTestFrameworks(childFrameworks) + + then: + 1 * parentSpan.setAllTags([ + (Tags.TEST_FRAMEWORK) : expectedNames, + (Tags.TEST_FRAMEWORK_VERSION): expectedVersions + ]) + + where: + childFrameworks | parentFrameworks | expectedFrameworks + [] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] + [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] | [] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] + [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] | [new TestFramework("Spock", "2.3")] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("Spock", "2.3"), + new TestFramework("TestNG", "7.4.0")] + } + + def "test tag propagation: #childValue and #parentValue with spec #tagSpec"() { + given: + def parentSpan = Mock(DDSpan) + parentSpan.getTag("tag") >> parentValue + + def childSpan = Mock(DDSpan) + childSpan.getTag("tag") >> childValue + + def propagator = new SpanTagsPropagator(parentSpan) + + when: + propagator.propagateTags(childSpan, tagSpec) + + then: + if (expectedChange) { + 1 * parentSpan.setTag("tag", expectedValue) + } else { + 0 * parentSpan.setTag("tag", _) + } + + where: + tagSpec | childValue | parentValue | expectedChange | expectedValue + TagMergeSpec.of("tag") | "a" | "b" | true | "a" + TagMergeSpec.of("tag") | null | "b" | false | "b" + TagMergeSpec.of("tag") | null | null | false | null + TagMergeSpec.of("tag", Boolean::logicalOr) | true | false | true | true + TagMergeSpec.of("tag", Boolean::logicalOr) | false | false | true | false + } + + def "test synchronized propagation"() { + given: + def parentSpan = Mock(DDSpan) + def propagator = new SpanTagsPropagator(parentSpan) + def numThreads = 9 + def latch = new CountDownLatch(numThreads) + def executor = Executors.newFixedThreadPool(numThreads) + def exceptions = Collections.synchronizedList([]) + + when: + numThreads.times { i -> + executor.submit { + try { + switch (i % 3) { + case 0: + def childSpan = Mock(DDSpan) + childSpan.getTag(Tags.TEST_STATUS) >> TestStatus.fail + propagator.propagateStatus(childSpan) + break + case 1: + def frameworks = [new TestFramework("JUnit${i}", "5.${i}")] + propagator.mergeTestFrameworks(frameworks) + break + case 2: + def childSpan = Mock(DDSpan) + childSpan.getTag("custom.tag.${i}") >> "value${i}" + propagator.propagateTags(childSpan, TagMergeSpec.of("custom.tag.${i}")) + break + } + } catch (Exception e) { + exceptions.add(e) + } finally { + latch.countDown() + } + } + } + + latch.await(5, TimeUnit.SECONDS) + executor.shutdown() + + then: + exceptions.isEmpty() + 3 * parentSpan.setTag(Tags.TEST_STATUS, TestStatus.fail) + 3 * parentSpan.setAllTags(_) + 3 * parentSpan.setTag({ it.startsWith("custom.tag.") }, { it.startsWith("value") }) + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy index eac92fa8474..56b1a8ab640 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestImplTest.groovy @@ -18,7 +18,6 @@ import datadog.trace.civisibility.source.LinesResolver import datadog.trace.civisibility.source.NoOpSourcePathResolver import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl import datadog.trace.civisibility.test.ExecutionResults -import datadog.trace.civisibility.utils.SpanUtils class TestImplTest extends SpanWriterTest { def "test span is generated and tags populated"() { @@ -143,7 +142,7 @@ class TestImplTest extends SpanWriterTest { coverageStoreFactory, executionResults, libraryCapabilities, - SpanUtils.DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS + SpanTagsPropagator.NOOP_PROPAGATOR ) } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy index aed8ffa88db..c7819669a85 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy @@ -15,7 +15,6 @@ import datadog.trace.civisibility.source.LinesResolver import datadog.trace.civisibility.source.SourcePathResolver import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl import datadog.trace.civisibility.test.ExecutionResults -import datadog.trace.civisibility.utils.SpanUtils class TestSuiteImplTest extends SpanWriterTest { def "test suite span is generated and tags populated"() { @@ -86,7 +85,7 @@ class TestSuiteImplTest extends SpanWriterTest { coverageStoreFactory, executionResults, [], - SpanUtils.DO_NOT_PROPAGATE_CI_VISIBILITY_TAGS + SpanTagsPropagator.NOOP_PROPAGATOR ) } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy deleted file mode 100644 index 18508743281..00000000000 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/utils/SpanUtilsTest.groovy +++ /dev/null @@ -1,33 +0,0 @@ -package datadog.trace.civisibility.utils - -import datadog.trace.bootstrap.instrumentation.api.Tags -import datadog.trace.civisibility.ipc.TestFramework -import datadog.trace.core.DDSpan -import spock.lang.Specification - -class SpanUtilsTest extends Specification { - def "test getFrameworks"() { - when: - def span = createSpanWithFrameworks(frameworkTag, frameworkVersionTag) - def frameworks = SpanUtils.getFrameworks(span) - - then: - frameworks == expected - - where: - frameworkTag | frameworkVersionTag | expected - "name" | "version" | [new TestFramework("name", "version")] - "name" | null | [new TestFramework("name", null)] - null | "version" | [] - ["nameA", "nameB"] | ["versionA", "versionB"] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", "versionB")] - ["nameA", "nameB"] | null | [new TestFramework("nameA", null), new TestFramework("nameB", null)] - ["nameA", "nameB"] | ["versionA", null] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", null)] - } - - DDSpan createSpanWithFrameworks(Object frameworkTag, Object frameworkVersionTag) { - def span = Stub(DDSpan) - span.getTag(Tags.TEST_FRAMEWORK) >> frameworkTag - span.getTag(Tags.TEST_FRAMEWORK_VERSION) >> frameworkVersionTag - return span - } -} From c6fa8be9a925ce6e6f5e749acacde99bcf994d60 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 4 Aug 2025 16:42:19 +0200 Subject: [PATCH 5/6] fix: cleanup test --- .../domain/SpanTagsPropagatorTest.groovy | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy index 8532c374b47..8093450bc97 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy @@ -14,7 +14,10 @@ import spock.lang.Specification class SpanTagsPropagatorTest extends Specification { def "test getFrameworks"() { when: - def span = createSpanWithFrameworks(frameworkTag, frameworkVersionTag) + def span = Stub(DDSpan) + span.getTag(Tags.TEST_FRAMEWORK) >> frameworkTag + span.getTag(Tags.TEST_FRAMEWORK_VERSION) >> frameworkVersionTag + def frameworks = SpanTagsPropagator.getFrameworks(span) then: @@ -30,13 +33,6 @@ class SpanTagsPropagatorTest extends Specification { ["nameA", "nameB"] | ["versionA", null] | [new TestFramework("nameA", "versionA"), new TestFramework("nameB", null)] } - DDSpan createSpanWithFrameworks(Object frameworkTag, Object frameworkVersionTag) { - def span = Stub(DDSpan) - span.getTag(Tags.TEST_FRAMEWORK) >> frameworkTag - span.getTag(Tags.TEST_FRAMEWORK_VERSION) >> frameworkVersionTag - return span - } - def "test status propagation: #childStatus to #parentStatus"() { given: def parentSpan = Mock(DDSpan) @@ -95,17 +91,17 @@ class SpanTagsPropagatorTest extends Specification { ]) where: - childFrameworks | parentFrameworks | expectedFrameworks + childFrameworks | parentFrameworks | expectedFrameworks [] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] + new TestFramework("TestNG", "7.4.0")] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] + new TestFramework("TestNG", "7.4.0")] | [] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("TestNG", "7.4.0")] [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [new TestFramework("Spock", "2.3")] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("Spock", "2.3"), - new TestFramework("TestNG", "7.4.0")] + new TestFramework("TestNG", "7.4.0")] | [new TestFramework("Spock", "2.3")] | [new TestFramework("JUnit", "5.8.0"), + new TestFramework("Spock", "2.3"), + new TestFramework("TestNG", "7.4.0")] } def "test tag propagation: #childValue and #parentValue with spec #tagSpec"() { @@ -152,19 +148,19 @@ class SpanTagsPropagatorTest extends Specification { try { switch (i % 3) { case 0: - def childSpan = Mock(DDSpan) - childSpan.getTag(Tags.TEST_STATUS) >> TestStatus.fail - propagator.propagateStatus(childSpan) - break + def childSpan = Mock(DDSpan) + childSpan.getTag(Tags.TEST_STATUS) >> TestStatus.fail + propagator.propagateStatus(childSpan) + break case 1: - def frameworks = [new TestFramework("JUnit${i}", "5.${i}")] - propagator.mergeTestFrameworks(frameworks) - break + def frameworks = [new TestFramework("JUnit${i}", "5.${i}")] + propagator.mergeTestFrameworks(frameworks) + break case 2: - def childSpan = Mock(DDSpan) - childSpan.getTag("custom.tag.${i}") >> "value${i}" - propagator.propagateTags(childSpan, TagMergeSpec.of("custom.tag.${i}")) - break + def childSpan = Mock(DDSpan) + childSpan.getTag("custom.tag.${i}") >> "value${i}" + propagator.propagateTags(childSpan, TagMergeSpec.of("custom.tag.${i}")) + break } } catch (Exception e) { exceptions.add(e) From d738f842d8d6af25bc2d11f354f8f6a107b2f7a1 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 4 Aug 2025 16:56:23 +0200 Subject: [PATCH 6/6] fix: style --- .../domain/SpanTagsPropagatorTest.groovy | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy index 8093450bc97..f55cc33a753 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/SpanTagsPropagatorTest.groovy @@ -91,17 +91,10 @@ class SpanTagsPropagatorTest extends Specification { ]) where: - childFrameworks | parentFrameworks | expectedFrameworks - [] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] - [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] - [new TestFramework("JUnit", "5.8.0"), - new TestFramework("TestNG", "7.4.0")] | [new TestFramework("Spock", "2.3")] | [new TestFramework("JUnit", "5.8.0"), - new TestFramework("Spock", "2.3"), - new TestFramework("TestNG", "7.4.0")] + childFrameworks | parentFrameworks | expectedFrameworks + [] | [new TestFramework("JUnit", "5.8.0"), new TestFramework("TestNG", "7.4.0")] | [new TestFramework("JUnit", "5.8.0"), new TestFramework("TestNG", "7.4.0")] + [new TestFramework("JUnit", "5.8.0"), new TestFramework("TestNG", "7.4.0")] | [] | [new TestFramework("JUnit", "5.8.0"), new TestFramework("TestNG", "7.4.0")] + [new TestFramework("JUnit", "5.8.0"), new TestFramework("TestNG", "7.4.0")] | [new TestFramework("Spock", "2.3")] | [new TestFramework("JUnit", "5.8.0"), new TestFramework("Spock", "2.3"), new TestFramework("TestNG", "7.4.0")] } def "test tag propagation: #childValue and #parentValue with spec #tagSpec"() { @@ -148,19 +141,19 @@ class SpanTagsPropagatorTest extends Specification { try { switch (i % 3) { case 0: - def childSpan = Mock(DDSpan) - childSpan.getTag(Tags.TEST_STATUS) >> TestStatus.fail - propagator.propagateStatus(childSpan) - break + def childSpan = Mock(DDSpan) + childSpan.getTag(Tags.TEST_STATUS) >> TestStatus.fail + propagator.propagateStatus(childSpan) + break case 1: - def frameworks = [new TestFramework("JUnit${i}", "5.${i}")] - propagator.mergeTestFrameworks(frameworks) - break + def frameworks = [new TestFramework("JUnit${i}", "5.${i}")] + propagator.mergeTestFrameworks(frameworks) + break case 2: - def childSpan = Mock(DDSpan) - childSpan.getTag("custom.tag.${i}") >> "value${i}" - propagator.propagateTags(childSpan, TagMergeSpec.of("custom.tag.${i}")) - break + def childSpan = Mock(DDSpan) + childSpan.getTag("custom.tag.${i}") >> "value${i}" + propagator.propagateTags(childSpan, TagMergeSpec.of("custom.tag.${i}")) + break } } catch (Exception e) { exceptions.add(e)