Skip to content

Incorporating TagMap into the tracer #8589

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

Open
wants to merge 90 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
90 commits
Select commit Hold shift + click to select a range
5be81ef
Incorporating TagMap into the tracer
dougqh Mar 18, 2025
6eefbca
Making more locals final for consistency
dougqh Mar 18, 2025
81d3da6
Taking final off of the product specific helper locals for now
dougqh Mar 18, 2025
ca7b135
Fixed bug in TagMap$BucketGroup#_isEmpty
dougqh Mar 19, 2025
a570c4d
spotless
dougqh Mar 19, 2025
a5e62cf
Restoring actor.ip check
dougqh Mar 19, 2025
b5cb015
Removing commented out tagsSize code
dougqh Mar 19, 2025
b13d557
Adding lots of Javadoc to TagMap
dougqh Mar 19, 2025
371bf18
More commenting of TagMap implementation
dougqh Mar 19, 2025
32d5849
More commenting of the TagMap implementation
dougqh Mar 19, 2025
c252e7b
Adding tests for BucketGroup
dougqh Mar 20, 2025
468aee6
BucketGroup chain tests
dougqh Mar 20, 2025
e58d8ee
More BucketGroup tests
dougqh Mar 20, 2025
93c8d08
spotless & doc clean-up
dougqh Mar 20, 2025
b489ce9
Tried default case per analysis tool - it was worse
dougqh Mar 20, 2025
6b01cb3
More comments to clarify optimizations
dougqh Mar 20, 2025
4d36dba
Fixing potential bug with SpanBuilder tag removal clobbering entry fr…
dougqh Mar 25, 2025
b9238c8
spotless
dougqh Mar 25, 2025
00adef8
Progress on fixing failing tests
dougqh Mar 26, 2025
4ae0c52
Fixed the last getTags() mock that I missed previously
dougqh Mar 26, 2025
fae5b14
Merge branch 'master' into dougqh/interceptor-bypass
dougqh Mar 26, 2025
376d623
spotless
dougqh Mar 26, 2025
55beae5
Fixing errant merge which left imports to recently removed classes
dougqh Mar 26, 2025
096a565
Fixing a failing test that was added by the latest merge?
dougqh Mar 26, 2025
b208ce1
Removing unnecessary comment
dougqh Mar 26, 2025
616e55d
Fixing infinite recursion found by SpotBugs
dougqh Mar 26, 2025
06c5575
spotless
dougqh Mar 26, 2025
ce2792a
quieting spotbugs
dougqh Mar 26, 2025
fe44114
spotless
dougqh Mar 27, 2025
ad8228b
Incorporating TagMap into the tracer
dougqh Mar 18, 2025
97bbb2c
Making more locals final for consistency
dougqh Mar 18, 2025
d296a7a
Taking final off of the product specific helper locals for now
dougqh Mar 18, 2025
b5ce9a7
Fixed bug in TagMap$BucketGroup#_isEmpty
dougqh Mar 19, 2025
ed3b5a8
spotless
dougqh Mar 19, 2025
fd45d8d
Restoring actor.ip check
dougqh Mar 19, 2025
456ef66
Removing commented out tagsSize code
dougqh Mar 19, 2025
3d89c07
Adding lots of Javadoc to TagMap
dougqh Mar 19, 2025
ad2ede7
More commenting of TagMap implementation
dougqh Mar 19, 2025
713be83
More commenting of the TagMap implementation
dougqh Mar 19, 2025
f94ffbc
Adding tests for BucketGroup
dougqh Mar 20, 2025
99cf6c5
BucketGroup chain tests
dougqh Mar 20, 2025
6d487ec
More BucketGroup tests
dougqh Mar 20, 2025
50c367f
spotless & doc clean-up
dougqh Mar 20, 2025
e7c6c26
Tried default case per analysis tool - it was worse
dougqh Mar 20, 2025
e474365
More comments to clarify optimizations
dougqh Mar 20, 2025
c4f4a00
Fixing potential bug with SpanBuilder tag removal clobbering entry fr…
dougqh Mar 25, 2025
77eba45
spotless
dougqh Mar 25, 2025
d3cc062
Progress on fixing failing tests
dougqh Mar 26, 2025
2c8c3ff
Fixed the last getTags() mock that I missed previously
dougqh Mar 26, 2025
cae4880
spotless
dougqh Mar 26, 2025
b0e0a88
Removing unnecessary comment
dougqh Mar 26, 2025
db83248
Fixing infinite recursion found by SpotBugs
dougqh Mar 26, 2025
6168fec
spotless
dougqh Mar 26, 2025
12f97b7
quieting spotbugs
dougqh Mar 26, 2025
6f41151
spotless
dougqh Mar 27, 2025
b1ddc8d
Fix-up after rebase oversight
dougqh Mar 27, 2025
fd610ca
Merge branch 'dougqh/interceptor-bypass' of github.com:DataDog/dd-tra…
dougqh Mar 27, 2025
2413e1d
spotless
dougqh Mar 27, 2025
5ff6f8f
Improving TagMap test coverage
dougqh Mar 27, 2025
d3b4a0e
More TagMap.Builder coverage
dougqh Mar 27, 2025
433650d
More TagMap test coverage - entrySet, keySet, values
dougqh Mar 27, 2025
1915fd6
Working on Entry test coverage
dougqh Mar 28, 2025
6335a47
Improving TagMap.Entry coverage - covering boolean conversions
dougqh Mar 28, 2025
c502dc3
More TagMap coverage
dougqh Mar 28, 2025
f7eee67
Added multi-threaded tests for TagMap.Entry
dougqh Mar 28, 2025
5e15723
Adding test for removal entry
dougqh Mar 28, 2025
34ed23d
Working on TagMapTest coverage
dougqh Mar 31, 2025
93cb23a
Improving coverage - added "test" for toInternalString
dougqh Mar 31, 2025
a4aaa71
Appease spotbugs
dougqh Mar 31, 2025
63b5a2e
Increasing bucket size to 32 - provides better results under "real" w…
dougqh Apr 7, 2025
6bd0655
Merge branch 'master' into dougqh/interceptor-bypass
dougqh Apr 22, 2025
04c48d8
Smaller array for EMPTY instance
dougqh Apr 23, 2025
e3d1faf
More tests
dougqh Apr 24, 2025
24af078
More tests
dougqh Apr 24, 2025
c5aa244
spotless
dougqh Apr 24, 2025
266b36d
Updated TagMap to track size
dougqh Apr 25, 2025
176846f
Optimizing putAll into a new empty map
dougqh Apr 25, 2025
0c324b1
Removing immutable
dougqh Apr 25, 2025
3487052
spotless & javadoc clean-up
dougqh Apr 25, 2025
f6a5794
Tweaking API
dougqh Apr 28, 2025
a395a5d
Changing TagMap.Builder -> TagMap.Ledger
dougqh Apr 29, 2025
8612213
Merge branch 'master' into dougqh/interceptor-bypass
dougqh May 8, 2025
9f610f9
TagMap that can "mostly" be toggled on / off
dougqh May 8, 2025
648c15c
spotless
dougqh May 8, 2025
ee432c9
More tests
dougqh May 9, 2025
96422cf
Merge branch 'master' into dougqh/interceptor-bypass
dougqh May 9, 2025
1c0b641
Adding configuration variable
dougqh May 9, 2025
fe96451
Merge branch 'dougqh/interceptor-bypass' of github.com:DataDog/dd-tra…
dougqh May 9, 2025
a37b6f8
Adding getOrDefault methods to TagMap
dougqh May 9, 2025
b258b90
Update internal-api/src/main/java/datadog/trace/api/TagMap.java
dougqh May 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import datadog.trace.api.Config;
import datadog.trace.api.DDTags;
import datadog.trace.api.DDTraceId;
import datadog.trace.api.TagMap;
import datadog.trace.api.TraceConfig;
import datadog.trace.api.function.TriConsumer;
import datadog.trace.api.function.TriFunction;
Expand Down Expand Up @@ -408,7 +409,7 @@ public String getSpanType() {
}

@Override
public Map<String, Object> getTags() {
public TagMap getTags() {
return serverSpan.getTags();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public AbstractTestSession(
AgentSpanContext traceContext =
new TagContext(
CIConstants.CIAPP_TEST_ORIGIN,
Collections.emptyMap(),
Copy link
Contributor Author

@dougqh dougqh Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have mixed feelings about this particular change. In effect, the constructor previously required the user to pass a mutable map. However if the provided Map was empty, the class would lazily construction a mutable Map to take place of the empty Map.

Because TagMap does not have an O(1) isEmpty, I didn't want to stick with this pattern.

What could be done instead is to pass TagMap.EMPTY and then check via a reference equality check. If others prefer that, I can adjust accordingly.

null,
null,
null,
PrioritySampling.UNSET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import datadog.trace.civisibility.test.ExecutionResults;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Collections;
import java.util.function.Consumer;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -101,8 +100,7 @@ public TestImpl(

this.context = new TestContextImpl(coverageStore);

AgentSpanContext traceContext =
new TagContext(CIConstants.CIAPP_TEST_ORIGIN, Collections.emptyMap());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same mutable empty Map issue

AgentSpanContext traceContext = new TagContext(CIConstants.CIAPP_TEST_ORIGIN, null);
AgentTracer.SpanBuilder spanBuilder =
AgentTracer.get()
.buildSpan(CI_VISIBILITY_INSTRUMENTATION_NAME, testDecorator.component() + ".test")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.datadog.debugger.util.ExceptionHelper;
import com.datadog.debugger.util.TestSnapshotListener;
import datadog.trace.api.Config;
import datadog.trace.api.TagMap;
import datadog.trace.bootstrap.debugger.CapturedContext;
import datadog.trace.bootstrap.debugger.CapturedStackFrame;
import datadog.trace.bootstrap.debugger.MethodLocation;
Expand All @@ -41,7 +42,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -57,7 +57,7 @@ public class DefaultExceptionDebuggerTest {
private ConfigurationUpdater configurationUpdater;
private DefaultExceptionDebugger exceptionDebugger;
private TestSnapshotListener listener;
private Map<String, Object> spanTags = new HashMap<>();
Copy link
Contributor Author

@dougqh dougqh Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the primary type of change that I've made throughout -- replacing HashMaps with TagMap-s.
To get the benefit of TagMap's quick Map-to-Map copying ability, both the source and destination Map need to be TagMap-s.

private TagMap spanTags = TagMap.create();

@BeforeEach
public void setUp() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.datadog.iast.model.Vulnerability
import com.datadog.iast.model.VulnerabilityType
import com.datadog.iast.overhead.Operation
import com.datadog.iast.overhead.OverheadController
import datadog.trace.api.TagMap
import datadog.trace.api.gateway.Flow
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.internal.TraceSegment
Expand Down Expand Up @@ -45,10 +46,10 @@ class HstsMissingHeaderModuleTest extends IastModuleImplTestBase {
final handler = new RequestEndedHandler(dependencies)
ctx.xForwardedProto = 'https'
ctx.contentType = "text/html"
span.getTags() >> [
span.getTags() >> TagMap.fromMap([
'http.url': 'https://localhost/a',
'http.status_code': 200i
]
])

when:
def flow = handler.apply(reqCtx, span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.datadog.iast.Reporter
import com.datadog.iast.RequestEndedHandler
import com.datadog.iast.model.Vulnerability
import com.datadog.iast.model.VulnerabilityType
import datadog.trace.api.TagMap
import datadog.trace.api.gateway.Flow
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.sink.InsecureAuthProtocolModule
Expand Down Expand Up @@ -42,9 +43,9 @@ class InsecureAuthProtocolModuleTest extends IastModuleImplTestBase{
given:
final handler = new RequestEndedHandler(dependencies)
ctx.authorization = value
span.getTags() >> [
span.getTags() >> TagMap.fromMap([
'http.status_code': status_code
]
])

when:
def flow = handler.apply(reqCtx, span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.datadog.iast.Reporter
import com.datadog.iast.RequestEndedHandler
import com.datadog.iast.model.Vulnerability
import com.datadog.iast.model.VulnerabilityType
import datadog.trace.api.TagMap
import datadog.trace.api.gateway.Flow
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.internal.TraceSegment
Expand Down Expand Up @@ -33,9 +34,9 @@ class XContentTypeOptionsModuleTest extends IastModuleImplTestBase {
given:
final handler = new RequestEndedHandler(dependencies)
ctx.contentType = "text/html"
span.getTags() >> [
span.getTags() >> TagMap.fromMap([
'http.status_code': 200i
]
])

when:
def flow = handler.apply(reqCtx, span)
Expand All @@ -56,10 +57,10 @@ class XContentTypeOptionsModuleTest extends IastModuleImplTestBase {
final handler = new RequestEndedHandler(dependencies)
ctx.xForwardedProto = 'https'
ctx.contentType = "text/html"
span.getTags() >> [
span.getTags() >> TagMap.fromMap([
'http.url': url,
'http.status_code': status
]
])

when:
def flow = handler.apply(reqCtx, span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import datadog.remoteconfig.ConfigurationEndListener
import datadog.remoteconfig.ConfigurationPoller
import datadog.remoteconfig.Product
import datadog.trace.api.Config
import datadog.trace.api.TagMap
import datadog.trace.api.internal.TraceSegment
import datadog.trace.api.gateway.Flow
import datadog.trace.api.gateway.IGSpanInfo
Expand Down Expand Up @@ -72,7 +73,7 @@ class AppSecSystemSpecification extends DDSpecification {
requestEndedCB.apply(requestContext, span)

then:
1 * span.getTags() >> ['http.client_ip':'1.1.1.1']
1 * span.getTags() >> TagMap.fromMap(['http.client_ip':'1.1.1.1'])
1 * subService.registerCallback(EVENTS.requestEnded(), _) >> { requestEndedCB = it[1]; null }
1 * requestContext.getData(RequestContextSlot.APPSEC) >> appSecReqCtx
1 * requestContext.traceSegment >> traceSegment
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

package com.datadog.appsec.gateway

import com.datadog.appsec.AppSecSystem
Expand All @@ -9,6 +10,7 @@ import com.datadog.appsec.event.data.DataBundle
import com.datadog.appsec.event.data.KnownAddresses
import com.datadog.appsec.report.AppSecEvent
import com.datadog.appsec.report.AppSecEventWrapper
import datadog.trace.api.TagMap
import datadog.trace.api.function.TriConsumer
import datadog.trace.api.function.TriFunction
import datadog.trace.api.gateway.BlockResponseFunction
Expand Down Expand Up @@ -167,7 +169,7 @@ class GatewayBridgeSpecification extends DDSpecification {
def flow = requestEndedCB.apply(mockCtx, spanInfo)

then:
1 * spanInfo.getTags() >> ['http.client_ip': '1.1.1.1']
1 * spanInfo.getTags() >> TagMap.fromMap(['http.client_ip': '1.1.1.1'])
1 * mockAppSecCtx.transferCollectedEvents() >> [event]
1 * mockAppSecCtx.peerAddress >> '2001::1'
1 * mockAppSecCtx.close()
Expand Down Expand Up @@ -206,7 +208,7 @@ class GatewayBridgeSpecification extends DDSpecification {

then:
1 * mockAppSecCtx.transferCollectedEvents() >> [Stub(AppSecEvent)]
1 * spanInfo.getTags() >> ['http.client_ip': '8.8.8.8']
1 * spanInfo.getTags() >> TagMap.fromMap(['http.client_ip': '8.8.8.8'])
1 * traceSegment.setTagTop('actor.ip', '8.8.8.8')
}

Expand Down Expand Up @@ -987,7 +989,7 @@ class GatewayBridgeSpecification extends DDSpecification {
getTraceSegment() >> traceSegment
}
final spanInfo = Mock(AgentSpan) {
getTags() >> ['http.route':'/']
getTags() >> TagMap.fromMap(['http.route':'/'])
}

when:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,7 @@ public final class GeneralConfig {
public static final String APM_TRACING_ENABLED = "apm.tracing.enabled";
public static final String JDK_SOCKET_ENABLED = "jdk.socket.enabled";

public static final String OPTIMIZED_MAP_ENABLED = "optimized.map.enabled";

private GeneralConfig() {}
}
Loading