-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add capabilities tagging #8499
Add capabilities tagging #8499
Changes from 34 commits
8e9bb96
7ab85a0
b8c748f
45735e6
ee63fa7
8ec555d
04a8084
408a7d3
7d28e5a
cfd4fda
7877125
5a796b9
9a320be
02c6bcc
568f709
2b25ea8
ef2664d
3538287
a52cc21
bc0d0b3
c9a0114
8129852
103a089
6685a5b
79df146
063b82a
c0cf509
e12b7c8
6792874
2bee4c2
bc8ab0c
bf6d237
b073736
0de9f87
877fc63
8f6e0d7
2e9d18c
4238c3b
0dcea22
c31a7d7
f7aa755
28c2310
bd09eef
02fa357
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package datadog.trace.civisibility.config; | ||
|
||
import datadog.trace.api.DDTags; | ||
import datadog.trace.api.civisibility.config.LibraryCapability; | ||
import java.util.Collection; | ||
import java.util.EnumMap; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
public abstract class LibraryCapabilityUtils { | ||
|
||
public static final Map<LibraryCapability, String> CAPABILITY_TAG_MAP = capabilitiesTagMap(); | ||
|
||
private static Map<LibraryCapability, String> capabilitiesTagMap() { | ||
Map<LibraryCapability, String> capabilitiesTags = new EnumMap<>(LibraryCapability.class); | ||
capabilitiesTags.put(LibraryCapability.TIA, DDTags.LIBRARY_CAPABILITIES_TIA); | ||
capabilitiesTags.put(LibraryCapability.EFD, DDTags.LIBRARY_CAPABILITIES_EFD); | ||
capabilitiesTags.put(LibraryCapability.ATR, DDTags.LIBRARY_CAPABILITIES_ATR); | ||
capabilitiesTags.put(LibraryCapability.IMPACTED, DDTags.LIBRARY_CAPABILITIES_IMPACTED_TESTS); | ||
capabilitiesTags.put( | ||
LibraryCapability.FAIL_FAST, DDTags.LIBRARY_CAPABILITIES_FAIL_FAST_TEST_ORDER); | ||
capabilitiesTags.put(LibraryCapability.QUARANTINE, DDTags.LIBRARY_CAPABILITIES_QUARANTINE); | ||
capabilitiesTags.put(LibraryCapability.DISABLED, DDTags.LIBRARY_CAPABILITIES_DISABLED); | ||
capabilitiesTags.put( | ||
LibraryCapability.ATTEMPT_TO_FIX, DDTags.LIBRARY_CAPABILITIES_ATTEMPT_TO_FIX); | ||
return capabilitiesTags; | ||
} | ||
|
||
public static Map<LibraryCapability, Boolean> filterCapabilities( | ||
Collection<LibraryCapability> available, Map<LibraryCapability, Boolean> capabilitiesStatus) { | ||
Map<LibraryCapability, Boolean> capabilities = new HashMap<>(capabilitiesStatus); | ||
capabilities.keySet().retainAll(available); | ||
return capabilities; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
import datadog.trace.api.civisibility.CIConstants; | ||
import datadog.trace.api.civisibility.DDTest; | ||
import datadog.trace.api.civisibility.InstrumentationTestBridge; | ||
import datadog.trace.api.civisibility.config.LibraryCapability; | ||
import datadog.trace.api.civisibility.config.TestIdentifier; | ||
import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; | ||
import datadog.trace.api.civisibility.coverage.CoverageStore; | ||
|
@@ -37,6 +38,7 @@ | |
import datadog.trace.bootstrap.instrumentation.api.TagContext; | ||
import datadog.trace.bootstrap.instrumentation.api.Tags; | ||
import datadog.trace.civisibility.codeowners.Codeowners; | ||
import datadog.trace.civisibility.config.LibraryCapabilityUtils; | ||
import datadog.trace.civisibility.decorator.TestDecorator; | ||
import datadog.trace.civisibility.source.LinesResolver; | ||
import datadog.trace.civisibility.source.SourcePathResolver; | ||
|
@@ -45,6 +47,7 @@ | |
import java.lang.reflect.Method; | ||
import java.util.Collection; | ||
import java.util.Collections; | ||
import java.util.Map; | ||
import java.util.function.Consumer; | ||
import javax.annotation.Nonnull; | ||
import javax.annotation.Nullable; | ||
|
@@ -86,6 +89,7 @@ public TestImpl( | |
Codeowners codeowners, | ||
CoverageStore.Factory coverageStoreFactory, | ||
ExecutionResults executionResults, | ||
Map<LibraryCapability, Boolean> libraryCapabilities, | ||
Consumer<AgentSpan> onSpanFinish) { | ||
this.instrumentation = instrumentation; | ||
this.metricCollector = metricCollector; | ||
|
@@ -144,6 +148,11 @@ public TestImpl( | |
span.setTag(Tags.ITR_CORRELATION_ID, itrCorrelationId); | ||
} | ||
|
||
for (Map.Entry<LibraryCapability, Boolean> entry : libraryCapabilities.entrySet()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understood correctly, we might end up with a |
||
String capabilityTag = LibraryCapabilityUtils.CAPABILITY_TAG_MAP.get(entry.getKey()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than doing a map lookup, can't we store the tag as a field in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I separated the |
||
span.setTag(capabilityTag, entry.getValue()); | ||
} | ||
|
||
testDecorator.afterStart(span); | ||
|
||
metricCollector.add(CiVisibilityCountMetric.EVENT_CREATED, 1, instrumentation, EventType.TEST); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
|
||
import datadog.trace.api.Config; | ||
import datadog.trace.api.civisibility.DDTestSuite; | ||
import datadog.trace.api.civisibility.config.LibraryCapability; | ||
import datadog.trace.api.civisibility.coverage.CoverageStore; | ||
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; | ||
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; | ||
|
@@ -26,6 +27,7 @@ | |
import datadog.trace.civisibility.utils.SpanUtils; | ||
import java.lang.reflect.Method; | ||
import java.util.Collection; | ||
import java.util.Map; | ||
import java.util.function.Consumer; | ||
import javax.annotation.Nullable; | ||
import org.slf4j.Logger; | ||
|
@@ -52,6 +54,7 @@ public class TestSuiteImpl implements DDTestSuite { | |
private final CoverageStore.Factory coverageStoreFactory; | ||
private final ExecutionResults executionResults; | ||
private final boolean parallelized; | ||
private final Map<LibraryCapability, Boolean> libraryCapabilities; | ||
private final Consumer<AgentSpan> onSpanFinish; | ||
|
||
public TestSuiteImpl( | ||
|
@@ -72,6 +75,7 @@ public TestSuiteImpl( | |
LinesResolver linesResolver, | ||
CoverageStore.Factory coverageStoreFactory, | ||
ExecutionResults executionResults, | ||
Map<LibraryCapability, Boolean> libraryCapabilities, | ||
Consumer<AgentSpan> onSpanFinish) { | ||
this.moduleSpanContext = moduleSpanContext; | ||
this.moduleName = moduleName; | ||
|
@@ -88,6 +92,7 @@ public TestSuiteImpl( | |
this.linesResolver = linesResolver; | ||
this.coverageStoreFactory = coverageStoreFactory; | ||
this.executionResults = executionResults; | ||
this.libraryCapabilities = libraryCapabilities; | ||
this.onSpanFinish = onSpanFinish; | ||
|
||
AgentTracer.SpanBuilder spanBuilder = | ||
|
@@ -259,6 +264,12 @@ public TestImpl testStart( | |
codeowners, | ||
coverageStoreFactory, | ||
executionResults, | ||
SpanUtils.propagateCiVisibilityTagsTo(span)); | ||
libraryCapabilities, | ||
this::propagateTestTags); | ||
} | ||
|
||
private void propagateTestTags(AgentSpan testSpan) { | ||
SpanUtils.propagateCiVisibilityTags(span, testSpan); | ||
SpanUtils.propagateLibraryCapabilities(span, testSpan); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the other hand, if capabilities are already available in session, module, and suite, why do we propagate them from a child span rather than setting them directly in constructor along with the other tags? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed offline, let's remove the propagation altogether, as the spec only mandates that the capabilities be present in the test events |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that a capability is available by definition :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very true 👍