From 4356eda3bcd31e9d7c63cc02c6a4e6662a434e64 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 6 Feb 2025 12:19:11 +0100 Subject: [PATCH 01/32] Fix org.json iast instrumentation test for latest dependency --- .../instrumentation/org-json/build.gradle | 2 +- .../instrumentation/org-json/gradle.lockfile | 2 +- .../json/JSONArrayInstrumentation.java | 27 -------- .../json/JSONObjectInstrumentation.java | 27 -------- .../JSONArrayInstrumentationTest.groovy | 61 ++++++++-------- .../JSONObjectInstrumentationTest.groovy | 69 +++++++------------ 6 files changed, 55 insertions(+), 133 deletions(-) diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index ac7e4b067c9..a9526cd5d9d 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -18,5 +18,5 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') //FIXME: ASM - latestDepTestImplementation group: 'org.json', name: 'json', version: '20240303' + latestDepTestImplementation group: 'org.json', name: 'json', version: '+' } diff --git a/dd-java-agent/instrumentation/org-json/gradle.lockfile b/dd-java-agent/instrumentation/org-json/gradle.lockfile index 46487dd2105..098936c2c5f 100644 --- a/dd-java-agent/instrumentation/org-json/gradle.lockfile +++ b/dd-java-agent/instrumentation/org-json/gradle.lockfile @@ -116,7 +116,7 @@ org.hamcrest:hamcrest-core:1.3=latestDepTestCompileClasspath,latestDepTestRuntim org.hamcrest:hamcrest:2.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.jctools:jctools-core:3.3.0=instrumentPluginClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testRuntimeClasspath org.json:json:20230227=compileClasspath,testCompileClasspath,testRuntimeClasspath -org.json:json:20240303=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath +org.json:json:20250107=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath org.junit.jupiter:junit-jupiter-api:5.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.junit.jupiter:junit-jupiter-engine:5.9.2=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-commons:1.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 942a3905592..7bd4f6ebc64 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -35,13 +35,6 @@ public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isConstructor().and(takesArguments(String.class)), getClass().getName() + "$ConstructorAdvice"); - transformer.applyAdvice( - isMethod() - .and(isPublic()) - .and(returns(Object.class)) - .and(named("get")) - .and(takesArguments(1)), - getClass().getName() + "$GetAdvice"); transformer.applyAdvice( isMethod().and(isPublic()).and(returns(Object.class)).and(named("opt")), getClass().getName() + "$OptAdvice"); @@ -58,26 +51,6 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } - public static class GetAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } - public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index c260d5390ad..9746eda1fd5 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -33,13 +33,6 @@ public String instrumentedType() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isConstructor().and(takesArguments(1)), getClass().getName() + "$ConstructorAdvice"); - transformer.applyAdvice( - isMethod() - .and(isPublic()) - .and(returns(Object.class)) - .and(named("get")) - .and(takesArguments(String.class)), - getClass().getName() + "$GetAdvice"); transformer.applyAdvice( isMethod() .and(isPublic()) @@ -60,26 +53,6 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } - public static class GetAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } - public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy index d0a97012811..4136d2d3043 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy @@ -7,6 +7,16 @@ import org.json.JSONTokener class JSONArrayInstrumentationTest extends AgentTestRunner { + private static json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" + @Override void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") @@ -16,28 +26,17 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + final jsonObject = new JSONObject(json) + final menuObject = jsonObject.getJSONObject("menu") when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").getJSONArray("labels").get(0) + final array = menuObject.getJSONArray("labels") then: - name == "File" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) - 2 * module.taintStringIfTainted("File", _ as JSONArray) + array.length() == 2 + array.get(0) == "File" + array.get(1) == "Edit" + 1 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) 0 * _ } @@ -45,28 +44,22 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + final jsonObject = new JSONObject(json) + final jsonArray =jsonObject.getJSONObject("menu").getJSONArray("labels") when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").getJSONArray("labels").optString(0, "defaultvalue") + final name = jsonArray.optString(0, "defaultvalue") then: name == "File" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) 1 * module.taintStringIfTainted("File", _ as JSONArray) 0 * _ + + where: + method | arguments + "opt" | [0] + "optString" | [0, "defaultvalue"] + "get" | [0] + "getString" | [0, "defaultvalue"] } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy index 49be03842d2..3b9296c6b4d 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy @@ -25,83 +25,66 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { }}""" when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").get("name") + new JSONObject(json) then: - name == "nameTest" 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } - void 'test JSONObject opt'() { + void 'test JSONObject JSonTokenizer constructor'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" + final json = '{"name": "nameTest", "value" : "valueTest"}' + final jsonTokener = new JSONTokener(json) when: - final jsonObject = new JSONObject(json) - final name = jsonObject.getJSONObject("menu").optString("name") + new JSONObject(jsonTokener) then: - name == "nameTest" - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) 0 * _ } - - - void 'test JSONObject JSonTokenizer constructor'() { + void 'test JSONObject map constructor'(){ given: + final Map map = new HashMap<>() + map.put("name", "nameTest") + map.put("age", "22") + map.put("city", "chicago") final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = '{"name": "nameTest", "value" : "valueTest"}' when: - final jsonObject = new JSONObject(new JSONTokener(json)) - final name = jsonObject.get("name") + new JSONObject(map) then: - name == "nameTest" - 1 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONObject, map) 0 * _ } - void 'test JSONObject map constructor'(){ + + void 'test JSONObject #method'() { given: - final Map map = new HashMap<>() - map.put("name", "nameTest") - map.put("age", "22") - map.put("city", "chicago") final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) + final json = """{"menu": { + "name": "nameTest" + }}""" + final jsonObject = new JSONObject(json) + final getObject =jsonObject.getJSONObject("menu") when: - final jsonObject = new JSONObject(map) - jsonObject.get("name") + final name = getObject."$method"('name') then: - 1 * module.taintObjectIfTainted(_ as JSONObject, map) - 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) + name == "nameTest" + 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ + + where: + method << ['get', 'getString', 'opt', 'optString'] } } From f149563984812597f85a12d7f82e9fea5a26acb1 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 6 Feb 2025 13:28:37 +0100 Subject: [PATCH 02/32] fix codenarc --- .../org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy index 4136d2d3043..e654c0f89a3 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy @@ -3,7 +3,6 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONArray import org.json.JSONObject -import org.json.JSONTokener class JSONArrayInstrumentationTest extends AgentTestRunner { From 08dd68f814b1d7cb06ae537b33891d9a4397fe1d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 7 Feb 2025 15:20:39 +0100 Subject: [PATCH 03/32] add new instrumentation and cover more constructors --- .../iastinstrumenter/iast_exclusion.trie | 2 + .../java/lang/InputStreamReaderCallSite.java | 1 + .../io/InputStreamReaderCallSiteTest.groovy | 9 ++- .../foo/bar/TestInputStreamReaderSuite.java | 4 + .../instrumentation/org-json/build.gradle | 1 + .../json/JSONArrayInstrumentation.java | 27 +------ ...SONObjectAfter20250107Instrumentation.java | 80 +++++++++++++++++++ ...NObjectBefore20250107Instrumentation.java} | 40 ++++------ .../json/JSONTokenerInstrumentation.java | 3 +- .../trace/instrumentation/json/OptAdvice.java | 28 +++++++ .../JSONCookieInstrumentationTest.groovy | 3 +- .../JSONObjectInstrumentationTest.groovy | 67 +++++++++++++++- .../JSONTokenerInstrumentationTest.groovy | 27 +++++-- 13 files changed, 231 insertions(+), 61 deletions(-) create mode 100644 dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObjectInstrumentation.java => JSONObjectBefore20250107Instrumentation.java} (66%) create mode 100644 dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java diff --git a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie index e3ef2739905..699a84adcaf 100644 --- a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie +++ b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie @@ -256,6 +256,8 @@ 1 org.jooq.* 1 org.jruby.* 1 org.json.* +#Needed for JSON propagation +2 org.json.JSONTokener 1 org.jsoup.* 1 org.junit.* 1 org.jvnet.hk2.* diff --git a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java index e78db178697..444879d51b5 100644 --- a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java +++ b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java @@ -12,6 +12,7 @@ @CallSite(spi = IastCallSites.class) public class InputStreamReaderCallSite { + @CallSite.After("void java.io.InputStreamReader.(java.io.InputStream)") @CallSite.After( "void java.io.InputStreamReader.(java.io.InputStream, java.nio.charset.Charset)") public static InputStreamReader afterInit( diff --git a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy index 7b5a3e568ff..5dce907e0df 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy @@ -14,10 +14,17 @@ class InputStreamReaderCallSiteTest extends BaseIoCallSiteTest{ InstrumentationBridge.registerIastModule(iastModule) when: - TestInputStreamReaderSuite.init(new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()) + TestInputStreamReaderSuite.init(*args) then: 1 * iastModule.taintObjectIfTainted(_ as InputStreamReader, _ as InputStream) 0 * _ + + where: + args << [ + [new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()], + // InputStream input + [new ByteArrayInputStream("test".getBytes())]// Reader input + ] } } diff --git a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java index 57c572575a5..1d79a1a7b9b 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java +++ b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java @@ -9,4 +9,8 @@ public class TestInputStreamReaderSuite { public static InputStreamReader init(final InputStream in, Charset charset) { return new InputStreamReader(in, charset); } + + public static InputStreamReader init(final InputStream in) { + return new InputStreamReader(in); + } } diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index a9526cd5d9d..6bce025b3d6 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -16,6 +16,7 @@ dependencies { testImplementation group: 'org.json', name: 'json', version: '20230227' testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') + testRuntimeOnly project(':dd-java-agent:instrumentation:java-io') //Needed for Reader //FIXME: ASM latestDepTestImplementation group: 'org.json', name: 'json', version: '+' diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 7bd4f6ebc64..1ad5895dfb1 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -5,6 +5,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -14,8 +15,6 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; -import org.json.JSONArray; -import org.json.JSONObject; @AutoService(InstrumenterModule.class) public class JSONArrayInstrumentation extends InstrumenterModule.Iast @@ -33,11 +32,11 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(String.class)), + isConstructor().and(takesArguments(0)).and(takesArgument(0, named("org.json.JSONTokener"))), getClass().getName() + "$ConstructorAdvice"); transformer.applyAdvice( isMethod().and(isPublic()).and(returns(Object.class)).and(named("opt")), - getClass().getName() + "$OptAdvice"); + packageName + ".OptAdvice"); } public static class ConstructorAdvice { @@ -50,24 +49,4 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } - - public static class OptAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java new file mode 100644 index 00000000000..c8f74ad02fd --- /dev/null +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java @@ -0,0 +1,80 @@ +package datadog.trace.instrumentation.json; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumenterModule.class) +public class JSONObjectAfter20250107Instrumentation extends InstrumenterModule.Iast + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + public JSONObjectAfter20250107Instrumentation() { + super("org-json"); + } + + // Avoid matching servlet 3 which has its own instrumentation + static final ElementMatcher.Junction AFTER_20250107 = + hasClassNamed("org.json.StringBuilderWriter"); + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return AFTER_20250107; + } + + @Override + public String instrumentedType() { + return "org.json.JSONObject"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + // public JSONObject(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) + transformer.applyAdvice( + isConstructor() + .and(takesArguments(2)) + .and(takesArgument(0, named("org.json.JSONTokener"))) + .and(takesArgument(1, named("org.json.JSONParserConfiguration"))), + getClass().getName() + "$ConstructorAdvice"); + // private JSONObject(Map m, int recursionDepth, JSONParserConfiguration + // jsonParserConfiguration) + transformer.applyAdvice( + isConstructor() + .and(takesArguments(3)) + .and(takesArgument(0, Map.class)) + .and(takesArgument(1, int.class)) + .and(takesArgument(2, named("org.json.JSONParserConfiguration"))), + getClass().getName() + "$ConstructorAdvice"); + transformer.applyAdvice( + isMethod() + .and(isPublic()) + .and(returns(Object.class)) + .and(named("opt")) + .and(takesArguments(String.class)), + packageName + ".OptAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final Object input) { + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null && input != null) { + iastModule.taintObjectIfTainted(self, input); + } + } + } +} diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java similarity index 66% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java index 9746eda1fd5..bac52b01b54 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java @@ -1,9 +1,11 @@ package datadog.trace.instrumentation.json; +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -14,16 +16,24 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; -import org.json.JSONArray; -import org.json.JSONObject; +import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) -public class JSONObjectInstrumentation extends InstrumenterModule.Iast +public class JSONObjectBefore20250107Instrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectInstrumentation() { + public JSONObjectBefore20250107Instrumentation() { super("org-json"); } + // Avoid matching servlet 3 which has its own instrumentation + static final ElementMatcher.Junction BEFORE_20250107 = + not(hasClassNamed("org.json.StringBuilderWriter")); + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return BEFORE_20250107; + } + @Override public String instrumentedType() { return "org.json.JSONObject"; @@ -39,7 +49,7 @@ public void methodAdvice(MethodTransformer transformer) { .and(returns(Object.class)) .and(named("opt")) .and(takesArguments(String.class)), - getClass().getName() + "$OptAdvice"); + packageName + ".OptAdvice"); } public static class ConstructorAdvice { @@ -52,24 +62,4 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } - - public static class OptAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } - } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java index eb51d248770..e0126f8a76b 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java @@ -9,6 +9,7 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; +import java.io.Reader; import net.bytebuddy.asm.Advice; @AutoService(InstrumenterModule.class) @@ -27,7 +28,7 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(String.class)), + isConstructor().and(takesArguments(Reader.class)), getClass().getName() + "$ConstructorAdvice"); } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java new file mode 100644 index 00000000000..4e40cb0d098 --- /dev/null +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java @@ -0,0 +1,28 @@ +package datadog.trace.instrumentation.json; + +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.propagation.PropagationModule; +import net.bytebuddy.asm.Advice; +import org.json.JSONArray; +import org.json.JSONObject; + +public class OptAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } +} diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy index 004b7afdd22..ca7f07eddfe 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy @@ -23,7 +23,8 @@ class JSONCookieInstrumentationTest extends AgentTestRunner { then: 1 * module.taintObjectIfTainted(_ as JSONObject, cookie) - 1 * module.taintObjectIfTainted(_ as JSONTokener, cookie) + 1 * module.taintObjectIfTainted(_ as Reader, cookie) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) 0 * _ } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy index 3b9296c6b4d..ee0e3b897f7 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy @@ -3,6 +3,7 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONObject import org.json.JSONTokener +import spock.lang.Shared class JSONObjectInstrumentationTest extends AgentTestRunner { @@ -10,6 +11,15 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { injectSysConfig("dd.iast.enabled", "true") } + @Shared + Map tainteds = new IdentityHashMap<>() + + + void setup() { + tainteds.clear() + } + + void 'test JSONObject string constructor'() { given: final module = Mock(PropagationModule) @@ -28,8 +38,10 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { new JSONObject(json) then: - 1 * module.taintObjectIfTainted(_ as JSONObject, json) - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 1 * module.taintObjectIfTainted(_ as Reader, json) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) + // Two calls are necessary, once for the whole object and other for the menu object + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) 0 * _ } @@ -65,13 +77,18 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { 0 * _ } - void 'test JSONObject #method'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) final json = """{"menu": { - "name": "nameTest" + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] }}""" final jsonObject = new JSONObject(json) final getObject =jsonObject.getJSONObject("menu") @@ -87,4 +104,46 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { where: method << ['get', 'getString', 'opt', 'optString'] } + + void 'test JSONObject elements are tainted'() { + given: + final module = Mock(PropagationModule) { + taintObjectIfTainted(_, _) >> { + if (tainteds.containsKey(it[1])) { + tainteds.put(it[0], null) + } + } + taintStringIfTainted(_, _) >> { + if (tainteds.containsKey(it[1])) { + tainteds.put(it[0], null) + } + } + } + InstrumentationBridge.registerIastModule(module) + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" + tainteds.put(json, null) + final jsonObject = new JSONObject(json) + final getObject =jsonObject.getJSONObject("menu") + + when: + final name = getObject.get('name') + final file = getObject.get('labels').get(0) + final edit = getObject.get('labels').get(1) + + then: + name == "nameTest" + tainteds.containsKey(name) + file == "File" + tainteds.containsKey(file) + edit == "Edit" + tainteds.containsKey(edit) + } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy index de8fa1da40c..806a691074a 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy @@ -5,21 +5,38 @@ import org.json.JSONTokener class JSONTokenerInstrumentationTest extends AgentTestRunner { - @Override void configurePreAgent() { + private static final String JSON_STRING = '{"name": "nameTest", "value" : "valueTest"}' // Reused JSON String + + @Override + void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") } - void 'test JSONTokener string constructor'() { + void 'test JSONTokener constructor with different argument types'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = '{"name": "nameTest", "value" : "valueTest"}' when: - new JSONTokener(json) + new JSONTokener(args) then: - 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 1 * module.taintObjectIfTainted(_ as JSONTokener, _) + if(args instanceof String) { + 1 * module.taintObjectIfTainted(_ as Reader, _ as String) + } else if (args instanceof InputStream) { + 1 * module.taintObjectIfTainted(_ as Reader, _ as InputStream) + } 0 * _ + + where: + args << [ + JSON_STRING, + // String input + new ByteArrayInputStream(JSON_STRING.bytes), + // InputStream input + new StringReader(JSON_STRING) // Reader input + ] } + } From 05e6525178b28c7c8ffae0e34d7a4c015eed8b70 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 10:43:13 +0100 Subject: [PATCH 04/32] fix muzzle and check previous latest version --- .../instrumentation/org-json/build.gradle | 16 ++++++++++++- .../json/JSONArrayInstrumentation.java | 5 ++++ .../json/JSONCookieInstrumentation.java | 5 ++++ ...=> JSONObject20241224Instrumentation.java} | 14 +++++++---- ...on.java => JSONObjectInstrumentation.java} | 24 ++++++++++++++----- .../json/JSONTokenerInstrumentation.java | 5 ++++ 6 files changed, 57 insertions(+), 12 deletions(-) rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObjectAfter20250107Instrumentation.java => JSONObject20241224Instrumentation.java} (90%) rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObjectBefore20250107Instrumentation.java => JSONObjectInstrumentation.java} (73%) diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index 6bce025b3d6..07bb471caa7 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -1,10 +1,25 @@ muzzle { pass { + name = 'all' group = 'org.json' module = 'json' versions = '[20070829, ]' assertInverse = true } + pass { + name = 'before_20241224' + group = 'org.json' + module = 'json' + versions = '[20070829, 20241224)' + assertInverse = true + } + pass { + name = 'after_20241224' + group = 'org.json' + module = 'json' + versions = '[20241224, ]' + assertInverse = true + } } apply from: "$rootDir/gradle/java.gradle" @@ -18,6 +33,5 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') testRuntimeOnly project(':dd-java-agent:instrumentation:java-io') //Needed for Reader - //FIXME: ASM latestDepTestImplementation group: 'org.json', name: 'json', version: '+' } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 1ad5895dfb1..869de8beeb8 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -24,6 +24,11 @@ public JSONArrayInstrumentation() { super("org-json"); } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public String instrumentedType() { return "org.json.JSONArray"; diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java index f9a25abb04d..26e332f9956 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java @@ -19,6 +19,11 @@ public JSONCookieInstrumentation() { super("org-json"); } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public String instrumentedType() { return "org.json.Cookie"; diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java similarity index 90% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java index c8f74ad02fd..5cb96ad7ffa 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java @@ -20,19 +20,23 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) -public class JSONObjectAfter20250107Instrumentation extends InstrumenterModule.Iast +public class JSONObject20241224Instrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectAfter20250107Instrumentation() { + public JSONObject20241224Instrumentation() { super("org-json"); } - // Avoid matching servlet 3 which has its own instrumentation - static final ElementMatcher.Junction AFTER_20250107 = + static final ElementMatcher.Junction AFTER_20241224 = hasClassNamed("org.json.StringBuilderWriter"); + @Override + public String muzzleDirective() { + return "after_20241224"; + } + @Override public ElementMatcher.Junction classLoaderMatcher() { - return AFTER_20250107; + return AFTER_20241224; } @Override diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java similarity index 73% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index bac52b01b54..dabee96e25c 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -7,6 +7,7 @@ import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -15,23 +16,28 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) -public class JSONObjectBefore20250107Instrumentation extends InstrumenterModule.Iast +public class JSONObjectInstrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectBefore20250107Instrumentation() { + public JSONObjectInstrumentation() { super("org-json"); } - // Avoid matching servlet 3 which has its own instrumentation - static final ElementMatcher.Junction BEFORE_20250107 = + static final ElementMatcher.Junction BEFORE_20241224 = not(hasClassNamed("org.json.StringBuilderWriter")); @Override public ElementMatcher.Junction classLoaderMatcher() { - return BEFORE_20250107; + return BEFORE_20241224; + } + + @Override + public String muzzleDirective() { + return "before_20241224"; } @Override @@ -41,8 +47,14 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { + // public JSONObject(JSONTokener x) + transformer.applyAdvice( + isConstructor().and(takesArguments(1)).and(takesArgument(0, named("org.json.JSONTokener"))), + getClass().getName() + "$ConstructorAdvice"); + // private JSONObject(Map m) transformer.applyAdvice( - isConstructor().and(takesArguments(1)), getClass().getName() + "$ConstructorAdvice"); + isConstructor().and(takesArguments(1)).and(takesArgument(0, Map.class)), + getClass().getName() + "$ConstructorAdvice"); transformer.applyAdvice( isMethod() .and(isPublic()) diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java index e0126f8a76b..b4fb20a64c3 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java @@ -25,6 +25,11 @@ public String instrumentedType() { return "org.json.JSONTokener"; } + @Override + public String muzzleDirective() { + return "all"; + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( From a2eafb2efc238f191f1610a14df0e96421fc184f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 16 Jan 2025 09:56:43 +0100 Subject: [PATCH 05/32] add new config property apm sampling enabled and move to _dd.p.ts --- .../main/java/com/datadog/iast/Reporter.java | 3 +- .../com/datadog/iast/ReporterTest.groovy | 9 +- .../datadog/appsec/gateway/GatewayBridge.java | 7 +- .../appsec/powerwaf/PowerWAFModule.java | 3 +- .../gateway/GatewayBridgeSpecification.groovy | 9 +- .../build.gradle | 0 .../gradle.lockfile | 0 .../apmtracingdisabled}/AppConfig.java | 2 +- .../apmtracingdisabled}/Controller.java | 2 +- .../SpringbootApplication.java | 2 +- ...bstractApmTracingDisabledSmokeTest.groovy} | 6 +- .../ApmTracingDisabledMatrixSmokeTest.groovy} | 50 ++--- ...pmTracingDisabledSamplingSmokeTest.groovy} | 8 +- .../ApmTracingDisabledSmokeTest.groovy} | 20 +- .../trace/api/config/AppSecConfig.java | 2 - .../trace/api/config/GeneralConfig.java | 1 + ...er.java => ApmTracingDisabledSampler.java} | 15 +- .../trace/common/sampling/Sampler.java | 4 +- .../common/writer/ddagent/DDAgentApi.java | 2 +- .../datadog/trace/core/DDSpanContext.java | 6 +- .../datadog/trace/core/TraceCollector.java | 10 +- .../core/propagation/PropagationTags.java | 4 +- .../propagation/ptags/DatadogPTagsCodec.java | 10 +- .../core/propagation/ptags/PTagsCodec.java | 28 ++- .../core/propagation/ptags/PTagsFactory.java | 42 ++-- .../core/propagation/ptags/W3CPTagsCodec.java | 15 +- .../core/taginterceptor/TagInterceptor.java | 4 +- ...y => ApmTracingDisabledSamplerTest.groovy} | 4 +- .../trace/common/sampling/SamplerTest.groovy | 4 +- .../DatadogPropagationTagsTest.groovy | 24 ++- .../propagation/W3CPropagationTagsTest.groovy | 35 +-- .../taginterceptor/TagInterceptorTest.groovy | 7 +- .../main/java/datadog/trace/api/Config.java | 204 +++++++++--------- .../java/datadog/trace/api/ProductTs.java | 33 +++ .../trace/api/sampling/SamplingMechanism.java | 2 +- .../bootstrap/instrumentation/api/Tags.java | 2 +- .../datadog/trace/api/ProductTsTest.groovy | 60 ++++++ .../api/sampling/SamplingMechanismTest.groovy | 2 +- settings.gradle | 2 +- 39 files changed, 383 insertions(+), 260 deletions(-) rename dd-smoke-tests/{asm-standalone-billing => apm-tracing-disabled}/build.gradle (100%) rename dd-smoke-tests/{asm-standalone-billing => apm-tracing-disabled}/gradle.lockfile (100%) rename dd-smoke-tests/{asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling => apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled}/AppConfig.java (94%) rename dd-smoke-tests/{asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling => apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled}/Controller.java (98%) rename dd-smoke-tests/{asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling => apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled}/SpringbootApplication.java (85%) rename dd-smoke-tests/{asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy => apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/AbstractApmTracingDisabledSmokeTest.groovy} (92%) rename dd-smoke-tests/{asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy => apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledMatrixSmokeTest.groovy} (86%) rename dd-smoke-tests/{asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy => apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSamplingSmokeTest.groovy} (95%) rename dd-smoke-tests/{asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy => apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSmokeTest.groovy} (80%) rename dd-trace-core/src/main/java/datadog/trace/common/sampling/{AsmStandaloneSampler.java => ApmTracingDisabledSampler.java} (68%) rename dd-trace-core/src/test/groovy/datadog/trace/common/sampling/{AsmStandaloneSamplerTest.groovy => ApmTracingDisabledSamplerTest.groovy} (92%) create mode 100644 internal-api/src/main/java/datadog/trace/api/ProductTs.java create mode 100644 internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java index db6632b6d88..c0c637234b6 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java @@ -8,6 +8,7 @@ import com.datadog.iast.model.VulnerabilityBatch; import com.datadog.iast.taint.TaintedObjects; import datadog.trace.api.Config; +import datadog.trace.api.ProductTs; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.internal.TraceSegment; @@ -125,7 +126,7 @@ private VulnerabilityBatch getOrCreateVulnerabilityBatch(final AgentSpan span) { // TODO: We need to check if we can have an API with more fine-grained semantics on why traces // are kept. segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); return batch; } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy index fdba6f7f157..59906365f83 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy @@ -6,6 +6,7 @@ import com.datadog.iast.model.Vulnerability import com.datadog.iast.model.VulnerabilityBatch import com.datadog.iast.model.VulnerabilityType import datadog.trace.api.Config +import datadog.trace.api.ProductTs import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.internal.TraceSegment @@ -85,7 +86,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 1 * reqCtx.getOrCreateMetaStructTop('_dd.stack', _) >> { stackTraceBatch } assertStackTrace(stackTraceBatch, v) 0 * _ @@ -135,7 +136,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 0 * _ } @@ -206,7 +207,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) assertStackTrace(stackTraceBatch, [v1, v2] as Vulnerability[]) 0 * _ } @@ -331,7 +332,7 @@ class ReporterTest extends DDSpecification { 1 * traceSegment.getDataTop('iast') >> null 1 * traceSegment.setDataTop('iast', _ as VulnerabilityBatch) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 1 * traceSegment.setTagTop('_dd.iast.enabled', 1) 1 * reqCtx.getOrCreateMetaStructTop('_dd.stack', _) >> new ConcurrentHashMap<>() 0 * _ diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 8e0ed020dba..52362448dcb 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -27,6 +27,7 @@ import com.datadog.appsec.report.AppSecEvent; import com.datadog.appsec.report.AppSecEventWrapper; import datadog.trace.api.Config; +import datadog.trace.api.ProductTs; import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.gateway.Events; import datadog.trace.api.gateway.Flow; @@ -214,7 +215,7 @@ private Flow onUser( // span with ASM data segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); // skip event if we have an SDK one if (mode != SDK) { @@ -275,7 +276,7 @@ private Flow onLoginEvent( // span with ASM data segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); // update span tags segment.setTagTop("appsec.events." + eventName + ".track", true, true); @@ -789,7 +790,7 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { if (!collectedEvents.isEmpty()) { // Set asm keep in case that root span was not available when events are detected traceSeg.setTagTop(Tags.ASM_KEEP, true); - traceSeg.setTagTop(Tags.PROPAGATED_APPSEC, true); + traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); traceSeg.setTagTop("appsec.event", true); traceSeg.setTagTop("network.client.ip", ctx.getPeerAddress()); diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java index 66ba35a9663..7648eb496c1 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java @@ -26,6 +26,7 @@ import datadog.communication.monitor.Monitoring; import datadog.trace.api.Config; import datadog.trace.api.ProductActivation; +import datadog.trace.api.ProductTs; import datadog.trace.api.gateway.Flow; import datadog.trace.api.telemetry.LogCollector; import datadog.trace.api.telemetry.WafMetricCollector; @@ -498,7 +499,7 @@ public void onDataAvailable( // If APM is disabled, inform downstream services that the current // distributed trace contains at least one ASM event and must inherit // the given force-keep priority - activeSpan.getLocalRootSpan().setTag(Tags.PROPAGATED_APPSEC, true); + activeSpan.getLocalRootSpan().setTag(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); } else { // If active span is not available the ASK_KEEP tag will be set in the GatewayBridge // when the request ends diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 31e87449c92..79179c6a8a6 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -8,6 +8,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.ProductTs import datadog.trace.api.UserIdCollectionMode import datadog.trace.api.appsec.LoginEventCallback import datadog.trace.api.function.TriConsumer @@ -1112,7 +1113,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, true) 1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1151,7 +1152,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, true) 1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1192,7 +1193,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.exists', false, true) 1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1221,7 +1222,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.my.event.track', true, true) 1 * traceSegment.setTagTop('appsec.events.my.event', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) 0 * eventDispatcher.publishDataEvent } diff --git a/dd-smoke-tests/asm-standalone-billing/build.gradle b/dd-smoke-tests/apm-tracing-disabled/build.gradle similarity index 100% rename from dd-smoke-tests/asm-standalone-billing/build.gradle rename to dd-smoke-tests/apm-tracing-disabled/build.gradle diff --git a/dd-smoke-tests/asm-standalone-billing/gradle.lockfile b/dd-smoke-tests/apm-tracing-disabled/gradle.lockfile similarity index 100% rename from dd-smoke-tests/asm-standalone-billing/gradle.lockfile rename to dd-smoke-tests/apm-tracing-disabled/gradle.lockfile diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/AppConfig.java similarity index 94% rename from dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java rename to dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/AppConfig.java index 3db04b37581..ec5c9e386fa 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/AppConfig.java +++ b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/AppConfig.java @@ -1,4 +1,4 @@ -package datadog.smoketest.asmstandalonebilling; +package datadog.smoketest.apmtracingdisabled; import java.util.EnumSet; import javax.servlet.ServletContext; diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java similarity index 98% rename from dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java rename to dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java index ebeecb88642..3bb55197614 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/Controller.java +++ b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java @@ -1,4 +1,4 @@ -package datadog.smoketest.asmstandalonebilling; +package datadog.smoketest.apmtracingdisabled; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.opentracing.Span; diff --git a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/SpringbootApplication.java similarity index 85% rename from dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java rename to dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/SpringbootApplication.java index 0ac36d7d2f7..9d1916cd37b 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/main/java/datadog/smoketest/asmstandalonebilling/SpringbootApplication.java +++ b/dd-smoke-tests/apm-tracing-disabled/src/main/java/datadog/smoketest/apmtracingdisabled/SpringbootApplication.java @@ -1,4 +1,4 @@ -package datadog.smoketest.asmstandalonebilling; +package datadog.smoketest.apmtracingdisabled; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/AbstractApmTracingDisabledSmokeTest.groovy similarity index 92% rename from dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy rename to dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/AbstractApmTracingDisabledSmokeTest.groovy index 8c8663fa946..bf7d1db9cee 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AbstractAsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/AbstractApmTracingDisabledSmokeTest.groovy @@ -1,10 +1,10 @@ -package datadog.smoketest.asmstandalonebilling +package datadog.smoketest.apmtracingdisabled import datadog.smoketest.AbstractServerSmokeTest import datadog.trace.api.sampling.PrioritySampling import datadog.trace.test.agent.decoder.DecodedTrace -abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmokeTest { +abstract class AbstractApmTracingDisabledSmokeTest extends AbstractServerSmokeTest { @Override File createTemporaryFile(int processIndex) { @@ -67,7 +67,7 @@ abstract class AbstractAsmStandaloneBillingSmokeTest extends AbstractServerSmoke } protected hasAppsecPropagationTag(DecodedTrace trace) { - return trace.spans[0].meta['_dd.p.appsec'] == "1" + return trace.spans[0].meta['_dd.p.ts'] == "02" } protected hasApmDisabledTag(DecodedTrace trace) { diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledMatrixSmokeTest.groovy similarity index 86% rename from dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy rename to dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledMatrixSmokeTest.groovy index 4cd1731206d..2e9016ab218 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingMatrixSmokeTest.groovy +++ b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledMatrixSmokeTest.groovy @@ -1,31 +1,31 @@ -package datadog.smoketest.asmstandalonebilling +package datadog.smoketest.apmtracingdisabled import datadog.trace.api.sampling.PrioritySampling import okhttp3.Request -class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { +class ApmTracingDisabledMatrixSmokeTest extends AbstractApmTracingDisabledSmokeTest { - static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-matrix-smoketest-app" - static final String STANDALONE_BILLING_SERVICE_NAME_2 = "asm-standalone-billing-matrix-smoketest-app2" + static final String APM_TRACING_DISABLED_SERVICE_NAME = "apm-tracing-disabled-matrix-smoketest-app" + static final String APM_TRACING_DISABLED_SERVICE_NAME_2 = "apm-tracing-disabled-matrix-smoketest-app2" static final String APM_ENABLED_SERVICE_NAME = "apm-enabled-matrix-smoketest-app" static final String ASM_ENABLED_SERVICE_NAME = "asm-enabled-matrix-smoketest-app" - static final String[] STANDALONE_BILLING_PROPERTIES = [ - "-Ddd.experimental.appsec.standalone.enabled=true", + static final String[] APM_TRACING_DISABLED_PROPERTIES = [ + "-Ddd.apm.tracing.enabled=false", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", + "-Ddd.service.name=${APM_TRACING_DISABLED_SERVICE_NAME}", ] - static final String[] STANDALONE_BILLING_PROPERTIES_2 = [ - "-Ddd.experimental.appsec.standalone.enabled=true", + static final String[] APM_TRACING_DISABLED_PROPERTIES_2 = [ + "-Ddd.apm.tracing.enabled=false", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME_2}", + "-Ddd.service.name=${APM_TRACING_DISABLED_SERVICE_NAME_2}", ] static final String[] APM_ENABLED_PROPERTIES = ["-Ddd.service.name=${APM_ENABLED_SERVICE_NAME}", "-Ddd.trace.tracer.metrics.enabled=true",] @@ -46,13 +46,13 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm ProcessBuilder createProcessBuilder(int processIndex) { switch (processIndex) { case 0: - return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) + return createProcess(processIndex, APM_TRACING_DISABLED_PROPERTIES) case 1: return createProcess(processIndex, APM_ENABLED_PROPERTIES) case 2: return createProcess(processIndex, ASM_ENABLED_PROPERTIES) case 3: - return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES_2) + return createProcess(processIndex, APM_TRACING_DISABLED_PROPERTIES_2) default: throw new IllegalArgumentException("Invalid process index: ${processIndex}") } @@ -82,7 +82,7 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm !hasApmDisabledTag (upstreamTrace) and:"No ASM events, resulting in the local sampling decision" - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) @@ -118,7 +118,7 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm !hasApmDisabledTag (upstreamTrace) and:"ASM events" - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) @@ -154,7 +154,7 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm !hasApmDisabledTag (upstreamTrace) and:"No ASM events, resulting in the local sampling decision" - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) @@ -184,13 +184,13 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: "Upstream standalone ASM service having ASM events result in force keep and propagation of the tag" - def upstreamTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2) + def upstreamTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME_2) checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (upstreamTrace) hasApmDisabledTag (upstreamTrace) and:"standalone service must keep the local trace with the local sampling priority" - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) @@ -224,11 +224,11 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm def upstreamTraceId = getServiceTrace(APM_ENABLED_SERVICE_NAME).spans[0].traceId and: 'No ASM events, resulting in the local sampling decision' - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) isSampledBySampler(standAloneBillingTrace) !hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) - def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + def standAloneBillingTraceId = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME).spans[0].traceId upstreamTraceId == standAloneBillingTraceId //There is propagation and: 'Propagation is stopped' @@ -255,18 +255,18 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm waitForTraceCount(3) and: 'Upstream ASM events' - def upstreamTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2) + def upstreamTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME_2) checkRootSpanPrioritySampling(upstreamTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (upstreamTrace) hasApmDisabledTag (upstreamTrace) - def upstreamTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME_2).spans[0].traceId + def upstreamTraceId = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME_2).spans[0].traceId and: 'No ASM events, resulting in the local sampling decision' - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) - def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + def standAloneBillingTraceId = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME).spans[0].traceId upstreamTraceId == standAloneBillingTraceId //There is propagation and: 'Default APM distributed tracing behavior with' @@ -300,11 +300,11 @@ class AsmStandaloneBillingMatrixSmokeTest extends AbstractAsmStandaloneBillingSm def upstreamTraceId = getServiceTrace(APM_ENABLED_SERVICE_NAME).spans[0].traceId and: 'ASM events, resulting in force keep and appsec propagation' - def standAloneBillingTrace = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME) + def standAloneBillingTrace = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME) checkRootSpanPrioritySampling(standAloneBillingTrace, PrioritySampling.USER_KEEP) hasAppsecPropagationTag (standAloneBillingTrace) hasApmDisabledTag (standAloneBillingTrace) - def standAloneBillingTraceId = getServiceTrace(STANDALONE_BILLING_SERVICE_NAME).spans[0].traceId + def standAloneBillingTraceId = getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME).spans[0].traceId upstreamTraceId == standAloneBillingTraceId //There is propagation and: 'Default APM distributed tracing behavior with' diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSamplingSmokeTest.groovy similarity index 95% rename from dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy rename to dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSamplingSmokeTest.groovy index 27f037d4db1..126e918344c 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSamplingSmokeTest.groovy +++ b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSamplingSmokeTest.groovy @@ -1,4 +1,4 @@ -package datadog.smoketest.asmstandalonebilling +package datadog.smoketest.apmtracingdisabled import datadog.trace.api.sampling.PrioritySampling import datadog.trace.test.util.Flaky @@ -6,17 +6,17 @@ import groovy.json.JsonSlurper import okhttp3.Request @Flaky -class AsmStandaloneBillingSamplingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { +class ApmTracingDisabledSamplingSmokeTest extends AbstractApmTracingDisabledSmokeTest { @Override ProcessBuilder createProcessBuilder(){ final String[] processProperties = [ - "-Ddd.experimental.appsec.standalone.enabled=true", + "-Ddd.apm.tracing.enabled=false", "-Ddd.iast.enabled=true", "-Ddd.appsec.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", - "-Ddd.service.name=asm-standalone-billing-sampling-spring-smoketest-app", + "-Ddd.service.name=apm-tracing-disabled-sampling-spring-smoketest-app", ] return createProcess(processProperties) } diff --git a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSmokeTest.groovy similarity index 80% rename from dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy rename to dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSmokeTest.groovy index f5d7761b676..b84782da595 100644 --- a/dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy +++ b/dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSmokeTest.groovy @@ -1,20 +1,20 @@ -package datadog.smoketest.asmstandalonebilling +package datadog.smoketest.apmtracingdisabled import okhttp3.Request -class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTest { +class ApmTracingDisabledSmokeTest extends AbstractApmTracingDisabledSmokeTest { - private static final String STANDALONE_BILLING_SERVICE_NAME = "asm-standalone-billing-smoketest-app" + private static final String APM_TRACING_DISABLED_SERVICE_NAME = "asm-standalone-billing-smoketest-app" private static final String APM_ENABLED_SERVICE_NAME = "apm-enabled-smoketest-app" - static final String[] STANDALONE_BILLING_PROPERTIES = [ - "-Ddd.experimental.appsec.standalone.enabled=true", + static final String[] APM_TRACING_DISABLED_PROPERTIES = [ + "-Ddd.apm.tracing.enabled=false", "-Ddd.iast.enabled=true", "-Ddd.iast.detection.mode=FULL", "-Ddd.iast.debug.enabled=true", "-Ddd.appsec.enabled=true", "-Ddd.trace.tracer.metrics.enabled=true", - "-Ddd.service.name=${STANDALONE_BILLING_SERVICE_NAME}", + "-Ddd.service.name=${APM_TRACING_DISABLED_SERVICE_NAME}", ] static final String[] APM_ENABLED_PROPERTIES = ["-Ddd.service.name=${APM_ENABLED_SERVICE_NAME}", "-Ddd.trace.tracer.metrics.enabled=true",] @@ -26,7 +26,7 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes @Override ProcessBuilder createProcessBuilder(int processIndex) { if(processIndex == 0){ - return createProcess(processIndex, STANDALONE_BILLING_PROPERTIES) + return createProcess(processIndex, APM_TRACING_DISABLED_PROPERTIES) } return createProcess(processIndex, APM_ENABLED_PROPERTIES) } @@ -46,7 +46,7 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes response1.successful response2.successful waitForTraceCount(2) - hasApmDisabledTag(getServiceTrace(STANDALONE_BILLING_SERVICE_NAME)) + hasApmDisabledTag(getServiceTrace(APM_TRACING_DISABLED_SERVICE_NAME)) !hasApmDisabledTag(getServiceTrace(APM_ENABLED_SERVICE_NAME)) } @@ -68,7 +68,7 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes isLogPresent { it.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled') } } - void 'test _dd.p.appsec propagation for appsec event'() { + void 'test _dd.p.ts propagation for appsec event'() { setup: final downstreamUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings" final url = localUrl + "url=${downstreamUrl}" @@ -77,7 +77,7 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes when: "Request to an endpoint that triggers ASM events and then calls another endpoint" final response1 = client.newCall(request).execute() - then: "Both traces should have a root span with _dd.p.appsec=1 tag" + then: "Both traces should have a root span with _dd.p.ts=02 tag" response1.successful waitForTraceCount(2) assert traces.size() == 2 diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index 1551a419cce..454ef0a5425 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -40,7 +40,5 @@ public final class AppSecConfig { public static final String APPSEC_MAX_STACKTRACE_DEPTH_DEPRECATED = "appsec.max.stacktrace.depth"; // old non-standard as a fallback alias - public static final String APPSEC_STANDALONE_ENABLED = "experimental.appsec.standalone.enabled"; - private AppSecConfig() {} } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java index 524dde00dcb..9a5caf5c4a2 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/GeneralConfig.java @@ -93,6 +93,7 @@ public final class GeneralConfig { "telemetry.dependency.resolution.period.millis"; public static final String AGENTLESS_LOG_SUBMISSION_LEVEL = "agentless.log.submission.level"; public static final String AGENTLESS_LOG_SUBMISSION_URL = "agentless.log.submission.url"; + public static final String APM_TRACING_ENABLED = "apm.tracing.enabled"; private GeneralConfig() {} } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java similarity index 68% rename from dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java rename to dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java index 8638d20205a..f546a289eb0 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java @@ -8,21 +8,20 @@ import org.slf4j.LoggerFactory; /** - * This class is designed to only allow 1 APM trace per minute as standalone ASM is only interested - * in the traces containing ASM events. But the service catalog and the billing need a continuous - * ingestion of at least at 1 trace per minute to consider a service as being live and billable. In - * the absence of ASM events, no APM traces must be sent, so we need to let some regular APM traces - * go through, even in the absence of ASM events. + * This class is designed to only allow 1 APM trace per minute for apm tracing disabled. The service + * catalog and the billing need a continuous ingestion of at least at 1 trace per minute to consider + * a service as being live and billable. In the absence of other products events, no APM traces must + * be sent, so we need to let some regular APM traces go through. */ -public class AsmStandaloneSampler implements Sampler, PrioritySampler { +public class ApmTracingDisabledSampler implements Sampler, PrioritySampler { - private static final Logger log = LoggerFactory.getLogger(AsmStandaloneSampler.class); + private static final Logger log = LoggerFactory.getLogger(ApmTracingDisabledSampler.class); private static final int RATE_IN_MILLISECONDS = 60000; // 1 minute private final AtomicLong lastSampleTime; private final Clock clock; - public AsmStandaloneSampler(final Clock clock) { + public ApmTracingDisabledSampler(final Clock clock) { this.clock = clock; this.lastSampleTime = new AtomicLong(clock.millis() - RATE_IN_MILLISECONDS); } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index 150b67e6b59..e591bfbbe7f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -35,9 +35,9 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { - if (config.isAppSecStandaloneEnabled()) { + if (!config.isApmTracingEnabled()) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new AsmStandaloneSampler(Clock.systemUTC()); + return new ApmTracingDisabledSampler(Clock.systemUTC()); } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index 92af3027b91..645bbc4b9e9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -112,7 +112,7 @@ public Response sendSerializedTraces(final Payload payload) { (metricsEnabled && featuresDiscovery.supportsMetrics()) // Disabling the computation agent-side of the APM trace metrics by // pretending it was already done by the library - || Config.get().isAppSecStandaloneEnabled() + || !Config.get().isApmTracingEnabled() ? "true" : "") .put(payload.toRequest()) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index fcbf046122a..4c207b73def 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -512,8 +512,10 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void updateAppsecPropagation(boolean value) { - propagationTags.updateAppsecPropagation(value); + public void updatePropagatedTraceSource(final Object value) { + if (value instanceof Integer) { + propagationTags.updatePropagatedTraceSource((Integer) value); + } } public void updateDebugPropagation(String value) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java index 260379ad4d3..51949938652 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java @@ -2,6 +2,7 @@ import datadog.trace.api.Config; import datadog.trace.api.DDTraceId; +import datadog.trace.api.ProductTs; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.time.TimeSource; import datadog.trace.bootstrap.instrumentation.api.AgentTraceCollector; @@ -63,9 +64,12 @@ public void setSamplingPriorityIfNecessary() { // Locks inside DDSpanContext ensure the correct behavior in the race case DDSpan rootSpan = getRootSpan(); if (traceConfig.sampler instanceof PrioritySampler && rootSpan != null) { - // Ignore the force-keep priority in the absence of propagated _dd.p.appsec span tag. - if ((Config.get().isAppSecStandaloneEnabled() - && !rootSpan.context().getPropagationTags().isAppsecPropagationEnabled()) + // Ignore the force-keep priority in the absence of propagated _dd.p.ts span tag marked for + // ASM. + if ((!Config.get().isApmTracingEnabled() + && !ProductTs.isProductMarked( + rootSpan.context().getPropagationTags().getPropagatedTraceSource(), + ProductTs.ASM)) || rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index b770ac703c7..240b4411418 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -101,9 +101,9 @@ public interface Factory { public abstract void fillTagMap(Map tagMap); /** Add the appsec propagation tag to the propagation tags. */ - public abstract void updateAppsecPropagation(boolean enabled); + public abstract void updatePropagatedTraceSource(int product); - public abstract boolean isAppsecPropagationEnabled(); + public abstract int getPropagatedTraceSource(); public abstract void updateDebugPropagation(String value); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java index e5731e44a89..8ced6b094cb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java @@ -1,5 +1,6 @@ package datadog.trace.core.propagation.ptags; +import datadog.trace.api.ProductTs; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; import datadog.trace.core.propagation.ptags.TagElement.Encoding; @@ -61,7 +62,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { int tagPos = 0; TagValue decisionMakerTagValue = null; TagValue traceIdTagValue = null; - boolean appsecPropagationEnabled = false; + int traceSource = 0; while (tagPos < len) { int tagKeyEndsAt = validateCharsUntilSeparatorOrEnd( @@ -96,8 +97,8 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { decisionMakerTagValue = tagValue; } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; - } else if (tagKey.equals(APPSEC_TAG)) { - appsecPropagationEnabled = tagValue.charAt(0) == '1'; + } else if (tagKey.equals(TRACE_SOURCE_TAG)) { + traceSource = ProductTs.parseBitfieldHex(tagValue.toString()); } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six @@ -110,8 +111,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } tagPos = tagValueEndsAt + 1; } - return tagsFactory.createValid( - tagPairs, decisionMakerTagValue, traceIdTagValue, appsecPropagationEnabled); + return tagsFactory.createValid(tagPairs, decisionMakerTagValue, traceIdTagValue, traceSource); } @Override diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index 31796025b91..d2b03f014eb 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -2,6 +2,7 @@ import static datadog.trace.core.propagation.ptags.PTagsFactory.PROPAGATION_ERROR_TAG_KEY; +import datadog.trace.api.ProductTs; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; import datadog.trace.core.propagation.ptags.TagElement.Encoding; @@ -15,12 +16,11 @@ abstract class PTagsCodec { protected static final TagKey DECISION_MAKER_TAG = TagKey.from("dm"); protected static final TagKey TRACE_ID_TAG = TagKey.from("tid"); - protected static final TagKey APPSEC_TAG = TagKey.from("appsec"); + protected static final TagKey TRACE_SOURCE_TAG = TagKey.from("ts"); protected static final TagKey DEBUG_TAG = TagKey.from("debug"); protected static final String PROPAGATION_ERROR_MALFORMED_TID = "malformed_tid "; protected static final String PROPAGATION_ERROR_INCONSISTENT_TID = "inconsistent_tid "; protected static final TagKey UPSTREAM_SERVICES_DEPRECATED_TAG = TagKey.from("upstream_services"); - protected static final TagValue APPSEC_ENABLED_TAG_VALUE = TagValue.from("1"); static String headerValue(PTagsCodec codec, PTags ptags) { int estimate = codec.estimateHeaderSize(ptags); @@ -38,8 +38,13 @@ static String headerValue(PTagsCodec codec, PTags ptags) { if (ptags.getTraceIdHighOrderBitsHexTagValue() != null) { size = codec.appendTag(sb, TRACE_ID_TAG, ptags.getTraceIdHighOrderBitsHexTagValue(), size); } - if (ptags.isAppsecPropagationEnabled()) { - size = codec.appendTag(sb, APPSEC_TAG, APPSEC_ENABLED_TAG_VALUE, size); + if (ptags.getPropagatedTraceSource() != 0) { + size = + codec.appendTag( + sb, + TRACE_SOURCE_TAG, + TagValue.from(ProductTs.getBitfieldHex(ptags.getPropagatedTraceSource())), + size); } if (ptags.getDebugPropagation() != null) { size = codec.appendTag(sb, DEBUG_TAG, TagValue.from(ptags.getDebugPropagation()), size); @@ -87,10 +92,12 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { DECISION_MAKER_TAG.forType(Encoding.DATADOG).toString(), propagationTags.getDecisionMakerTagValue().forType(Encoding.DATADOG).toString()); } - if (propagationTags.isAppsecPropagationEnabled()) { + if (propagationTags.getPropagatedTraceSource() != 0) { tagMap.put( - APPSEC_TAG.forType(Encoding.DATADOG).toString(), - APPSEC_ENABLED_TAG_VALUE.forType(Encoding.DATADOG).toString()); + TRACE_SOURCE_TAG.forType(Encoding.DATADOG).toString(), + TagValue.from(ProductTs.getBitfieldHex(propagationTags.getPropagatedTraceSource())) + .forType(Encoding.DATADOG) + .toString()); } if (propagationTags.getDebugPropagation() != null) { tagMap.put( @@ -158,7 +165,7 @@ protected static boolean validateTagValue(TagKey tagKey, TagValue tagValue) { return false; } else if (tagKey.equals(TRACE_ID_TAG) && !validateTraceId(tagValue)) { return false; - } else if (tagKey.equals(APPSEC_TAG) && !validateAppsecTagValue(tagValue)) { + } else if (tagKey.equals(TRACE_SOURCE_TAG) && !validateTraceSourceTagValue(tagValue)) { return false; } return true; @@ -222,8 +229,9 @@ private static boolean validateTraceId(TagValue value) { return true; } - private static boolean validateAppsecTagValue(TagValue value) { - return value.length() == 1 && (value.charAt(0) == '1' || value.charAt(0) == '0'); + private static boolean validateTraceSourceTagValue(TagValue value) { + // Ensure the string is not null, has a length of 2, and matches the hex pattern + return value != null && value.length() == 2 && value.toString().matches("^[0-9A-Fa-f]{2}$"); } protected static boolean isDigit(char c) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index fe111d7ea50..4f8423624c8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -2,11 +2,11 @@ import static datadog.trace.core.propagation.PropagationTags.HeaderType.DATADOG; import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C; -import static datadog.trace.core.propagation.ptags.PTagsCodec.APPSEC_ENABLED_TAG_VALUE; -import static datadog.trace.core.propagation.ptags.PTagsCodec.APPSEC_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.DECISION_MAKER_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; +import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_SOURCE_TAG; +import datadog.trace.api.ProductTs; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; @@ -18,6 +18,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; public class PTagsFactory implements PropagationTags.Factory { @@ -47,7 +48,7 @@ PTagsCodec getDecoderEncoder(@Nonnull HeaderType headerType) { @Override public final PropagationTags empty() { - return createValid(null, null, null, false); + return createValid(null, null, null, 0); } @Override @@ -59,9 +60,8 @@ PropagationTags createValid( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - boolean appsecPropagationEnabled) { - return new PTags( - this, tagPairs, decisionMakerTagValue, traceIdTagValue, appsecPropagationEnabled); + int productTs) { + return new PTags(this, tagPairs, decisionMakerTagValue, traceIdTagValue, productTs); } PropagationTags createInvalid(String error) { @@ -81,7 +81,7 @@ static class PTags extends PropagationTags { // extracted decision maker tag for easier updates private volatile TagValue decisionMakerTagValue; - private volatile boolean appsecPropagationEnabled; + private AtomicInteger productTs; private volatile String debugPropagation; // xDatadogTagsSize of the tagPairs, does not include the decision maker tag @@ -119,13 +119,13 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - boolean appsecPropagationEnabled) { + int productTs) { this( factory, tagPairs, decisionMakerTagValue, traceIdTagValue, - appsecPropagationEnabled, + productTs, PrioritySampling.UNSET, null, null); @@ -136,7 +136,7 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - boolean appsecPropagationEnabled, + int productTs, int samplingPriority, CharSequence origin, CharSequence lastParentId) { @@ -145,7 +145,7 @@ public PTags( this.tagPairs = tagPairs; this.canChangeDecisionMaker = decisionMakerTagValue == null; this.decisionMakerTagValue = decisionMakerTagValue; - this.appsecPropagationEnabled = appsecPropagationEnabled; + this.productTs = new AtomicInteger(productTs); this.samplingPriority = samplingPriority; this.origin = origin; this.lastParentId = lastParentId; @@ -160,7 +160,7 @@ public PTags( } static PTags withError(PTagsFactory factory, String error) { - PTags pTags = new PTags(factory, null, null, null, false, PrioritySampling.UNSET, null, null); + PTags pTags = new PTags(factory, null, null, null, 0, PrioritySampling.UNSET, null, null); pTags.error = error; return pTags; } @@ -204,18 +204,20 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan } @Override - public void updateAppsecPropagation(boolean enabled) { - if (appsecPropagationEnabled != enabled) { + public void updatePropagatedTraceSource(final int product) { + // if is nort marked for the product + if (!ProductTs.isProductMarked(productTs.get(), product)) { // This should invalidate any cached w3c and datadog header clearCachedHeader(DATADOG); clearCachedHeader(W3C); + productTs.updateAndGet( + value -> ProductTs.updateProduct(value, product)); // Set the bit for the given product } - appsecPropagationEnabled = enabled; } @Override - public boolean isAppsecPropagationEnabled() { - return appsecPropagationEnabled; + public int getPropagatedTraceSource() { + return productTs.get(); } @Override @@ -353,8 +355,10 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(getTagPairs()); size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); - if (appsecPropagationEnabled) { - size = PTagsCodec.calcXDatadogTagsSize(size, APPSEC_TAG, APPSEC_ENABLED_TAG_VALUE); + if (productTs.get() != 0) { + size = + PTagsCodec.calcXDatadogTagsSize( + size, TRACE_SOURCE_TAG, TagValue.from(ProductTs.getBitfieldHex(productTs.get()))); } xDatadogTagsSize = size; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index df020a9e53e..47718d8dcf6 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -2,6 +2,7 @@ import static datadog.trace.api.internal.util.LongStringUtils.toHexStringPadded; +import datadog.trace.api.ProductTs; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; @@ -93,7 +94,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { CharSequence origin = null; TagValue decisionMakerTagValue = null; TagValue traceIdTagValue = null; - boolean appsecPropagationEnabled = false; + int traceSource = 0; int maxUnknownSize = 0; CharSequence lastParentId = null; while (tagPos < ddMemberValueEnd) { @@ -147,8 +148,8 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { decisionMakerTagValue = tagValue; } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; - } else if (tagKey.equals(APPSEC_TAG)) { - appsecPropagationEnabled = tagValue.charAt(0) == '1'; + } else if (tagKey.equals(TRACE_SOURCE_TAG)) { + traceSource = ProductTs.parseBitfieldHex(tagValue.toString()); } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six @@ -182,7 +183,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { tagPairs, decisionMakerTagValue, traceIdTagValue, - appsecPropagationEnabled, + traceSource, samplingPriority, origin, value, @@ -707,7 +708,7 @@ private static W3CPTags empty( null, null, null, - false, + 0, PrioritySampling.UNSET, null, original, @@ -739,7 +740,7 @@ public W3CPTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - boolean appsecPropagationEnabled, + int traceSource, int samplingPriority, CharSequence origin, String original, @@ -753,7 +754,7 @@ public W3CPTags( tagPairs, decisionMakerTagValue, traceIdTagValue, - appsecPropagationEnabled, + traceSource, samplingPriority, origin, lastParentId); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 56ae6da4c75..3be491ebe6a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -112,8 +112,8 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return false; case Tags.SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); - case Tags.PROPAGATED_APPSEC: - span.updateAppsecPropagation(asBoolean(value)); + case Tags.PROPAGATED_TRACE_SOURCE: + span.updatePropagatedTraceSource(value); return true; case Tags.PROPAGATED_DEBUG: span.updateDebugPropagation(String.valueOf(value)); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy similarity index 92% rename from dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy rename to dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy index 1d83510ec52..3a1ea329747 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy @@ -7,7 +7,7 @@ import datadog.trace.api.sampling.PrioritySampling import java.time.Clock import java.util.concurrent.atomic.AtomicLong -class AsmStandaloneSamplerTest extends DDCoreSpecification{ +class ApmTracingDisabledSamplerTest extends DDCoreSpecification{ def writer = new ListWriter() @@ -19,7 +19,7 @@ class AsmStandaloneSamplerTest extends DDCoreSpecification{ current.get() } } - def sampler = new AsmStandaloneSampler(clock) + def sampler = new ApmTracingDisabledSampler(clock) def tracer = tracerBuilder().writer(writer).sampler(sampler).build() when: diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy index f231d62a6a5..e8c84d886d2 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy @@ -7,13 +7,13 @@ class SamplerTest extends DDSpecification{ void "test that TimeSampler is selected when experimentalAppSecStandalone is enabled"() { setup: - System.setProperty("dd.experimental.appsec.standalone.enabled", "true") + System.setProperty("dd.apm.tracing.enabled", "false") Config config = new Config() when: Sampler sampler = Sampler.Builder.forConfig(config, null) then: - sampler instanceof AsmStandaloneSampler + sampler instanceof ApmTracingDisabledSampler } } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index caccaf98b4f..2ee47ecc31e 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.core.propagation import datadog.trace.api.Config +import datadog.trace.api.ProductTs import datadog.trace.core.test.DDCoreSpecification import static datadog.trace.api.sampling.PrioritySampling.* @@ -72,9 +73,9 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { "_dd.p.tid=123456789ABCDEF0" | null | ["_dd.propagation_error": "malformed_tid 123456789ABCDEF0"] // invalid tid tag value: upper-case characters "_dd.p.tid=123456789abcdefg" | null | ["_dd.propagation_error": "malformed_tid 123456789abcdefg"] // invalid tid tag value: non-hexadecimal characters "_dd.p.tid=-123456789abcdef" | null | ["_dd.propagation_error": "malformed_tid -123456789abcdef"] // invalid tid tag value: non-hexadecimal characters - "_dd.p.appsec=1" | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] - "_dd.p.appsec=0" | null | [:] - "_dd.p.appsec=foo" | null | ["_dd.propagation_error":"decoding_error"] + "_dd.p.ts=02" | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=00" | null | [:] + "_dd.p.ts=foo" | null | ["_dd.propagation_error":"decoding_error"] } def "datadog propagation tags should translate to w3c tags #headerValue"() { @@ -138,7 +139,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { ",_dd.p.dm=Value" | SAMPLER_KEEP | AGENT_RATE | "_dd.p.dm=-1" | ["_dd.propagation_error": "decoding_error", "_dd.p.dm": "-1"] } - def "update propagation tags appsec propagation #originalTagSet"() { + def "update propagation tags ts propagation #originalTagSet"() { setup: def config = Mock(Config) config.getxDatadogTagsMaxLength() >> 512 @@ -146,20 +147,23 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { def propagationTags = propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, originalTagSet) when: - propagationTags.updateAppsecPropagation(enabled) + propagationTags.updatePropagatedTraceSource(product) then: propagationTags.headerValue(PropagationTags.HeaderType.DATADOG) == expectedHeaderValue propagationTags.createTagMap() == tags where: - originalTagSet | enabled | expectedHeaderValue | tags + originalTagSet | product | expectedHeaderValue | tags // keep the existing dm tag as is - "_dd.p.appsec=1" | true | "_dd.p.appsec=1" | ["_dd.p.appsec": "1"] - "_dd.p.appsec=0" | false | null | [:] - "" | false | null | [:] + "" | ProductTs.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=00" | ProductTs.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=02" | ProductTs.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] //Invalid input - "_dd.p.appsec=foo" | false | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=" | 0 | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=0" | 0 | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=GG" | 0 | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=foo" | 0 | null | ["_dd.propagation_error": "decoding_error"] } def extractionLimitExceeded() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index 3bd0ddfe7b1..1ec9f3e0543 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.core.propagation import datadog.trace.api.Config +import datadog.trace.api.ProductTs import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.sampling.SamplingMechanism import datadog.trace.core.test.DDCoreSpecification @@ -237,9 +238,9 @@ class W3CPropagationTagsTest extends DDCoreSpecification { null | null | [:] '' | null | [:] 'dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] - 'dd=s:0;t.appsec:1' | 'dd=s:0;t.appsec:1' | ['_dd.p.appsec': '1'] - 'dd=s:0;t.appsec:0' | 'dd=s:0' | [:] - 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | 'dd=s:0;t.dm:934086a686-4;t.appsec:1' | ['_dd.p.dm': '934086a686-4', '_dd.p.appsec': '1'] + 'dd=s:0;t.ts:02' | 'dd=s:0;t.ts:02' | ['_dd.p.ts': '02'] + 'dd=s:0;t.ts:00' | 'dd=s:0' | [:] + 'dd=s:0;t.dm:934086a686-4;t.ts:02' | 'dd=s:0;t.dm:934086a686-4;t.ts:02' | ['_dd.p.dm': '934086a686-4', '_dd.p.ts': '02'] 'other=whatever,dd=s:0;t.dm:934086a686-4' | 'dd=s:0;t.dm:934086a686-4,other=whatever' | ['_dd.p.dm': '934086a686-4'] 'dd=s:0;t.dm:934086a687-3,other=whatever' | 'dd=s:0;t.dm:934086a687-3,other=whatever' | ['_dd.p.dm': '934086a687-3'] 'some=thing,dd=s:0;t.dm:934086a687-3,other=whatever' | 'dd=s:0;t.dm:934086a687-3,some=thing,other=whatever' | ['_dd.p.dm': '934086a687-3'] @@ -278,14 +279,15 @@ class W3CPropagationTagsTest extends DDCoreSpecification { propagationTags.createTagMap() == tags where: - headerValue | expectedHeaderValue | tags - 'dd=s:0;t.dm:934086a686-4' | '_dd.p.dm=934086a686-4' | ['_dd.p.dm': '934086a686-4'] - 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~' | '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] - 'dd=s:0;t.appsec:1' | '_dd.p.appsec=1' | ['_dd.p.appsec': '1'] - 'dd=s:0;t.appsec:0' | null | [:] - 'dd=s:0;t.appsec:invalid' | null | [:] - 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~;t.appsec:1' | '_dd.p.dm=934086a686-4,_dd.p.appsec=1,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t==', '_dd.p.appsec': '1'] - 'some=thing,other=whatever' | null | [:] + headerValue | expectedHeaderValue | tags + 'dd=s:0;t.dm:934086a686-4' | '_dd.p.dm=934086a686-4' | ['_dd.p.dm': '934086a686-4'] + 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~' | '_dd.p.dm=934086a686-4,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t=='] + 'dd=s:0;t.ts:02' | '_dd.p.ts=02' | ['_dd.p.ts': '02'] + 'dd=s:0;t.ts:00' | null | [:] + 'dd=s:0;t.ts:0' | null | [:] + 'dd=s:0;t.ts:invalid' | null | [:] + 'other=whatever,dd=s:0;t.dm:934086a686-4;t.f:w00t~~;t.ts:02' | '_dd.p.dm=934086a686-4,_dd.p.ts=02,_dd.p.f=w00t==' | ['_dd.p.dm': '934086a686-4', '_dd.p.f': 'w00t==', '_dd.p.ts': '02'] + 'some=thing,other=whatever' | null | [:] } def "propagation tags should be updated by sampling and origin #headerValue #priority #mechanism #origin"() { @@ -317,7 +319,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { 'dd=s:1;o:some;t.dm:934086a686-4' | PrioritySampling.SAMPLER_DROP | SamplingMechanism.EXTERNAL_OVERRIDE | "other" | 'dd=s:0;o:other' | [:] } - def "propagation tags should be updated by appsec propagation #appsec"() { + def "propagation tags should be updated by product trace source propagation #product"() { setup: def config = Mock(Config) config.getxDatadogTagsMaxLength() >> 512 @@ -330,17 +332,16 @@ class W3CPropagationTagsTest extends DDCoreSpecification { propagationTags.headerValue(HeaderType.W3C) != expectedHeaderValue when: - propagationTags.updateAppsecPropagation(appsec) + propagationTags.updatePropagatedTraceSource(product) then: propagationTags.headerValue(HeaderType.W3C) == expectedHeaderValue propagationTags.createTagMap() == tags where: - headerValue | appsec | expectedHeaderValue | tags - 'dd=t.appsec:1;x:unknown' | false | 'dd=x:unknown' | [:] - 'dd=x:unknown' | true | 'dd=t.appsec:1;x:unknown' | ['_dd.p.appsec': '1'] - 'dd=t.appsec:0;x:unknown' | true | 'dd=t.appsec:1;x:unknown' | ['_dd.p.appsec': '1'] + headerValue | product | expectedHeaderValue | tags + 'dd=x:unknown' | ProductTs.ASM | 'dd=t.ts:02;x:unknown' | ['_dd.p.ts': '02'] + 'dd=t.ts:02;x:unknown' | ProductTs.DBM | 'dd=t.ts:12;x:unknown' | ['_dd.p.ts': '12'] } static private String toLcAlpha(String cs) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 9156ce067bb..1dabab9af6c 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -3,6 +3,7 @@ package datadog.trace.core.taginterceptor import static datadog.trace.api.ConfigDefaults.DEFAULT_SERVICE_NAME import static datadog.trace.api.ConfigDefaults.DEFAULT_SERVLET_ROOT_CONTEXT_SERVICE_NAME import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE +import datadog.trace.api.ProductTs import static datadog.trace.api.config.TracerConfig.SPLIT_BY_TAGS import datadog.trace.api.DDSpanTypes @@ -730,7 +731,7 @@ class TagInterceptorTest extends DDCoreSpecification { "test" | "test" } - void "When intercepts appsec propagation tag addAppsecPropagationTag is called"() { + void "When intercepts product trace source propagation tag updatePropagatedTraceSource is called"() { setup: final ruleFlags = Mock(RuleFlags) ruleFlags.isEnabled(_) >> true @@ -738,9 +739,9 @@ class TagInterceptorTest extends DDCoreSpecification { final context = Mock(DDSpanContext) when: - interceptor.interceptTag(context, Tags.PROPAGATED_APPSEC, true) + interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM) then: - 1 * context.updateAppsecPropagation(true) + 1 * context.updatePropagatedTraceSource(ProductTs.ASM) } } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index c6c4258b3be..6ee4f839830 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -287,7 +287,6 @@ public static String getHostName() { private final boolean appSecStackTraceEnabled; private final int appSecMaxStackTraces; private final int appSecMaxStackTraceDepth; - private final boolean appSecStandaloneEnabled; private final boolean apiSecurityEnabled; private final float apiSecurityRequestSampleRate; @@ -554,6 +553,8 @@ public static String getHostName() { private final long dependecyResolutionPeriodMillis; + private final boolean apmTracingEnabled; + // Read order: System Properties -> Env Variables, [-> properties file], [-> default value] private Config() { this(ConfigProvider.createDefault()); @@ -1292,7 +1293,6 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) configProvider.getStringNotEmpty(APPSEC_AUTO_USER_INSTRUMENTATION_MODE, null), configProvider.getStringNotEmpty(APPSEC_AUTOMATED_USER_EVENTS_TRACKING, null)); appSecScaEnabled = configProvider.getBoolean(APPSEC_SCA_ENABLED); - appSecStandaloneEnabled = configProvider.getBoolean(APPSEC_STANDALONE_ENABLED, false); appSecRaspEnabled = configProvider.getBoolean(APPSEC_RASP_ENABLED, DEFAULT_APPSEC_RASP_ENABLED); appSecStackTraceEnabled = configProvider.getBoolean( @@ -1915,6 +1915,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) "AppSec SCA is enabled but telemetry is disabled. AppSec SCA will not work."); } + this.apmTracingEnabled = configProvider.getBoolean(GeneralConfig.APM_TRACING_ENABLED, true); + log.debug("New instance: {}", this); } @@ -2333,7 +2335,7 @@ public boolean isPerfMetricsEnabled() { public boolean isTracerMetricsEnabled() { // When ASM Standalone Billing is enabled metrics should be disabled - return tracerMetricsEnabled && !isAppSecStandaloneEnabled(); + return tracerMetricsEnabled && isApmTracingEnabled(); } public boolean isTracerMetricsBufferingEnabled() { @@ -3445,6 +3447,10 @@ public String getDataJobsCommandPattern() { return dataJobsCommandPattern; } + public boolean isApmTracingEnabled() { + return apmTracingEnabled; + } + /** @return A map of tags to be applied only to the local application root span. */ public Map getLocalRootSpanTags() { final Map runtimeTags = getRuntimeTags(); @@ -3453,7 +3459,7 @@ public Map getLocalRootSpanTags() { result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version()); result.put(DDTags.PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0); - if (isAppSecStandaloneEnabled()) { + if (!isApmTracingEnabled()) { result.put(APM_ENABLED, 0); } @@ -3636,97 +3642,6 @@ private Map getProcessIdTag() { return Collections.singletonMap(PID_TAG, getProcessId()); } - private Map getAzureAppServicesTags() { - // These variable names and derivations are copied from the dotnet tracer - // See - // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/PlatformHelpers/AzureAppServices.cs - // and - // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/TraceContext.cs#L207 - Map aasTags = new HashMap<>(); - - /// The site name of the site instance in Azure where the traced application is running. - String siteName = getEnv("WEBSITE_SITE_NAME"); - if (siteName != null) { - aasTags.put("aas.site.name", siteName); - } - - // The kind of application instance running in Azure. - // Possible values: app, api, mobileapp, app_linux, app_linux_container, functionapp, - // functionapp_linux, functionapp_linux_container - - // The type of application instance running in Azure. - // Possible values: app, function - if (getEnv("FUNCTIONS_WORKER_RUNTIME") != null - || getEnv("FUNCTIONS_EXTENSIONS_VERSION") != null) { - aasTags.put("aas.site.kind", "functionapp"); - aasTags.put("aas.site.type", "function"); - } else { - aasTags.put("aas.site.kind", "app"); - aasTags.put("aas.site.type", "app"); - } - - // The resource group of the site instance in Azure App Services - String resourceGroup = getEnv("WEBSITE_RESOURCE_GROUP"); - if (resourceGroup != null) { - aasTags.put("aas.resource.group", resourceGroup); - } - - // Example: 8c500027-5f00-400e-8f00-60000000000f+apm-dotnet-EastUSwebspace - // Format: {subscriptionId}+{planResourceGroup}-{hostedInRegion} - String websiteOwner = getEnv("WEBSITE_OWNER_NAME"); - int plusIndex = websiteOwner == null ? -1 : websiteOwner.indexOf("+"); - - // The subscription ID of the site instance in Azure App Services - String subscriptionId = null; - if (plusIndex > 0) { - subscriptionId = websiteOwner.substring(0, plusIndex); - aasTags.put("aas.subscription.id", subscriptionId); - } - - if (subscriptionId != null && siteName != null && resourceGroup != null) { - // The resource ID of the site instance in Azure App Services - String resourceId = - "/subscriptions/" - + subscriptionId - + "/resourcegroups/" - + resourceGroup - + "/providers/microsoft.web/sites/" - + siteName; - resourceId = resourceId.toLowerCase(Locale.ROOT); - aasTags.put("aas.resource.id", resourceId); - } else { - log.warn( - "Unable to generate resource id subscription id: {}, site name: {}, resource group {}", - subscriptionId, - siteName, - resourceGroup); - } - - // The instance ID in Azure - String instanceId = getEnv("WEBSITE_INSTANCE_ID"); - instanceId = instanceId == null ? "unknown" : instanceId; - aasTags.put("aas.environment.instance_id", instanceId); - - // The instance name in Azure - String instanceName = getEnv("COMPUTERNAME"); - instanceName = instanceName == null ? "unknown" : instanceName; - aasTags.put("aas.environment.instance_name", instanceName); - - // The operating system in Azure - String operatingSystem = getEnv("WEBSITE_OS"); - operatingSystem = operatingSystem == null ? "unknown" : operatingSystem; - aasTags.put("aas.environment.os", operatingSystem); - - // The version of the extension installed - String siteExtensionVersion = getEnv("DD_AAS_JAVA_EXTENSION_VERSION"); - siteExtensionVersion = siteExtensionVersion == null ? "unknown" : siteExtensionVersion; - aasTags.put("aas.environment.extension_version", siteExtensionVersion); - - aasTags.put("aas.environment.runtime", getProp("java.vm.name", "unknown")); - - return aasTags; - } - private int schemaVersionFromConfig() { String versionStr = configProvider.getString(TRACE_SPAN_ATTRIBUTE_SCHEMA, "v" + SpanNaming.SCHEMA_MIN_VERSION); @@ -3948,6 +3863,97 @@ public boolean isTelemetryDebugRequestsEnabled() { return telemetryDebugRequestsEnabled; } + private Map getAzureAppServicesTags() { + // These variable names and derivations are copied from the dotnet tracer + // See + // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/PlatformHelpers/AzureAppServices.cs + // and + // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/TraceContext.cs#L207 + Map aasTags = new HashMap<>(); + + /// The site name of the site instance in Azure where the traced application is running. + String siteName = getEnv("WEBSITE_SITE_NAME"); + if (siteName != null) { + aasTags.put("aas.site.name", siteName); + } + + // The kind of application instance running in Azure. + // Possible values: app, api, mobileapp, app_linux, app_linux_container, functionapp, + // functionapp_linux, functionapp_linux_container + + // The type of application instance running in Azure. + // Possible values: app, function + if (getEnv("FUNCTIONS_WORKER_RUNTIME") != null + || getEnv("FUNCTIONS_EXTENSIONS_VERSION") != null) { + aasTags.put("aas.site.kind", "functionapp"); + aasTags.put("aas.site.type", "function"); + } else { + aasTags.put("aas.site.kind", "app"); + aasTags.put("aas.site.type", "app"); + } + + // The resource group of the site instance in Azure App Services + String resourceGroup = getEnv("WEBSITE_RESOURCE_GROUP"); + if (resourceGroup != null) { + aasTags.put("aas.resource.group", resourceGroup); + } + + // Example: 8c500027-5f00-400e-8f00-60000000000f+apm-dotnet-EastUSwebspace + // Format: {subscriptionId}+{planResourceGroup}-{hostedInRegion} + String websiteOwner = getEnv("WEBSITE_OWNER_NAME"); + int plusIndex = websiteOwner == null ? -1 : websiteOwner.indexOf('+'); + + // The subscription ID of the site instance in Azure App Services + String subscriptionId = null; + if (plusIndex > 0) { + subscriptionId = websiteOwner.substring(0, plusIndex); + aasTags.put("aas.subscription.id", subscriptionId); + } + + if (subscriptionId != null && siteName != null && resourceGroup != null) { + // The resource ID of the site instance in Azure App Services + String resourceId = + "/subscriptions/" + + subscriptionId + + "/resourcegroups/" + + resourceGroup + + "/providers/microsoft.web/sites/" + + siteName; + resourceId = resourceId.toLowerCase(Locale.ROOT); + aasTags.put("aas.resource.id", resourceId); + } else { + log.warn( + "Unable to generate resource id subscription id: {}, site name: {}, resource group {}", + subscriptionId, + siteName, + resourceGroup); + } + + // The instance ID in Azure + String instanceId = getEnv("WEBSITE_INSTANCE_ID"); + instanceId = instanceId == null ? "unknown" : instanceId; + aasTags.put("aas.environment.instance_id", instanceId); + + // The instance name in Azure + String instanceName = getEnv("COMPUTERNAME"); + instanceName = instanceName == null ? "unknown" : instanceName; + aasTags.put("aas.environment.instance_name", instanceName); + + // The operating system in Azure + String operatingSystem = getEnv("WEBSITE_OS"); + operatingSystem = operatingSystem == null ? "unknown" : operatingSystem; + aasTags.put("aas.environment.os", operatingSystem); + + // The version of the extension installed + String siteExtensionVersion = getEnv("DD_AAS_JAVA_EXTENSION_VERSION"); + siteExtensionVersion = siteExtensionVersion == null ? "unknown" : siteExtensionVersion; + aasTags.put("aas.environment.extension_version", siteExtensionVersion); + + aasTags.put("aas.environment.runtime", getProp("java.vm.name", "unknown")); + + return aasTags; + } + public boolean isAgentlessLogSubmissionEnabled() { return agentlessLogSubmissionEnabled; } @@ -3972,10 +3978,6 @@ public Boolean getAppSecScaEnabled() { return appSecScaEnabled; } - public boolean isAppSecStandaloneEnabled() { - return appSecStandaloneEnabled; - } - public boolean isAppSecRaspEnabled() { return appSecRaspEnabled; } @@ -4684,8 +4686,8 @@ public String toString() { + dataJobsEnabled + ", dataJobsCommandPattern=" + dataJobsCommandPattern - + ", appSecStandaloneEnabled=" - + appSecStandaloneEnabled + + ", apmTracingEnabled=" + + apmTracingEnabled + ", cloudRequestPayloadTagging=" + cloudRequestPayloadTagging + ", cloudResponsePayloadTagging=" diff --git a/internal-api/src/main/java/datadog/trace/api/ProductTs.java b/internal-api/src/main/java/datadog/trace/api/ProductTs.java new file mode 100644 index 00000000000..a1b1a552725 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/ProductTs.java @@ -0,0 +1,33 @@ +package datadog.trace.api; + +public class ProductTs { + + public static final int APM = 0x01; + public static final int ASM = 0x02; + public static final int DSM = 0x04; + public static final int DJM = 0x08; + public static final int DBM = 0x10; + + // Update (set) the bitfield for a specific product + public static int updateProduct(int bitfield, int product) { + return bitfield |= product; // Set the bit for the given product + } + + // Check if the bitfield is marked for a specific product + public static boolean isProductMarked(final int bitfield, int product) { + return (bitfield & product) != 0; // Check if the bit is set + } + + // Get the current bitfield as a hexadecimal string + public static String getBitfieldHex(final int bitfield) { + return String.format("%02X", bitfield); // Convert to 2-character hex + } + + // Parse a hexadecimal string back to an integer + public static int parseBitfieldHex(final String hexString) { + if (hexString == null || hexString.isEmpty()) { + return 0; // Return 0 if the string is empty + } + return Integer.parseInt(hexString, 16); // Parse the string as a base-16 number + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java index 0c71c9968ab..b21855bdb3e 100644 --- a/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java +++ b/internal-api/src/main/java/datadog/trace/api/sampling/SamplingMechanism.java @@ -69,7 +69,7 @@ public static boolean validateWithSamplingPriority(int mechanism, int priority) * @return */ public static boolean canAvoidSamplingPriorityLock(int priority, int mechanism) { - return Config.get().isAppSecStandaloneEnabled() && mechanism == SamplingMechanism.APPSEC; + return !Config.get().isApmTracingEnabled() && mechanism == SamplingMechanism.APPSEC; } private SamplingMechanism() {} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java index 5c15a92f206..587d70bb93f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/Tags.java @@ -146,6 +146,6 @@ public class Tags { /** ASM force tracer to keep the trace */ public static final String ASM_KEEP = "asm.keep"; - public static final String PROPAGATED_APPSEC = "_dd.p.appsec"; + public static final String PROPAGATED_TRACE_SOURCE = "_dd.p.ts"; public static final String PROPAGATED_DEBUG = "_dd.p.debug"; } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy new file mode 100644 index 00000000000..e4b399bc651 --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy @@ -0,0 +1,60 @@ +package datadog.trace.api + +import datadog.trace.test.util.DDSpecification + +class ProductTsTest extends DDSpecification { + + void 'test updateProduct'(){ + when: + final result = ProductTs.updateProduct(init, newProduct) + + then: + result == expected + + where: + init | newProduct | expected + 0 | ProductTs.ASM | ProductTs.ASM + ProductTs.ASM | ProductTs.DSM | 6 + } + + void 'test isProductMarked'(){ + when: + final result = ProductTs.isProductMarked(value, product) + + then: + result == expected + + where: + value | product | expected + ProductTs.ASM | ProductTs.ASM | true + ProductTs.DSM | ProductTs.ASM | false + } + + void 'test getBitfieldHex'(){ + when: + final result = ProductTs.getBitfieldHex(value) + + then: + result == expected + + where: + value | expected + 0 | "00" + ProductTs.ASM | "02" + } + + void 'test parseBitfieldHex'(){ + when: + final result = ProductTs.parseBitfieldHex(hex) + + then: + result == expected + + where: + hex | expected + "00" | 0 + null | 0 + "" | 0 + "02" | ProductTs.ASM + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy index 60c3634d0e6..4a4890435c1 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/sampling/SamplingMechanismTest.groovy @@ -98,7 +98,7 @@ class SamplingMechanismTest extends DDSpecification { void 'Test canAvoidSamplingPriorityLock'(){ setup: - injectSysConfig("dd.experimental.appsec.standalone.enabled", "true") + injectSysConfig("dd.apm.tracing.enabled", "false") expect: canAvoidSamplingPriorityLock(priority, mechanism) == valid diff --git a/settings.gradle b/settings.gradle index f6082cc6b63..dd5d810b232 100644 --- a/settings.gradle +++ b/settings.gradle @@ -93,7 +93,7 @@ include ':utils:version-utils' // smoke tests include ':dd-smoke-tests:armeria-grpc' -include ':dd-smoke-tests:asm-standalone-billing' +include ':dd-smoke-tests:apm-tracing-disabled' include ':dd-smoke-tests:backend-mock' include ':dd-smoke-tests:cli' include ':dd-smoke-tests:crashtracking' From def470bc05cee2e12e0ac3a1022c3f1bc57dba00 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 4 Feb 2025 12:35:21 +0100 Subject: [PATCH 06/32] rename productTs --- .../main/java/com/datadog/iast/Reporter.java | 4 +- .../com/datadog/iast/ReporterTest.groovy | 10 ++-- .../datadog/appsec/gateway/GatewayBridge.java | 8 +-- .../appsec/powerwaf/PowerWAFModule.java | 6 +- .../gateway/GatewayBridgeSpecification.groovy | 10 ++-- .../datadog/trace/core/TraceCollector.java | 6 +- .../propagation/ptags/DatadogPTagsCodec.java | 4 +- .../core/propagation/ptags/PTagsCodec.java | 7 ++- .../core/propagation/ptags/PTagsFactory.java | 12 ++-- .../core/propagation/ptags/W3CPTagsCodec.java | 4 +- .../DatadogPropagationTagsTest.groovy | 8 +-- .../propagation/W3CPropagationTagsTest.groovy | 6 +- .../taginterceptor/TagInterceptorTest.groovy | 6 +- ...ProductTs.java => ProductTraceSource.java} | 2 +- .../trace/api/ProductTraceSourceTest.groovy | 60 +++++++++++++++++++ .../datadog/trace/api/ProductTsTest.groovy | 60 ------------------- 16 files changed, 110 insertions(+), 103 deletions(-) rename internal-api/src/main/java/datadog/trace/api/{ProductTs.java => ProductTraceSource.java} (97%) create mode 100644 internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy delete mode 100644 internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java index c0c637234b6..6dfb458b768 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/Reporter.java @@ -8,7 +8,7 @@ import com.datadog.iast.model.VulnerabilityBatch; import com.datadog.iast.taint.TaintedObjects; import datadog.trace.api.Config; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.internal.TraceSegment; @@ -126,7 +126,7 @@ private VulnerabilityBatch getOrCreateVulnerabilityBatch(final AgentSpan span) { // TODO: We need to check if we can have an API with more fine-grained semantics on why traces // are kept. segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); return batch; } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy index 59906365f83..27ae2dfffdc 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/ReporterTest.groovy @@ -6,7 +6,7 @@ import com.datadog.iast.model.Vulnerability import com.datadog.iast.model.VulnerabilityBatch import com.datadog.iast.model.VulnerabilityType import datadog.trace.api.Config -import datadog.trace.api.ProductTs +import datadog.trace.api.ProductTraceSource import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.internal.TraceSegment @@ -86,7 +86,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 1 * reqCtx.getOrCreateMetaStructTop('_dd.stack', _) >> { stackTraceBatch } assertStackTrace(stackTraceBatch, v) 0 * _ @@ -136,7 +136,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 0 * _ } @@ -207,7 +207,7 @@ class ReporterTest extends DDSpecification { ] }''', batch.toString(), true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) assertStackTrace(stackTraceBatch, [v1, v2] as Vulnerability[]) 0 * _ } @@ -332,7 +332,7 @@ class ReporterTest extends DDSpecification { 1 * traceSegment.getDataTop('iast') >> null 1 * traceSegment.setDataTop('iast', _ as VulnerabilityBatch) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 1 * traceSegment.setTagTop('_dd.iast.enabled', 1) 1 * reqCtx.getOrCreateMetaStructTop('_dd.stack', _) >> new ConcurrentHashMap<>() 0 * _ diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index 52362448dcb..a21bb0616c3 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -27,7 +27,7 @@ import com.datadog.appsec.report.AppSecEvent; import com.datadog.appsec.report.AppSecEventWrapper; import datadog.trace.api.Config; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.gateway.Events; import datadog.trace.api.gateway.Flow; @@ -215,7 +215,7 @@ private Flow onUser( // span with ASM data segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); // skip event if we have an SDK one if (mode != SDK) { @@ -276,7 +276,7 @@ private Flow onLoginEvent( // span with ASM data segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); + segment.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); // update span tags segment.setTagTop("appsec.events." + eventName + ".track", true, true); @@ -790,7 +790,7 @@ private NoopFlow onRequestEnded(RequestContext ctx_, IGSpanInfo spanInfo) { if (!collectedEvents.isEmpty()) { // Set asm keep in case that root span was not available when events are detected traceSeg.setTagTop(Tags.ASM_KEEP, true); - traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); + traceSeg.setTagTop(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); traceSeg.setTagTop("appsec.event", true); traceSeg.setTagTop("network.client.ip", ctx.getPeerAddress()); diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java index 7648eb496c1..35562832c89 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/powerwaf/PowerWAFModule.java @@ -26,7 +26,7 @@ import datadog.communication.monitor.Monitoring; import datadog.trace.api.Config; import datadog.trace.api.ProductActivation; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.gateway.Flow; import datadog.trace.api.telemetry.LogCollector; import datadog.trace.api.telemetry.WafMetricCollector; @@ -499,7 +499,9 @@ public void onDataAvailable( // If APM is disabled, inform downstream services that the current // distributed trace contains at least one ASM event and must inherit // the given force-keep priority - activeSpan.getLocalRootSpan().setTag(Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM); + activeSpan + .getLocalRootSpan() + .setTag(Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM); } else { // If active span is not available the ASK_KEEP tag will be set in the GatewayBridge // when the request ends diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index 79179c6a8a6..e5285983003 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -8,7 +8,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.ProductTs +import datadog.trace.api.ProductTraceSource import datadog.trace.api.UserIdCollectionMode import datadog.trace.api.appsec.LoginEventCallback import datadog.trace.api.function.TriConsumer @@ -1113,7 +1113,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, true) 1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1152,7 +1152,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, true) 1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1193,7 +1193,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.exists', false, true) 1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> if (mode == SDK) { assert db.get(KnownAddresses.USER_ID) == expectedUser @@ -1222,7 +1222,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * traceSegment.setTagTop('appsec.events.my.event.track', true, true) 1 * traceSegment.setTagTop('appsec.events.my.event', ['key1': 'value1', 'key2': 'value2'], true) 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.ts', ProductTs.ASM) + 1 * traceSegment.setTagTop('_dd.p.ts', ProductTraceSource.ASM) 0 * eventDispatcher.publishDataEvent } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java index 51949938652..af59adc9549 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java @@ -2,7 +2,7 @@ import datadog.trace.api.Config; import datadog.trace.api.DDTraceId; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.time.TimeSource; import datadog.trace.bootstrap.instrumentation.api.AgentTraceCollector; @@ -67,9 +67,9 @@ public void setSamplingPriorityIfNecessary() { // Ignore the force-keep priority in the absence of propagated _dd.p.ts span tag marked for // ASM. if ((!Config.get().isApmTracingEnabled() - && !ProductTs.isProductMarked( + && !ProductTraceSource.isProductMarked( rootSpan.context().getPropagationTags().getPropagatedTraceSource(), - ProductTs.ASM)) + ProductTraceSource.ASM)) || rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java index 8ced6b094cb..f01b3f40005 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java @@ -1,6 +1,6 @@ package datadog.trace.core.propagation.ptags; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; import datadog.trace.core.propagation.ptags.TagElement.Encoding; @@ -98,7 +98,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; } else if (tagKey.equals(TRACE_SOURCE_TAG)) { - traceSource = ProductTs.parseBitfieldHex(tagValue.toString()); + traceSource = ProductTraceSource.parseBitfieldHex(tagValue.toString()); } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index d2b03f014eb..e03761cef5f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -2,7 +2,7 @@ import static datadog.trace.core.propagation.ptags.PTagsFactory.PROPAGATION_ERROR_TAG_KEY; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; import datadog.trace.core.propagation.ptags.TagElement.Encoding; @@ -43,7 +43,7 @@ static String headerValue(PTagsCodec codec, PTags ptags) { codec.appendTag( sb, TRACE_SOURCE_TAG, - TagValue.from(ProductTs.getBitfieldHex(ptags.getPropagatedTraceSource())), + TagValue.from(ProductTraceSource.getBitfieldHex(ptags.getPropagatedTraceSource())), size); } if (ptags.getDebugPropagation() != null) { @@ -95,7 +95,8 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { if (propagationTags.getPropagatedTraceSource() != 0) { tagMap.put( TRACE_SOURCE_TAG.forType(Encoding.DATADOG).toString(), - TagValue.from(ProductTs.getBitfieldHex(propagationTags.getPropagatedTraceSource())) + TagValue.from( + ProductTraceSource.getBitfieldHex(propagationTags.getPropagatedTraceSource())) .forType(Encoding.DATADOG) .toString()); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 4f8423624c8..eb12c19fb0d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -6,7 +6,7 @@ import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_SOURCE_TAG; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; @@ -206,12 +206,14 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan @Override public void updatePropagatedTraceSource(final int product) { // if is nort marked for the product - if (!ProductTs.isProductMarked(productTs.get(), product)) { + if (!ProductTraceSource.isProductMarked(productTs.get(), product)) { // This should invalidate any cached w3c and datadog header clearCachedHeader(DATADOG); clearCachedHeader(W3C); productTs.updateAndGet( - value -> ProductTs.updateProduct(value, product)); // Set the bit for the given product + value -> + ProductTraceSource.updateProduct( + value, product)); // Set the bit for the given product } } @@ -358,7 +360,9 @@ int getXDatadogTagsSize() { if (productTs.get() != 0) { size = PTagsCodec.calcXDatadogTagsSize( - size, TRACE_SOURCE_TAG, TagValue.from(ProductTs.getBitfieldHex(productTs.get()))); + size, + TRACE_SOURCE_TAG, + TagValue.from(ProductTraceSource.getBitfieldHex(productTs.get()))); } xDatadogTagsSize = size; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 47718d8dcf6..632043aa359 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -2,7 +2,7 @@ import static datadog.trace.api.internal.util.LongStringUtils.toHexStringPadded; -import datadog.trace.api.ProductTs; +import datadog.trace.api.ProductTraceSource; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.core.propagation.PropagationTags; import datadog.trace.core.propagation.ptags.PTagsFactory.PTags; @@ -149,7 +149,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { } else if (tagKey.equals(TRACE_ID_TAG)) { traceIdTagValue = tagValue; } else if (tagKey.equals(TRACE_SOURCE_TAG)) { - traceSource = ProductTs.parseBitfieldHex(tagValue.toString()); + traceSource = ProductTraceSource.parseBitfieldHex(tagValue.toString()); } else { if (tagPairs == null) { // This is roughly the size of a two element linked list but can hold six diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index 2ee47ecc31e..c891c2fa997 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -1,7 +1,7 @@ package datadog.trace.core.propagation import datadog.trace.api.Config -import datadog.trace.api.ProductTs +import datadog.trace.api.ProductTraceSource import datadog.trace.core.test.DDCoreSpecification import static datadog.trace.api.sampling.PrioritySampling.* @@ -156,9 +156,9 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { where: originalTagSet | product | expectedHeaderValue | tags // keep the existing dm tag as is - "" | ProductTs.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=00" | ProductTs.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=02" | ProductTs.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] + "" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=00" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=02" | ProductTraceSource.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] //Invalid input "_dd.p.ts=" | 0 | null | ["_dd.propagation_error": "decoding_error"] "_dd.p.ts=0" | 0 | null | ["_dd.propagation_error": "decoding_error"] diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index 1ec9f3e0543..a735ca18323 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -1,7 +1,7 @@ package datadog.trace.core.propagation import datadog.trace.api.Config -import datadog.trace.api.ProductTs +import datadog.trace.api.ProductTraceSource import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.sampling.SamplingMechanism import datadog.trace.core.test.DDCoreSpecification @@ -340,8 +340,8 @@ class W3CPropagationTagsTest extends DDCoreSpecification { where: headerValue | product | expectedHeaderValue | tags - 'dd=x:unknown' | ProductTs.ASM | 'dd=t.ts:02;x:unknown' | ['_dd.p.ts': '02'] - 'dd=t.ts:02;x:unknown' | ProductTs.DBM | 'dd=t.ts:12;x:unknown' | ['_dd.p.ts': '12'] + 'dd=x:unknown' | ProductTraceSource.ASM | 'dd=t.ts:02;x:unknown' | ['_dd.p.ts': '02'] + 'dd=t.ts:02;x:unknown' | ProductTraceSource.DBM | 'dd=t.ts:12;x:unknown' | ['_dd.p.ts': '12'] } static private String toLcAlpha(String cs) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 1dabab9af6c..2b9d4b9c3cf 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -3,7 +3,7 @@ package datadog.trace.core.taginterceptor import static datadog.trace.api.ConfigDefaults.DEFAULT_SERVICE_NAME import static datadog.trace.api.ConfigDefaults.DEFAULT_SERVLET_ROOT_CONTEXT_SERVICE_NAME import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE -import datadog.trace.api.ProductTs +import datadog.trace.api.ProductTraceSource import static datadog.trace.api.config.TracerConfig.SPLIT_BY_TAGS import datadog.trace.api.DDSpanTypes @@ -739,9 +739,9 @@ class TagInterceptorTest extends DDCoreSpecification { final context = Mock(DDSpanContext) when: - interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTs.ASM) + interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) then: - 1 * context.updatePropagatedTraceSource(ProductTs.ASM) + 1 * context.updatePropagatedTraceSource(ProductTraceSource.ASM) } } diff --git a/internal-api/src/main/java/datadog/trace/api/ProductTs.java b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java similarity index 97% rename from internal-api/src/main/java/datadog/trace/api/ProductTs.java rename to internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java index a1b1a552725..7400081efb1 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProductTs.java +++ b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java @@ -1,6 +1,6 @@ package datadog.trace.api; -public class ProductTs { +public class ProductTraceSource { public static final int APM = 0x01; public static final int ASM = 0x02; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy new file mode 100644 index 00000000000..0bf4617a0a7 --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy @@ -0,0 +1,60 @@ +package datadog.trace.api + +import datadog.trace.test.util.DDSpecification + +class ProductTraceSourceTest extends DDSpecification { + + void 'test updateProduct'(){ + when: + final result = ProductTraceSource.updateProduct(init, newProduct) + + then: + result == expected + + where: + init | newProduct | expected + 0 | ProductTraceSource.ASM | ProductTraceSource.ASM + ProductTraceSource.ASM | ProductTraceSource.DSM | 6 + } + + void 'test isProductMarked'(){ + when: + final result = ProductTraceSource.isProductMarked(value, product) + + then: + result == expected + + where: + value | product | expected + ProductTraceSource.ASM | ProductTraceSource.ASM | true + ProductTraceSource.DSM | ProductTraceSource.ASM | false + } + + void 'test getBitfieldHex'(){ + when: + final result = ProductTraceSource.getBitfieldHex(value) + + then: + result == expected + + where: + value | expected + 0 | "00" + ProductTraceSource.ASM | "02" + } + + void 'test parseBitfieldHex'(){ + when: + final result = ProductTraceSource.parseBitfieldHex(hex) + + then: + result == expected + + where: + hex | expected + "00" | 0 + null | 0 + "" | 0 + "02" | ProductTraceSource.ASM + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy deleted file mode 100644 index e4b399bc651..00000000000 --- a/internal-api/src/test/groovy/datadog/trace/api/ProductTsTest.groovy +++ /dev/null @@ -1,60 +0,0 @@ -package datadog.trace.api - -import datadog.trace.test.util.DDSpecification - -class ProductTsTest extends DDSpecification { - - void 'test updateProduct'(){ - when: - final result = ProductTs.updateProduct(init, newProduct) - - then: - result == expected - - where: - init | newProduct | expected - 0 | ProductTs.ASM | ProductTs.ASM - ProductTs.ASM | ProductTs.DSM | 6 - } - - void 'test isProductMarked'(){ - when: - final result = ProductTs.isProductMarked(value, product) - - then: - result == expected - - where: - value | product | expected - ProductTs.ASM | ProductTs.ASM | true - ProductTs.DSM | ProductTs.ASM | false - } - - void 'test getBitfieldHex'(){ - when: - final result = ProductTs.getBitfieldHex(value) - - then: - result == expected - - where: - value | expected - 0 | "00" - ProductTs.ASM | "02" - } - - void 'test parseBitfieldHex'(){ - when: - final result = ProductTs.parseBitfieldHex(hex) - - then: - result == expected - - where: - hex | expected - "00" | 0 - null | 0 - "" | 0 - "02" | ProductTs.ASM - } -} From e05d1add7e950ec3eb84eb8577febf7fc393bf18 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 4 Feb 2025 13:11:36 +0100 Subject: [PATCH 07/32] rename productTs in PTagsFactory --- .../core/propagation/ptags/PTagsFactory.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index eb12c19fb0d..29c5d9ec4a8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -81,7 +81,7 @@ static class PTags extends PropagationTags { // extracted decision maker tag for easier updates private volatile TagValue decisionMakerTagValue; - private AtomicInteger productTs; + private final AtomicInteger productTraceSource; private volatile String debugPropagation; // xDatadogTagsSize of the tagPairs, does not include the decision maker tag @@ -119,13 +119,13 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - int productTs) { + int productTraceSource) { this( factory, tagPairs, decisionMakerTagValue, traceIdTagValue, - productTs, + productTraceSource, PrioritySampling.UNSET, null, null); @@ -136,7 +136,7 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - int productTs, + int productTraceSource, int samplingPriority, CharSequence origin, CharSequence lastParentId) { @@ -145,7 +145,7 @@ public PTags( this.tagPairs = tagPairs; this.canChangeDecisionMaker = decisionMakerTagValue == null; this.decisionMakerTagValue = decisionMakerTagValue; - this.productTs = new AtomicInteger(productTs); + this.productTraceSource = new AtomicInteger(productTraceSource); this.samplingPriority = samplingPriority; this.origin = origin; this.lastParentId = lastParentId; @@ -206,11 +206,11 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan @Override public void updatePropagatedTraceSource(final int product) { // if is nort marked for the product - if (!ProductTraceSource.isProductMarked(productTs.get(), product)) { + if (!ProductTraceSource.isProductMarked(productTraceSource.get(), product)) { // This should invalidate any cached w3c and datadog header clearCachedHeader(DATADOG); clearCachedHeader(W3C); - productTs.updateAndGet( + productTraceSource.updateAndGet( value -> ProductTraceSource.updateProduct( value, product)); // Set the bit for the given product @@ -219,7 +219,7 @@ public void updatePropagatedTraceSource(final int product) { @Override public int getPropagatedTraceSource() { - return productTs.get(); + return productTraceSource.get(); } @Override @@ -357,12 +357,12 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(getTagPairs()); size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); - if (productTs.get() != 0) { + if (productTraceSource.get() != 0) { size = PTagsCodec.calcXDatadogTagsSize( size, TRACE_SOURCE_TAG, - TagValue.from(ProductTraceSource.getBitfieldHex(productTs.get()))); + TagValue.from(ProductTraceSource.getBitfieldHex(productTraceSource.get()))); } xDatadogTagsSize = size; } From 7fe3d345dea77db2a67d08f40b2de33c9e6a1b76 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 6 Feb 2025 13:10:49 +0100 Subject: [PATCH 08/32] Fix AtomicInteger wrong use --- .../core/propagation/ptags/PTagsFactory.java | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 29c5d9ec4a8..ae8325fd941 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -205,16 +205,20 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan @Override public void updatePropagatedTraceSource(final int product) { - // if is nort marked for the product - if (!ProductTraceSource.isProductMarked(productTraceSource.get(), product)) { - // This should invalidate any cached w3c and datadog header - clearCachedHeader(DATADOG); - clearCachedHeader(W3C); - productTraceSource.updateAndGet( - value -> - ProductTraceSource.updateProduct( - value, product)); // Set the bit for the given product - } + productTraceSource.updateAndGet( + currentValue -> { + // If the product is already marked, return the same value (no change) + if (ProductTraceSource.isProductMarked(currentValue, product)) { + return currentValue; + } + + // Invalidate cached headers (atomic context ensures correctness) + clearCachedHeader(DATADOG); + clearCachedHeader(W3C); + + // Set the bit for the given product + return ProductTraceSource.updateProduct(currentValue, product); + }); } @Override @@ -357,12 +361,13 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(getTagPairs()); size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); - if (productTraceSource.get() != 0) { + int currentProductTraceSource = productTraceSource.get(); + if (currentProductTraceSource != 0) { size = PTagsCodec.calcXDatadogTagsSize( size, TRACE_SOURCE_TAG, - TagValue.from(ProductTraceSource.getBitfieldHex(productTraceSource.get()))); + TagValue.from(ProductTraceSource.getBitfieldHex(currentProductTraceSource))); } xDatadogTagsSize = size; } From dc2155cb48422670ba6a288dc4d73207d105cf7e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 11:00:22 +0100 Subject: [PATCH 09/32] remove unnecessary refactor --- .../main/java/datadog/trace/api/Config.java | 182 +++++++++--------- 1 file changed, 91 insertions(+), 91 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 6ee4f839830..e6441d3cbdf 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -3642,6 +3642,97 @@ private Map getProcessIdTag() { return Collections.singletonMap(PID_TAG, getProcessId()); } + private Map getAzureAppServicesTags() { + // These variable names and derivations are copied from the dotnet tracer + // See + // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/PlatformHelpers/AzureAppServices.cs + // and + // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/TraceContext.cs#L207 + Map aasTags = new HashMap<>(); + + /// The site name of the site instance in Azure where the traced application is running. + String siteName = getEnv("WEBSITE_SITE_NAME"); + if (siteName != null) { + aasTags.put("aas.site.name", siteName); + } + + // The kind of application instance running in Azure. + // Possible values: app, api, mobileapp, app_linux, app_linux_container, functionapp, + // functionapp_linux, functionapp_linux_container + + // The type of application instance running in Azure. + // Possible values: app, function + if (getEnv("FUNCTIONS_WORKER_RUNTIME") != null + || getEnv("FUNCTIONS_EXTENSIONS_VERSION") != null) { + aasTags.put("aas.site.kind", "functionapp"); + aasTags.put("aas.site.type", "function"); + } else { + aasTags.put("aas.site.kind", "app"); + aasTags.put("aas.site.type", "app"); + } + + // The resource group of the site instance in Azure App Services + String resourceGroup = getEnv("WEBSITE_RESOURCE_GROUP"); + if (resourceGroup != null) { + aasTags.put("aas.resource.group", resourceGroup); + } + + // Example: 8c500027-5f00-400e-8f00-60000000000f+apm-dotnet-EastUSwebspace + // Format: {subscriptionId}+{planResourceGroup}-{hostedInRegion} + String websiteOwner = getEnv("WEBSITE_OWNER_NAME"); + int plusIndex = websiteOwner == null ? -1 : websiteOwner.indexOf("+"); + + // The subscription ID of the site instance in Azure App Services + String subscriptionId = null; + if (plusIndex > 0) { + subscriptionId = websiteOwner.substring(0, plusIndex); + aasTags.put("aas.subscription.id", subscriptionId); + } + + if (subscriptionId != null && siteName != null && resourceGroup != null) { + // The resource ID of the site instance in Azure App Services + String resourceId = + "/subscriptions/" + + subscriptionId + + "/resourcegroups/" + + resourceGroup + + "/providers/microsoft.web/sites/" + + siteName; + resourceId = resourceId.toLowerCase(Locale.ROOT); + aasTags.put("aas.resource.id", resourceId); + } else { + log.warn( + "Unable to generate resource id subscription id: {}, site name: {}, resource group {}", + subscriptionId, + siteName, + resourceGroup); + } + + // The instance ID in Azure + String instanceId = getEnv("WEBSITE_INSTANCE_ID"); + instanceId = instanceId == null ? "unknown" : instanceId; + aasTags.put("aas.environment.instance_id", instanceId); + + // The instance name in Azure + String instanceName = getEnv("COMPUTERNAME"); + instanceName = instanceName == null ? "unknown" : instanceName; + aasTags.put("aas.environment.instance_name", instanceName); + + // The operating system in Azure + String operatingSystem = getEnv("WEBSITE_OS"); + operatingSystem = operatingSystem == null ? "unknown" : operatingSystem; + aasTags.put("aas.environment.os", operatingSystem); + + // The version of the extension installed + String siteExtensionVersion = getEnv("DD_AAS_JAVA_EXTENSION_VERSION"); + siteExtensionVersion = siteExtensionVersion == null ? "unknown" : siteExtensionVersion; + aasTags.put("aas.environment.extension_version", siteExtensionVersion); + + aasTags.put("aas.environment.runtime", getProp("java.vm.name", "unknown")); + + return aasTags; + } + private int schemaVersionFromConfig() { String versionStr = configProvider.getString(TRACE_SPAN_ATTRIBUTE_SCHEMA, "v" + SpanNaming.SCHEMA_MIN_VERSION); @@ -3863,97 +3954,6 @@ public boolean isTelemetryDebugRequestsEnabled() { return telemetryDebugRequestsEnabled; } - private Map getAzureAppServicesTags() { - // These variable names and derivations are copied from the dotnet tracer - // See - // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/PlatformHelpers/AzureAppServices.cs - // and - // https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/TraceContext.cs#L207 - Map aasTags = new HashMap<>(); - - /// The site name of the site instance in Azure where the traced application is running. - String siteName = getEnv("WEBSITE_SITE_NAME"); - if (siteName != null) { - aasTags.put("aas.site.name", siteName); - } - - // The kind of application instance running in Azure. - // Possible values: app, api, mobileapp, app_linux, app_linux_container, functionapp, - // functionapp_linux, functionapp_linux_container - - // The type of application instance running in Azure. - // Possible values: app, function - if (getEnv("FUNCTIONS_WORKER_RUNTIME") != null - || getEnv("FUNCTIONS_EXTENSIONS_VERSION") != null) { - aasTags.put("aas.site.kind", "functionapp"); - aasTags.put("aas.site.type", "function"); - } else { - aasTags.put("aas.site.kind", "app"); - aasTags.put("aas.site.type", "app"); - } - - // The resource group of the site instance in Azure App Services - String resourceGroup = getEnv("WEBSITE_RESOURCE_GROUP"); - if (resourceGroup != null) { - aasTags.put("aas.resource.group", resourceGroup); - } - - // Example: 8c500027-5f00-400e-8f00-60000000000f+apm-dotnet-EastUSwebspace - // Format: {subscriptionId}+{planResourceGroup}-{hostedInRegion} - String websiteOwner = getEnv("WEBSITE_OWNER_NAME"); - int plusIndex = websiteOwner == null ? -1 : websiteOwner.indexOf('+'); - - // The subscription ID of the site instance in Azure App Services - String subscriptionId = null; - if (plusIndex > 0) { - subscriptionId = websiteOwner.substring(0, plusIndex); - aasTags.put("aas.subscription.id", subscriptionId); - } - - if (subscriptionId != null && siteName != null && resourceGroup != null) { - // The resource ID of the site instance in Azure App Services - String resourceId = - "/subscriptions/" - + subscriptionId - + "/resourcegroups/" - + resourceGroup - + "/providers/microsoft.web/sites/" - + siteName; - resourceId = resourceId.toLowerCase(Locale.ROOT); - aasTags.put("aas.resource.id", resourceId); - } else { - log.warn( - "Unable to generate resource id subscription id: {}, site name: {}, resource group {}", - subscriptionId, - siteName, - resourceGroup); - } - - // The instance ID in Azure - String instanceId = getEnv("WEBSITE_INSTANCE_ID"); - instanceId = instanceId == null ? "unknown" : instanceId; - aasTags.put("aas.environment.instance_id", instanceId); - - // The instance name in Azure - String instanceName = getEnv("COMPUTERNAME"); - instanceName = instanceName == null ? "unknown" : instanceName; - aasTags.put("aas.environment.instance_name", instanceName); - - // The operating system in Azure - String operatingSystem = getEnv("WEBSITE_OS"); - operatingSystem = operatingSystem == null ? "unknown" : operatingSystem; - aasTags.put("aas.environment.os", operatingSystem); - - // The version of the extension installed - String siteExtensionVersion = getEnv("DD_AAS_JAVA_EXTENSION_VERSION"); - siteExtensionVersion = siteExtensionVersion == null ? "unknown" : siteExtensionVersion; - aasTags.put("aas.environment.extension_version", siteExtensionVersion); - - aasTags.put("aas.environment.runtime", getProp("java.vm.name", "unknown")); - - return aasTags; - } - public boolean isAgentlessLogSubmissionEnabled() { return agentlessLogSubmissionEnabled; } From 2f82fc477deeb181679d9d7e2047e39a636848ef Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 12:03:16 +0100 Subject: [PATCH 10/32] keep it alphabetically --- settings.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle b/settings.gradle index dd5d810b232..54bdd9e80dc 100644 --- a/settings.gradle +++ b/settings.gradle @@ -92,8 +92,8 @@ include ':utils:time-utils' include ':utils:version-utils' // smoke tests -include ':dd-smoke-tests:armeria-grpc' include ':dd-smoke-tests:apm-tracing-disabled' +include ':dd-smoke-tests:armeria-grpc' include ':dd-smoke-tests:backend-mock' include ':dd-smoke-tests:cli' include ':dd-smoke-tests:crashtracking' From b2d49223c7a51bbf6e957af53eeb61865b641f49 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 12:11:15 +0100 Subject: [PATCH 11/32] Add meaningful constant UNSET to ProductTraceSource 0 --- .../trace/core/propagation/ptags/PTagsCodec.java | 4 ++-- .../java/datadog/trace/api/ProductTraceSource.java | 2 ++ .../datadog/trace/api/ProductTraceSourceTest.groovy | 10 +++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index e03761cef5f..986b11b97f4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -38,7 +38,7 @@ static String headerValue(PTagsCodec codec, PTags ptags) { if (ptags.getTraceIdHighOrderBitsHexTagValue() != null) { size = codec.appendTag(sb, TRACE_ID_TAG, ptags.getTraceIdHighOrderBitsHexTagValue(), size); } - if (ptags.getPropagatedTraceSource() != 0) { + if (ptags.getPropagatedTraceSource() != ProductTraceSource.UNSET) { size = codec.appendTag( sb, @@ -92,7 +92,7 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { DECISION_MAKER_TAG.forType(Encoding.DATADOG).toString(), propagationTags.getDecisionMakerTagValue().forType(Encoding.DATADOG).toString()); } - if (propagationTags.getPropagatedTraceSource() != 0) { + if (propagationTags.getPropagatedTraceSource() != ProductTraceSource.UNSET) { tagMap.put( TRACE_SOURCE_TAG.forType(Encoding.DATADOG).toString(), TagValue.from( diff --git a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java index 7400081efb1..bad21902ef9 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java +++ b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java @@ -2,6 +2,8 @@ public class ProductTraceSource { + public static final int UNSET = 0; + public static final int APM = 0x01; public static final int ASM = 0x02; public static final int DSM = 0x04; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy index 0bf4617a0a7..bc376154217 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ProductTraceSourceTest.groovy @@ -13,7 +13,7 @@ class ProductTraceSourceTest extends DDSpecification { where: init | newProduct | expected - 0 | ProductTraceSource.ASM | ProductTraceSource.ASM + ProductTraceSource.UNSET | ProductTraceSource.ASM | ProductTraceSource.ASM ProductTraceSource.ASM | ProductTraceSource.DSM | 6 } @@ -39,7 +39,7 @@ class ProductTraceSourceTest extends DDSpecification { where: value | expected - 0 | "00" + ProductTraceSource.UNSET | "00" ProductTraceSource.ASM | "02" } @@ -52,9 +52,9 @@ class ProductTraceSourceTest extends DDSpecification { where: hex | expected - "00" | 0 - null | 0 - "" | 0 + "00" | ProductTraceSource.UNSET + null | ProductTraceSource.UNSET + "" | ProductTraceSource.UNSET "02" | ProductTraceSource.ASM } } From 8bdff199e21887c066800d88ba1d9a93479eb293 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 12:20:45 +0100 Subject: [PATCH 12/32] Improve javadoc --- .../core/propagation/PropagationTags.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index 240b4411418..46a053f3529 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -3,6 +3,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_X_DATADOG_TAGS_MAX_LENGTH; import datadog.trace.api.Config; +import datadog.trace.api.ProductTraceSource; import datadog.trace.core.propagation.ptags.PTagsFactory; import java.util.HashMap; import java.util.Map; @@ -100,9 +101,27 @@ public interface Factory { */ public abstract void fillTagMap(Map tagMap); - /** Add the appsec propagation tag to the propagation tags. */ + /** + * Updates the propagated trace source propagation tag to include the specified product. + * + *

The product value is parsed and interpreted according to the logic in {@link + * ProductTraceSource}. This method ensures that the given product is marked in the propagated + * trace source. + * + * @param product the product identifier to be added to the propagated trace source. Refer to + * {@link ProductTraceSource} for details on how the value is interpreted. + */ public abstract void updatePropagatedTraceSource(int product); + /** + * Retrieves the current propagated trace source propagation tag. + * + *

The returned value is an encoded bitfield that represents the included products. To + * understand how this value is parsed and interpreted, refer to {@link ProductTraceSource}. + * + * @return the propagated trace source as an integer bitfield. See {@link ProductTraceSource} for + * details on its structure and usage. + */ public abstract int getPropagatedTraceSource(); public abstract void updateDebugPropagation(String value); From 13aaa1e184c7aa5cda7cd44b0e4c6c4d4e4f32a4 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 10 Feb 2025 13:05:44 +0100 Subject: [PATCH 13/32] remove deprecated datadog.trace.api.sampling.PrioritySampling --- .../trace/common/sampling/ApmTracingDisabledSampler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java index f546a289eb0..62dd39200d8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java @@ -1,5 +1,8 @@ package datadog.trace.common.sampling; +import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP; +import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; + import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.core.CoreSpan; import java.time.Clock; @@ -38,10 +41,10 @@ public > void setSamplingPriority(final T span) { if (shouldSample()) { log.debug("Set SAMPLER_KEEP for span {}", span.getSpanId()); - span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP, SamplingMechanism.APPSEC); + span.setSamplingPriority(SAMPLER_KEEP, SamplingMechanism.APPSEC); } else { log.debug("Set SAMPLER_DROP for span {}", span.getSpanId()); - span.setSamplingPriority(PrioritySampling.SAMPLER_DROP, SamplingMechanism.APPSEC); + span.setSamplingPriority(SAMPLER_DROP, SamplingMechanism.APPSEC); } } From 32f7b6e95b6a2017bed0b9a2b75fa6dc53db2b4d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 07:29:56 +0100 Subject: [PATCH 14/32] lowercase the trace source tag value --- .../datadog/trace/core/propagation/ptags/PTagsCodec.java | 5 ++++- .../src/main/java/datadog/trace/api/ProductTraceSource.java | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index 986b11b97f4..48a03bcd729 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -232,7 +232,10 @@ private static boolean validateTraceId(TagValue value) { private static boolean validateTraceSourceTagValue(TagValue value) { // Ensure the string is not null, has a length of 2, and matches the hex pattern - return value != null && value.length() == 2 && value.toString().matches("^[0-9A-Fa-f]{2}$"); + return value != null + && value.length() == 2 + && isHexDigit(value.charAt(0)) + && isHexDigit(value.charAt(1)); } protected static boolean isDigit(char c) { diff --git a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java index bad21902ef9..7933609a5b2 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java +++ b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java @@ -22,7 +22,7 @@ public static boolean isProductMarked(final int bitfield, int product) { // Get the current bitfield as a hexadecimal string public static String getBitfieldHex(final int bitfield) { - return String.format("%02X", bitfield); // Convert to 2-character hex + return String.format("%02x", bitfield); // Convert to 2-character hex } // Parse a hexadecimal string back to an integer From 31f9ae012ab6c4be601fe968c2e321558f04a493 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 08:08:08 +0100 Subject: [PATCH 15/32] refactor to traceSource and addTraceSource move cast to TagInterceptor instead of doing it on DDSpanContext --- .../datadog/trace/core/DDSpanContext.java | 6 ++---- .../datadog/trace/core/TraceCollector.java | 3 +-- .../core/propagation/PropagationTags.java | 20 +++++++++---------- .../core/propagation/ptags/PTagsCodec.java | 9 ++++----- .../core/propagation/ptags/PTagsFactory.java | 20 +++++++++---------- .../core/taginterceptor/TagInterceptor.java | 7 +++++-- .../DatadogPropagationTagsTest.groovy | 20 +++++++++---------- .../propagation/W3CPropagationTagsTest.groovy | 2 +- 8 files changed, 43 insertions(+), 44 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 4c207b73def..67b0f20bf00 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -512,10 +512,8 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void updatePropagatedTraceSource(final Object value) { - if (value instanceof Integer) { - propagationTags.updatePropagatedTraceSource((Integer) value); - } + public void updatePropagatedTraceSource(final int value) { + propagationTags.addTraceSource(value); } public void updateDebugPropagation(String value) { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java index af59adc9549..777fc3889bf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/TraceCollector.java @@ -68,8 +68,7 @@ public void setSamplingPriorityIfNecessary() { // ASM. if ((!Config.get().isApmTracingEnabled() && !ProductTraceSource.isProductMarked( - rootSpan.context().getPropagationTags().getPropagatedTraceSource(), - ProductTraceSource.ASM)) + rootSpan.context().getPropagationTags().getTraceSource(), ProductTraceSource.ASM)) || rootSpan.context().getSamplingPriority() == PrioritySampling.UNSET) { ((PrioritySampler) traceConfig.sampler).setSamplingPriority(rootSpan); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index 46a053f3529..7d8ad25a44d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -102,27 +102,27 @@ public interface Factory { public abstract void fillTagMap(Map tagMap); /** - * Updates the propagated trace source propagation tag to include the specified product. + * Updates the trace source to include the specified product. * *

The product value is parsed and interpreted according to the logic in {@link - * ProductTraceSource}. This method ensures that the given product is marked in the propagated - * trace source. + * ProductTraceSource}. This method ensures that the given product is marked as part of the trace + * source. * - * @param product the product identifier to be added to the propagated trace source. Refer to - * {@link ProductTraceSource} for details on how the value is interpreted. + * @param product the product identifier to be added to the trace source. Refer to {@link + * ProductTraceSource} for details on how the value is interpreted. */ - public abstract void updatePropagatedTraceSource(int product); + public abstract void addTraceSource(int product); /** - * Retrieves the current propagated trace source propagation tag. + * Retrieves the current trace source. * *

The returned value is an encoded bitfield that represents the included products. To * understand how this value is parsed and interpreted, refer to {@link ProductTraceSource}. * - * @return the propagated trace source as an integer bitfield. See {@link ProductTraceSource} for - * details on its structure and usage. + * @return the trace source as an integer bitfield. See {@link ProductTraceSource} for details on + * its structure and usage. */ - public abstract int getPropagatedTraceSource(); + public abstract int getTraceSource(); public abstract void updateDebugPropagation(String value); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index 48a03bcd729..eb083f054c7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -38,12 +38,12 @@ static String headerValue(PTagsCodec codec, PTags ptags) { if (ptags.getTraceIdHighOrderBitsHexTagValue() != null) { size = codec.appendTag(sb, TRACE_ID_TAG, ptags.getTraceIdHighOrderBitsHexTagValue(), size); } - if (ptags.getPropagatedTraceSource() != ProductTraceSource.UNSET) { + if (ptags.getTraceSource() != ProductTraceSource.UNSET) { size = codec.appendTag( sb, TRACE_SOURCE_TAG, - TagValue.from(ProductTraceSource.getBitfieldHex(ptags.getPropagatedTraceSource())), + TagValue.from(ProductTraceSource.getBitfieldHex(ptags.getTraceSource())), size); } if (ptags.getDebugPropagation() != null) { @@ -92,11 +92,10 @@ static void fillTagMap(PTags propagationTags, Map tagMap) { DECISION_MAKER_TAG.forType(Encoding.DATADOG).toString(), propagationTags.getDecisionMakerTagValue().forType(Encoding.DATADOG).toString()); } - if (propagationTags.getPropagatedTraceSource() != ProductTraceSource.UNSET) { + if (propagationTags.getTraceSource() != ProductTraceSource.UNSET) { tagMap.put( TRACE_SOURCE_TAG.forType(Encoding.DATADOG).toString(), - TagValue.from( - ProductTraceSource.getBitfieldHex(propagationTags.getPropagatedTraceSource())) + TagValue.from(ProductTraceSource.getBitfieldHex(propagationTags.getTraceSource())) .forType(Encoding.DATADOG) .toString()); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index ae8325fd941..d5ca6dfc43f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -81,7 +81,7 @@ static class PTags extends PropagationTags { // extracted decision maker tag for easier updates private volatile TagValue decisionMakerTagValue; - private final AtomicInteger productTraceSource; + private final AtomicInteger traceSource; private volatile String debugPropagation; // xDatadogTagsSize of the tagPairs, does not include the decision maker tag @@ -119,13 +119,13 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - int productTraceSource) { + int traceSource) { this( factory, tagPairs, decisionMakerTagValue, traceIdTagValue, - productTraceSource, + traceSource, PrioritySampling.UNSET, null, null); @@ -136,7 +136,7 @@ public PTags( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - int productTraceSource, + int traceSource, int samplingPriority, CharSequence origin, CharSequence lastParentId) { @@ -145,7 +145,7 @@ public PTags( this.tagPairs = tagPairs; this.canChangeDecisionMaker = decisionMakerTagValue == null; this.decisionMakerTagValue = decisionMakerTagValue; - this.productTraceSource = new AtomicInteger(productTraceSource); + this.traceSource = new AtomicInteger(traceSource); this.samplingPriority = samplingPriority; this.origin = origin; this.lastParentId = lastParentId; @@ -204,8 +204,8 @@ public void updateTraceSamplingPriority(int samplingPriority, int samplingMechan } @Override - public void updatePropagatedTraceSource(final int product) { - productTraceSource.updateAndGet( + public void addTraceSource(final int product) { + traceSource.updateAndGet( currentValue -> { // If the product is already marked, return the same value (no change) if (ProductTraceSource.isProductMarked(currentValue, product)) { @@ -222,8 +222,8 @@ public void updatePropagatedTraceSource(final int product) { } @Override - public int getPropagatedTraceSource() { - return productTraceSource.get(); + public int getTraceSource() { + return traceSource.get(); } @Override @@ -361,7 +361,7 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(getTagPairs()); size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); - int currentProductTraceSource = productTraceSource.get(); + int currentProductTraceSource = traceSource.get(); if (currentProductTraceSource != 0) { size = PTagsCodec.calcXDatadogTagsSize( diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 3be491ebe6a..09c568dca30 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -113,8 +113,11 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { case Tags.SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); case Tags.PROPAGATED_TRACE_SOURCE: - span.updatePropagatedTraceSource(value); - return true; + if (value instanceof Integer) { + span.updatePropagatedTraceSource((Integer) value); + return true; + } + return false; case Tags.PROPAGATED_DEBUG: span.updateDebugPropagation(String.valueOf(value)); return true; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index c891c2fa997..764ea24de78 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -75,7 +75,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { "_dd.p.tid=-123456789abcdef" | null | ["_dd.propagation_error": "malformed_tid -123456789abcdef"] // invalid tid tag value: non-hexadecimal characters "_dd.p.ts=02" | "_dd.p.ts=02" | ["_dd.p.ts": "02"] "_dd.p.ts=00" | null | [:] - "_dd.p.ts=foo" | null | ["_dd.propagation_error":"decoding_error"] + "_dd.p.ts=foo" | null | ["_dd.propagation_error": "decoding_error"] } def "datadog propagation tags should translate to w3c tags #headerValue"() { @@ -147,23 +147,23 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { def propagationTags = propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, originalTagSet) when: - propagationTags.updatePropagatedTraceSource(product) + propagationTags.addTraceSource(product) then: propagationTags.headerValue(PropagationTags.HeaderType.DATADOG) == expectedHeaderValue propagationTags.createTagMap() == tags where: - originalTagSet | product | expectedHeaderValue | tags + originalTagSet | product | expectedHeaderValue | tags // keep the existing dm tag as is - "" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=00" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=02" | ProductTraceSource.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] + "" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=00" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=02" | ProductTraceSource.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] //Invalid input - "_dd.p.ts=" | 0 | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=0" | 0 | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=GG" | 0 | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=foo" | 0 | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=0" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=GG" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=foo" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] } def extractionLimitExceeded() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index a735ca18323..f65859922e2 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -332,7 +332,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { propagationTags.headerValue(HeaderType.W3C) != expectedHeaderValue when: - propagationTags.updatePropagatedTraceSource(product) + propagationTags.addTraceSource(product) then: propagationTags.headerValue(HeaderType.W3C) == expectedHeaderValue From 50763f1ea864715407dac1725b7829b3ea6e2db8 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 08:20:29 +0100 Subject: [PATCH 16/32] fix rebased compilation errors --- .../src/main/java/datadog/trace/core/CoreTracer.java | 12 ++++++------ ...agator.java => ApmTracionDisabledPropagator.java} | 11 ++++++----- 2 files changed, 12 insertions(+), 11 deletions(-) rename dd-trace-core/src/main/java/datadog/trace/core/propagation/{StandaloneAsmPropagator.java => ApmTracionDisabledPropagator.java} (78%) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 4b3d2e50b4a..730f8b9a82f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -90,7 +90,7 @@ import datadog.trace.core.propagation.ExtractedContext; import datadog.trace.core.propagation.HttpCodec; import datadog.trace.core.propagation.PropagationTags; -import datadog.trace.core.propagation.StandaloneAsmPropagator; +import datadog.trace.core.propagation.ApmTracionDisabledPropagator; import datadog.trace.core.propagation.TracingPropagator; import datadog.trace.core.propagation.XRayPropagator; import datadog.trace.core.scopemanager.ContinuableScopeManager; @@ -721,14 +721,14 @@ private CoreTracer( HttpCodec.Extractor tracingExtractor = extractor == null ? HttpCodec.createExtractor(config, this::captureTraceConfig) : extractor; TracingPropagator tracingPropagator = new TracingPropagator(injector, tracingExtractor); - // Check if standalone AppSec is enabled: - // If enabled, use the standalone AppSec propagator by default that will limit tracing concern + // Check if apm tracing is disabled: + // If disabled, use the APM tracing disabled propagator by default that will limit tracing concern // injection and delegate to the tracing propagator if needed, // If disabled, the most common case, use the usual tracing propagator by default. - boolean standaloneAppSec = config.isAppSecStandaloneEnabled(); + boolean apmTracingDisabled = !config.isApmTracingEnabled(); boolean dsm = config.isDataStreamsEnabled(); - Propagators.register(STANDALONE_ASM_CONCERN, new StandaloneAsmPropagator(), standaloneAppSec); - Propagators.register(TRACING_CONCERN, tracingPropagator, !standaloneAppSec); + Propagators.register(STANDALONE_ASM_CONCERN, new ApmTracionDisabledPropagator(), apmTracingDisabled); + Propagators.register(TRACING_CONCERN, tracingPropagator, !apmTracingDisabled); Propagators.register(XRAY_TRACING_CONCERN, new XRayPropagator(config), false); if (dsm) { Propagators.register(DSM_CONCERN, this.dataStreamsMonitoring.propagator()); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/StandaloneAsmPropagator.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java similarity index 78% rename from dd-trace-core/src/main/java/datadog/trace/core/propagation/StandaloneAsmPropagator.java rename to dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java index fc208c5de80..5693172afd5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/StandaloneAsmPropagator.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java @@ -8,33 +8,34 @@ import datadog.context.propagation.CarrierVisitor; import datadog.context.propagation.Propagator; import datadog.context.propagation.Propagators; +import datadog.trace.api.ProductTraceSource; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; import datadog.trace.core.DDSpanContext; import javax.annotation.ParametersAreNonnullByDefault; @ParametersAreNonnullByDefault -public class StandaloneAsmPropagator implements Propagator { +public class ApmTracionDisabledPropagator implements Propagator { @Override public void inject(Context context, C carrier, CarrierSetter setter) { - // Stop propagation if appsec propagation is disabled (no ASM events), stop propagation + // Stop propagation if no product events, stop propagation AgentSpan span; if ((span = fromContext(context)) != null) { AgentSpanContext spanContext = span.context(); if (spanContext instanceof DDSpanContext) { DDSpanContext ddSpanContext = (DDSpanContext) spanContext; - if (!ddSpanContext.getPropagationTags().isAppsecPropagationEnabled()) { + if (ddSpanContext.getPropagationTags().getTraceSource() == ProductTraceSource.UNSET) { return; } } - // Only propagate tracing for appsec standalone product + // Only propagate tracing if trace source set Propagators.forConcern(TRACING_CONCERN).inject(span, carrier, setter); } } @Override public Context extract(Context context, C carrier, CarrierVisitor visitor) { - // Only propagate tracing for appsec standalone product + // Only propagate tracing if trace source set return Propagators.forConcern(TRACING_CONCERN).extract(context, carrier, visitor); } } From fed37e5a56a5712c5fb9c1d0b8c8a0914dd32f85 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 09:08:23 +0100 Subject: [PATCH 17/32] fix spotless --- .../src/main/java/datadog/trace/core/CoreTracer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 730f8b9a82f..32c31760cbc 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -86,6 +86,7 @@ import datadog.trace.core.monitor.HealthMetrics; import datadog.trace.core.monitor.MonitoringImpl; import datadog.trace.core.monitor.TracerHealthMetrics; +import datadog.trace.core.propagation.ApmTracionDisabledPropagator; import datadog.trace.core.propagation.CorePropagation; import datadog.trace.core.propagation.ExtractedContext; import datadog.trace.core.propagation.HttpCodec; @@ -722,7 +723,8 @@ private CoreTracer( extractor == null ? HttpCodec.createExtractor(config, this::captureTraceConfig) : extractor; TracingPropagator tracingPropagator = new TracingPropagator(injector, tracingExtractor); // Check if apm tracing is disabled: - // If disabled, use the APM tracing disabled propagator by default that will limit tracing concern + // If disabled, use the APM tracing disabled propagator by default that will limit tracing + // concern // injection and delegate to the tracing propagator if needed, // If disabled, the most common case, use the usual tracing propagator by default. boolean apmTracingDisabled = !config.isApmTracingEnabled(); From 7f326328f5a66b2db6ddd21282d182d691ea2a0e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 09:47:03 +0100 Subject: [PATCH 18/32] fixTracingPropagatorTest --- .../core/propagation/TracingPropagatorTest.groovy | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/TracingPropagatorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/TracingPropagatorTest.groovy index 7c26cbc7e16..20a6ff8942d 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/TracingPropagatorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/TracingPropagatorTest.groovy @@ -157,9 +157,9 @@ class TracingPropagatorTest extends DDCoreSpecification { tracer.close() } - def 'test ASM standalone billing propagator stop propagation'() { + def 'test APM Tracing disabled propagator stop propagation'() { setup: - injectSysConfig('experimental.appsec.standalone.enabled', standaloneAsmEnabled.toString()) + injectSysConfig('apm.tracing.enabled', apmTracingEnabled.toString()) def tracer = tracerBuilder().build() def span = tracer.buildSpan('test', 'operation').start() def setter = Mock(CarrierSetter) @@ -169,10 +169,10 @@ class TracingPropagatorTest extends DDCoreSpecification { Propagators.defaultPropagator().inject(span, carrier, setter) then: - if (standaloneAsmEnabled) { - 0 * setter.set(_, _, _) - } else { + if (apmTracingEnabled) { (1.._) * setter.set(_, _, _) + } else { + 0 * setter.set(_, _, _) } cleanup: @@ -180,6 +180,6 @@ class TracingPropagatorTest extends DDCoreSpecification { tracer.close() where: - standaloneAsmEnabled << [true, false] + apmTracingEnabled << [true, false] } } From 497c568f6073005e2a7a8017ebcbf886ccca61d1 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 10:14:15 +0100 Subject: [PATCH 19/32] more meaningful constant changes and renaming Ts with TraceSource --- .../core/propagation/ptags/PTagsFactory.java | 19 ++++++++++++++----- .../core/propagation/ptags/W3CPTagsCodec.java | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index d5ca6dfc43f..942fa5b2821 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -48,7 +48,7 @@ PTagsCodec getDecoderEncoder(@Nonnull HeaderType headerType) { @Override public final PropagationTags empty() { - return createValid(null, null, null, 0); + return createValid(null, null, null, ProductTraceSource.UNSET); } @Override @@ -60,8 +60,8 @@ PropagationTags createValid( List tagPairs, TagValue decisionMakerTagValue, TagValue traceIdTagValue, - int productTs) { - return new PTags(this, tagPairs, decisionMakerTagValue, traceIdTagValue, productTs); + int productTraceSource) { + return new PTags(this, tagPairs, decisionMakerTagValue, traceIdTagValue, productTraceSource); } PropagationTags createInvalid(String error) { @@ -160,7 +160,16 @@ public PTags( } static PTags withError(PTagsFactory factory, String error) { - PTags pTags = new PTags(factory, null, null, null, 0, PrioritySampling.UNSET, null, null); + PTags pTags = + new PTags( + factory, + null, + null, + null, + ProductTraceSource.UNSET, + PrioritySampling.UNSET, + null, + null); pTags.error = error; return pTags; } @@ -362,7 +371,7 @@ int getXDatadogTagsSize() { size = PTagsCodec.calcXDatadogTagsSize(size, DECISION_MAKER_TAG, decisionMakerTagValue); size = PTagsCodec.calcXDatadogTagsSize(size, TRACE_ID_TAG, traceIdHighOrderBitsHexTagValue); int currentProductTraceSource = traceSource.get(); - if (currentProductTraceSource != 0) { + if (currentProductTraceSource != ProductTraceSource.UNSET) { size = PTagsCodec.calcXDatadogTagsSize( size, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 632043aa359..3b99aa1aa62 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -94,7 +94,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { CharSequence origin = null; TagValue decisionMakerTagValue = null; TagValue traceIdTagValue = null; - int traceSource = 0; + int traceSource = ProductTraceSource.UNSET; int maxUnknownSize = 0; CharSequence lastParentId = null; while (tagPos < ddMemberValueEnd) { @@ -708,7 +708,7 @@ private static W3CPTags empty( null, null, null, - 0, + ProductTraceSource.UNSET, PrioritySampling.UNSET, null, original, From 2d4965b4a8229a7e6b76edcb36215f3658aa52b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Tue, 11 Feb 2025 10:17:12 +0100 Subject: [PATCH 20/32] Update dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy Co-authored-by: Bruce Bujon --- .../trace/core/propagation/DatadogPropagationTagsTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index 764ea24de78..636b8b412b8 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -139,7 +139,7 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { ",_dd.p.dm=Value" | SAMPLER_KEEP | AGENT_RATE | "_dd.p.dm=-1" | ["_dd.propagation_error": "decoding_error", "_dd.p.dm": "-1"] } - def "update propagation tags ts propagation #originalTagSet"() { + def "update propagation tags trace source propagation #originalTagSet"() { setup: def config = Mock(Config) config.getxDatadogTagsMaxLength() >> 512 From a0edcc0d7a35e27c08493283afecf804beda9dc3 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 11:58:37 +0100 Subject: [PATCH 21/32] typo --- ...isabledPropagator.java => ApmTracingDisabledPropagator.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename dd-trace-core/src/main/java/datadog/trace/core/propagation/{ApmTracionDisabledPropagator.java => ApmTracingDisabledPropagator.java} (96%) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracingDisabledPropagator.java similarity index 96% rename from dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java rename to dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracingDisabledPropagator.java index 5693172afd5..8dfe988d6a6 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracionDisabledPropagator.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ApmTracingDisabledPropagator.java @@ -15,7 +15,7 @@ import javax.annotation.ParametersAreNonnullByDefault; @ParametersAreNonnullByDefault -public class ApmTracionDisabledPropagator implements Propagator { +public class ApmTracingDisabledPropagator implements Propagator { @Override public void inject(Context context, C carrier, CarrierSetter setter) { // Stop propagation if no product events, stop propagation From b7c425d227deb73b95d07fcea9a2dc10c8e3fe30 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 12:11:23 +0100 Subject: [PATCH 22/32] change decision maker to default for ApmTracingDisabledSampler --- .../trace/common/sampling/ApmTracingDisabledSampler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java index 62dd39200d8..18b9c233417 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java @@ -41,10 +41,10 @@ public > void setSamplingPriority(final T span) { if (shouldSample()) { log.debug("Set SAMPLER_KEEP for span {}", span.getSpanId()); - span.setSamplingPriority(SAMPLER_KEEP, SamplingMechanism.APPSEC); + span.setSamplingPriority(SAMPLER_KEEP, SamplingMechanism.DEFAULT); } else { log.debug("Set SAMPLER_DROP for span {}", span.getSpanId()); - span.setSamplingPriority(SAMPLER_DROP, SamplingMechanism.APPSEC); + span.setSamplingPriority(SAMPLER_DROP, SamplingMechanism.DEFAULT); } } From 3dc792685b50e7f51fb384a881ce40679b1b60bf Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 13:27:43 +0100 Subject: [PATCH 23/32] Revert "fix muzzle and check previous latest version" This reverts commit 3ec5d83684c3ab208c4d9b85d36e046c803db9fc. --- .../instrumentation/org-json/build.gradle | 16 +------------ .../json/JSONArrayInstrumentation.java | 5 ---- .../json/JSONCookieInstrumentation.java | 5 ---- ...ONObjectAfter20250107Instrumentation.java} | 14 ++++------- ...NObjectBefore20250107Instrumentation.java} | 24 +++++-------------- .../json/JSONTokenerInstrumentation.java | 5 ---- 6 files changed, 12 insertions(+), 57 deletions(-) rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObject20241224Instrumentation.java => JSONObjectAfter20250107Instrumentation.java} (90%) rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObjectInstrumentation.java => JSONObjectBefore20250107Instrumentation.java} (73%) diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index 07bb471caa7..6bce025b3d6 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -1,25 +1,10 @@ muzzle { pass { - name = 'all' group = 'org.json' module = 'json' versions = '[20070829, ]' assertInverse = true } - pass { - name = 'before_20241224' - group = 'org.json' - module = 'json' - versions = '[20070829, 20241224)' - assertInverse = true - } - pass { - name = 'after_20241224' - group = 'org.json' - module = 'json' - versions = '[20241224, ]' - assertInverse = true - } } apply from: "$rootDir/gradle/java.gradle" @@ -33,5 +18,6 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') testRuntimeOnly project(':dd-java-agent:instrumentation:java-io') //Needed for Reader + //FIXME: ASM latestDepTestImplementation group: 'org.json', name: 'json', version: '+' } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 869de8beeb8..1ad5895dfb1 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -24,11 +24,6 @@ public JSONArrayInstrumentation() { super("org-json"); } - @Override - public String muzzleDirective() { - return "all"; - } - @Override public String instrumentedType() { return "org.json.JSONArray"; diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java index 26e332f9956..f9a25abb04d 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONCookieInstrumentation.java @@ -19,11 +19,6 @@ public JSONCookieInstrumentation() { super("org-json"); } - @Override - public String muzzleDirective() { - return "all"; - } - @Override public String instrumentedType() { return "org.json.Cookie"; diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java similarity index 90% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java index 5cb96ad7ffa..c8f74ad02fd 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObject20241224Instrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java @@ -20,23 +20,19 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) -public class JSONObject20241224Instrumentation extends InstrumenterModule.Iast +public class JSONObjectAfter20250107Instrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObject20241224Instrumentation() { + public JSONObjectAfter20250107Instrumentation() { super("org-json"); } - static final ElementMatcher.Junction AFTER_20241224 = + // Avoid matching servlet 3 which has its own instrumentation + static final ElementMatcher.Junction AFTER_20250107 = hasClassNamed("org.json.StringBuilderWriter"); - @Override - public String muzzleDirective() { - return "after_20241224"; - } - @Override public ElementMatcher.Junction classLoaderMatcher() { - return AFTER_20241224; + return AFTER_20250107; } @Override diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java similarity index 73% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java index dabee96e25c..bac52b01b54 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java @@ -7,7 +7,6 @@ import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -16,28 +15,23 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; -import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumenterModule.class) -public class JSONObjectInstrumentation extends InstrumenterModule.Iast +public class JSONObjectBefore20250107Instrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectInstrumentation() { + public JSONObjectBefore20250107Instrumentation() { super("org-json"); } - static final ElementMatcher.Junction BEFORE_20241224 = + // Avoid matching servlet 3 which has its own instrumentation + static final ElementMatcher.Junction BEFORE_20250107 = not(hasClassNamed("org.json.StringBuilderWriter")); @Override public ElementMatcher.Junction classLoaderMatcher() { - return BEFORE_20241224; - } - - @Override - public String muzzleDirective() { - return "before_20241224"; + return BEFORE_20250107; } @Override @@ -47,14 +41,8 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { - // public JSONObject(JSONTokener x) - transformer.applyAdvice( - isConstructor().and(takesArguments(1)).and(takesArgument(0, named("org.json.JSONTokener"))), - getClass().getName() + "$ConstructorAdvice"); - // private JSONObject(Map m) transformer.applyAdvice( - isConstructor().and(takesArguments(1)).and(takesArgument(0, Map.class)), - getClass().getName() + "$ConstructorAdvice"); + isConstructor().and(takesArguments(1)), getClass().getName() + "$ConstructorAdvice"); transformer.applyAdvice( isMethod() .and(isPublic()) diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java index b4fb20a64c3..e0126f8a76b 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java @@ -25,11 +25,6 @@ public String instrumentedType() { return "org.json.JSONTokener"; } - @Override - public String muzzleDirective() { - return "all"; - } - @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( From 6d00e1e18dd1ca11068879ed9a44a758783b530d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 13:27:44 +0100 Subject: [PATCH 24/32] Revert "add new instrumentation and cover more constructors" This reverts commit 5dc237b54dce05a0d8131f579c2f9672676ef9c5. --- .../iastinstrumenter/iast_exclusion.trie | 2 - .../java/lang/InputStreamReaderCallSite.java | 1 - .../io/InputStreamReaderCallSiteTest.groovy | 9 +-- .../foo/bar/TestInputStreamReaderSuite.java | 4 - .../instrumentation/org-json/build.gradle | 1 - .../json/JSONArrayInstrumentation.java | 27 ++++++- ...SONObjectAfter20250107Instrumentation.java | 80 ------------------- ...on.java => JSONObjectInstrumentation.java} | 40 ++++++---- .../json/JSONTokenerInstrumentation.java | 3 +- .../trace/instrumentation/json/OptAdvice.java | 28 ------- .../JSONCookieInstrumentationTest.groovy | 3 +- .../JSONObjectInstrumentationTest.groovy | 67 +--------------- .../JSONTokenerInstrumentationTest.groovy | 27 ++----- 13 files changed, 61 insertions(+), 231 deletions(-) delete mode 100644 dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java rename dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/{JSONObjectBefore20250107Instrumentation.java => JSONObjectInstrumentation.java} (66%) delete mode 100644 dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java diff --git a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie index 699a84adcaf..e3ef2739905 100644 --- a/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie +++ b/dd-java-agent/instrumentation/iast-instrumenter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie @@ -256,8 +256,6 @@ 1 org.jooq.* 1 org.jruby.* 1 org.json.* -#Needed for JSON propagation -2 org.json.JSONTokener 1 org.jsoup.* 1 org.junit.* 1 org.jvnet.hk2.* diff --git a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java index 444879d51b5..e78db178697 100644 --- a/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java +++ b/dd-java-agent/instrumentation/java-io/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamReaderCallSite.java @@ -12,7 +12,6 @@ @CallSite(spi = IastCallSites.class) public class InputStreamReaderCallSite { - @CallSite.After("void java.io.InputStreamReader.(java.io.InputStream)") @CallSite.After( "void java.io.InputStreamReader.(java.io.InputStream, java.nio.charset.Charset)") public static InputStreamReader afterInit( diff --git a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy index 5dce907e0df..7b5a3e568ff 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-io/src/test/groovy/datadog/trace/instrumentation/java/io/InputStreamReaderCallSiteTest.groovy @@ -14,17 +14,10 @@ class InputStreamReaderCallSiteTest extends BaseIoCallSiteTest{ InstrumentationBridge.registerIastModule(iastModule) when: - TestInputStreamReaderSuite.init(*args) + TestInputStreamReaderSuite.init(new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()) then: 1 * iastModule.taintObjectIfTainted(_ as InputStreamReader, _ as InputStream) 0 * _ - - where: - args << [ - [new ByteArrayInputStream("test".getBytes()), Charset.defaultCharset()], - // InputStream input - [new ByteArrayInputStream("test".getBytes())]// Reader input - ] } } diff --git a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java index 1d79a1a7b9b..57c572575a5 100644 --- a/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java +++ b/dd-java-agent/instrumentation/java-io/src/test/java/foo/bar/TestInputStreamReaderSuite.java @@ -9,8 +9,4 @@ public class TestInputStreamReaderSuite { public static InputStreamReader init(final InputStream in, Charset charset) { return new InputStreamReader(in, charset); } - - public static InputStreamReader init(final InputStream in) { - return new InputStreamReader(in); - } } diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index 6bce025b3d6..a9526cd5d9d 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -16,7 +16,6 @@ dependencies { testImplementation group: 'org.json', name: 'json', version: '20230227' testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') - testRuntimeOnly project(':dd-java-agent:instrumentation:java-io') //Needed for Reader //FIXME: ASM latestDepTestImplementation group: 'org.json', name: 'json', version: '+' diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 1ad5895dfb1..7bd4f6ebc64 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -5,7 +5,6 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -15,6 +14,8 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; +import org.json.JSONArray; +import org.json.JSONObject; @AutoService(InstrumenterModule.class) public class JSONArrayInstrumentation extends InstrumenterModule.Iast @@ -32,11 +33,11 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(0)).and(takesArgument(0, named("org.json.JSONTokener"))), + isConstructor().and(takesArguments(String.class)), getClass().getName() + "$ConstructorAdvice"); transformer.applyAdvice( isMethod().and(isPublic()).and(returns(Object.class)).and(named("opt")), - packageName + ".OptAdvice"); + getClass().getName() + "$OptAdvice"); } public static class ConstructorAdvice { @@ -49,4 +50,24 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } + + public static class OptAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } + } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java deleted file mode 100644 index c8f74ad02fd..00000000000 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectAfter20250107Instrumentation.java +++ /dev/null @@ -1,80 +0,0 @@ -package datadog.trace.instrumentation.json; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isConstructor; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Propagation; -import datadog.trace.api.iast.propagation.PropagationModule; -import java.util.Map; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(InstrumenterModule.class) -public class JSONObjectAfter20250107Instrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectAfter20250107Instrumentation() { - super("org-json"); - } - - // Avoid matching servlet 3 which has its own instrumentation - static final ElementMatcher.Junction AFTER_20250107 = - hasClassNamed("org.json.StringBuilderWriter"); - - @Override - public ElementMatcher.Junction classLoaderMatcher() { - return AFTER_20250107; - } - - @Override - public String instrumentedType() { - return "org.json.JSONObject"; - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - // public JSONObject(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) - transformer.applyAdvice( - isConstructor() - .and(takesArguments(2)) - .and(takesArgument(0, named("org.json.JSONTokener"))) - .and(takesArgument(1, named("org.json.JSONParserConfiguration"))), - getClass().getName() + "$ConstructorAdvice"); - // private JSONObject(Map m, int recursionDepth, JSONParserConfiguration - // jsonParserConfiguration) - transformer.applyAdvice( - isConstructor() - .and(takesArguments(3)) - .and(takesArgument(0, Map.class)) - .and(takesArgument(1, int.class)) - .and(takesArgument(2, named("org.json.JSONParserConfiguration"))), - getClass().getName() + "$ConstructorAdvice"); - transformer.applyAdvice( - isMethod() - .and(isPublic()) - .and(returns(Object.class)) - .and(named("opt")) - .and(takesArguments(String.class)), - packageName + ".OptAdvice"); - } - - public static class ConstructorAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final Object input) { - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null && input != null) { - iastModule.taintObjectIfTainted(self, input); - } - } - } -} diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java similarity index 66% rename from dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java rename to dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index bac52b01b54..9746eda1fd5 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectBefore20250107Instrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -1,11 +1,9 @@ package datadog.trace.instrumentation.json; -import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -16,24 +14,16 @@ import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; import net.bytebuddy.asm.Advice; -import net.bytebuddy.matcher.ElementMatcher; +import org.json.JSONArray; +import org.json.JSONObject; @AutoService(InstrumenterModule.class) -public class JSONObjectBefore20250107Instrumentation extends InstrumenterModule.Iast +public class JSONObjectInstrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - public JSONObjectBefore20250107Instrumentation() { + public JSONObjectInstrumentation() { super("org-json"); } - // Avoid matching servlet 3 which has its own instrumentation - static final ElementMatcher.Junction BEFORE_20250107 = - not(hasClassNamed("org.json.StringBuilderWriter")); - - @Override - public ElementMatcher.Junction classLoaderMatcher() { - return BEFORE_20250107; - } - @Override public String instrumentedType() { return "org.json.JSONObject"; @@ -49,7 +39,7 @@ public void methodAdvice(MethodTransformer transformer) { .and(returns(Object.class)) .and(named("opt")) .and(takesArguments(String.class)), - packageName + ".OptAdvice"); + getClass().getName() + "$OptAdvice"); } public static class ConstructorAdvice { @@ -62,4 +52,24 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } } + + public static class OptAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } + } } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java index e0126f8a76b..eb51d248770 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONTokenerInstrumentation.java @@ -9,7 +9,6 @@ import datadog.trace.api.iast.InstrumentationBridge; import datadog.trace.api.iast.Propagation; import datadog.trace.api.iast.propagation.PropagationModule; -import java.io.Reader; import net.bytebuddy.asm.Advice; @AutoService(InstrumenterModule.class) @@ -28,7 +27,7 @@ public String instrumentedType() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isConstructor().and(takesArguments(Reader.class)), + isConstructor().and(takesArguments(String.class)), getClass().getName() + "$ConstructorAdvice"); } diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java deleted file mode 100644 index 4e40cb0d098..00000000000 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/OptAdvice.java +++ /dev/null @@ -1,28 +0,0 @@ -package datadog.trace.instrumentation.json; - -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Propagation; -import datadog.trace.api.iast.propagation.PropagationModule; -import net.bytebuddy.asm.Advice; -import org.json.JSONArray; -import org.json.JSONObject; - -public class OptAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { - boolean isString = result instanceof String; - boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); - if (!isString && !isJson) { - return; - } - final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; - if (iastModule != null) { - if (isString) { - iastModule.taintStringIfTainted((String) result, self); - } else { - iastModule.taintObjectIfTainted(result, self); - } - } - } -} diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy index ca7f07eddfe..004b7afdd22 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONCookieInstrumentationTest.groovy @@ -23,8 +23,7 @@ class JSONCookieInstrumentationTest extends AgentTestRunner { then: 1 * module.taintObjectIfTainted(_ as JSONObject, cookie) - 1 * module.taintObjectIfTainted(_ as Reader, cookie) - 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) + 1 * module.taintObjectIfTainted(_ as JSONTokener, cookie) 0 * _ } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy index ee0e3b897f7..3b9296c6b4d 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy @@ -3,7 +3,6 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONObject import org.json.JSONTokener -import spock.lang.Shared class JSONObjectInstrumentationTest extends AgentTestRunner { @@ -11,15 +10,6 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { injectSysConfig("dd.iast.enabled", "true") } - @Shared - Map tainteds = new IdentityHashMap<>() - - - void setup() { - tainteds.clear() - } - - void 'test JSONObject string constructor'() { given: final module = Mock(PropagationModule) @@ -38,10 +28,8 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { new JSONObject(json) then: - 1 * module.taintObjectIfTainted(_ as Reader, json) - 1 * module.taintObjectIfTainted(_ as JSONTokener, _ as Reader) - // Two calls are necessary, once for the whole object and other for the menu object - 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) 0 * _ } @@ -77,18 +65,13 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { 0 * _ } + void 'test JSONObject #method'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] + "name": "nameTest" }}""" final jsonObject = new JSONObject(json) final getObject =jsonObject.getJSONObject("menu") @@ -104,46 +87,4 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { where: method << ['get', 'getString', 'opt', 'optString'] } - - void 'test JSONObject elements are tainted'() { - given: - final module = Mock(PropagationModule) { - taintObjectIfTainted(_, _) >> { - if (tainteds.containsKey(it[1])) { - tainteds.put(it[0], null) - } - } - taintStringIfTainted(_, _) >> { - if (tainteds.containsKey(it[1])) { - tainteds.put(it[0], null) - } - } - } - InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" - tainteds.put(json, null) - final jsonObject = new JSONObject(json) - final getObject =jsonObject.getJSONObject("menu") - - when: - final name = getObject.get('name') - final file = getObject.get('labels').get(0) - final edit = getObject.get('labels').get(1) - - then: - name == "nameTest" - tainteds.containsKey(name) - file == "File" - tainteds.containsKey(file) - edit == "Edit" - tainteds.containsKey(edit) - } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy index 806a691074a..de8fa1da40c 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONTokenerInstrumentationTest.groovy @@ -5,38 +5,21 @@ import org.json.JSONTokener class JSONTokenerInstrumentationTest extends AgentTestRunner { - private static final String JSON_STRING = '{"name": "nameTest", "value" : "valueTest"}' // Reused JSON String - - @Override - void configurePreAgent() { + @Override void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") } - void 'test JSONTokener constructor with different argument types'() { + void 'test JSONTokener string constructor'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) + final json = '{"name": "nameTest", "value" : "valueTest"}' when: - new JSONTokener(args) + new JSONTokener(json) then: - 1 * module.taintObjectIfTainted(_ as JSONTokener, _) - if(args instanceof String) { - 1 * module.taintObjectIfTainted(_ as Reader, _ as String) - } else if (args instanceof InputStream) { - 1 * module.taintObjectIfTainted(_ as Reader, _ as InputStream) - } + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) 0 * _ - - where: - args << [ - JSON_STRING, - // String input - new ByteArrayInputStream(JSON_STRING.bytes), - // InputStream input - new StringReader(JSON_STRING) // Reader input - ] } - } From 8149451dbb1fd317fdd1bb2a450c5cc955db2ac0 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 13:27:45 +0100 Subject: [PATCH 25/32] Revert "fix codenarc" This reverts commit 289a048ffc9cfd97900d8389c0ade6565181b990. --- .../org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy index e654c0f89a3..4136d2d3043 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy @@ -3,6 +3,7 @@ import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.PropagationModule import org.json.JSONArray import org.json.JSONObject +import org.json.JSONTokener class JSONArrayInstrumentationTest extends AgentTestRunner { From 3c2f06808aa8c74d9946fecbbff3f5824ba88cc6 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 13:27:45 +0100 Subject: [PATCH 26/32] Revert "Fix org.json iast instrumentation test for latest dependency" This reverts commit 449d681b57c4fb66b9ce6e2ec9cabe62a16fd1ea. --- .../instrumentation/org-json/build.gradle | 2 +- .../instrumentation/org-json/gradle.lockfile | 2 +- .../json/JSONArrayInstrumentation.java | 27 ++++++++ .../json/JSONObjectInstrumentation.java | 27 ++++++++ .../JSONArrayInstrumentationTest.groovy | 61 ++++++++-------- .../JSONObjectInstrumentationTest.groovy | 69 ++++++++++++------- 6 files changed, 133 insertions(+), 55 deletions(-) diff --git a/dd-java-agent/instrumentation/org-json/build.gradle b/dd-java-agent/instrumentation/org-json/build.gradle index a9526cd5d9d..ac7e4b067c9 100644 --- a/dd-java-agent/instrumentation/org-json/build.gradle +++ b/dd-java-agent/instrumentation/org-json/build.gradle @@ -18,5 +18,5 @@ dependencies { testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') //FIXME: ASM - latestDepTestImplementation group: 'org.json', name: 'json', version: '+' + latestDepTestImplementation group: 'org.json', name: 'json', version: '20240303' } diff --git a/dd-java-agent/instrumentation/org-json/gradle.lockfile b/dd-java-agent/instrumentation/org-json/gradle.lockfile index 098936c2c5f..46487dd2105 100644 --- a/dd-java-agent/instrumentation/org-json/gradle.lockfile +++ b/dd-java-agent/instrumentation/org-json/gradle.lockfile @@ -116,7 +116,7 @@ org.hamcrest:hamcrest-core:1.3=latestDepTestCompileClasspath,latestDepTestRuntim org.hamcrest:hamcrest:2.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.jctools:jctools-core:3.3.0=instrumentPluginClasspath,latestDepTestRuntimeClasspath,muzzleTooling,runtimeClasspath,testRuntimeClasspath org.json:json:20230227=compileClasspath,testCompileClasspath,testRuntimeClasspath -org.json:json:20250107=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath +org.json:json:20240303=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath org.junit.jupiter:junit-jupiter-api:5.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.junit.jupiter:junit-jupiter-engine:5.9.2=latestDepTestRuntimeClasspath,testRuntimeClasspath org.junit.platform:junit-platform-commons:1.9.2=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java index 7bd4f6ebc64..942a3905592 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONArrayInstrumentation.java @@ -35,6 +35,13 @@ public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isConstructor().and(takesArguments(String.class)), getClass().getName() + "$ConstructorAdvice"); + transformer.applyAdvice( + isMethod() + .and(isPublic()) + .and(returns(Object.class)) + .and(named("get")) + .and(takesArguments(1)), + getClass().getName() + "$GetAdvice"); transformer.applyAdvice( isMethod().and(isPublic()).and(returns(Object.class)).and(named("opt")), getClass().getName() + "$OptAdvice"); @@ -51,6 +58,26 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } + public static class GetAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } + } + public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation diff --git a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java index 9746eda1fd5..c260d5390ad 100644 --- a/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java +++ b/dd-java-agent/instrumentation/org-json/src/main/java/datadog/trace/instrumentation/json/JSONObjectInstrumentation.java @@ -33,6 +33,13 @@ public String instrumentedType() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isConstructor().and(takesArguments(1)), getClass().getName() + "$ConstructorAdvice"); + transformer.applyAdvice( + isMethod() + .and(isPublic()) + .and(returns(Object.class)) + .and(named("get")) + .and(takesArguments(String.class)), + getClass().getName() + "$GetAdvice"); transformer.applyAdvice( isMethod() .and(isPublic()) @@ -53,6 +60,26 @@ public static void afterInit(@Advice.This Object self, @Advice.Argument(0) final } } + public static class GetAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Propagation + public static void afterMethod(@Advice.This Object self, @Advice.Return final Object result) { + boolean isString = result instanceof String; + boolean isJson = !isString && (result instanceof JSONObject || result instanceof JSONArray); + if (!isString && !isJson) { + return; + } + final PropagationModule iastModule = InstrumentationBridge.PROPAGATION; + if (iastModule != null) { + if (isString) { + iastModule.taintStringIfTainted((String) result, self); + } else { + iastModule.taintObjectIfTainted(result, self); + } + } + } + } + public static class OptAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Propagation diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy index 4136d2d3043..d0a97012811 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONArrayInstrumentationTest.groovy @@ -7,16 +7,6 @@ import org.json.JSONTokener class JSONArrayInstrumentationTest extends AgentTestRunner { - private static json = """{"menu": { - "name": "nameTest", - "value": "File", - "popup": "Popup", - "labels": [ - "File", - "Edit" - ] - }}""" - @Override void configurePreAgent() { injectSysConfig("dd.iast.enabled", "true") @@ -26,17 +16,28 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final jsonObject = new JSONObject(json) - final menuObject = jsonObject.getJSONObject("menu") + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" when: - final array = menuObject.getJSONArray("labels") + final jsonObject = new JSONObject(json) + final name = jsonObject.getJSONObject("menu").getJSONArray("labels").get(0) then: - array.length() == 2 - array.get(0) == "File" - array.get(1) == "Edit" - 1 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) + name == "File" + 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) + 2 * module.taintStringIfTainted("File", _ as JSONArray) 0 * _ } @@ -44,22 +45,28 @@ class JSONArrayInstrumentationTest extends AgentTestRunner { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final jsonObject = new JSONObject(json) - final jsonArray =jsonObject.getJSONObject("menu").getJSONArray("labels") + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" when: - final name = jsonArray.optString(0, "defaultvalue") + final jsonObject = new JSONObject(json) + final name = jsonObject.getJSONObject("menu").getJSONArray("labels").optString(0, "defaultvalue") then: name == "File" + 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 2 * module.taintObjectIfTainted(_ as JSONArray, _ as JSONObject) 1 * module.taintStringIfTainted("File", _ as JSONArray) 0 * _ - - where: - method | arguments - "opt" | [0] - "optString" | [0, "defaultvalue"] - "get" | [0] - "getString" | [0, "defaultvalue"] } } diff --git a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy index 3b9296c6b4d..49be03842d2 100644 --- a/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/org-json/src/test/groovy/JSONObjectInstrumentationTest.groovy @@ -25,66 +25,83 @@ class JSONObjectInstrumentationTest extends AgentTestRunner { }}""" when: - new JSONObject(json) + final jsonObject = new JSONObject(json) + final name = jsonObject.getJSONObject("menu").get("name") then: + name == "nameTest" 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } - void 'test JSONObject JSonTokenizer constructor'() { + void 'test JSONObject opt'() { given: final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = '{"name": "nameTest", "value" : "valueTest"}' - final jsonTokener = new JSONTokener(json) + final json = """{"menu": { + "name": "nameTest", + "value": "File", + "popup": "Popup", + "labels": [ + "File", + "Edit" + ] + }}""" when: - new JSONObject(jsonTokener) + final jsonObject = new JSONObject(json) + final name = jsonObject.getJSONObject("menu").optString("name") then: - 1 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + name == "nameTest" + 1 * module.taintObjectIfTainted(_ as JSONObject, json) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 2 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } - void 'test JSONObject map constructor'(){ + + + void 'test JSONObject JSonTokenizer constructor'() { given: - final Map map = new HashMap<>() - map.put("name", "nameTest") - map.put("age", "22") - map.put("city", "chicago") final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) + final json = '{"name": "nameTest", "value" : "valueTest"}' when: - new JSONObject(map) + final jsonObject = new JSONObject(new JSONTokener(json)) + final name = jsonObject.get("name") then: - 1 * module.taintObjectIfTainted(_ as JSONObject, map) + name == "nameTest" + 1 * module.taintObjectIfTainted(_ as JSONObject, _ as JSONTokener) + 1 * module.taintObjectIfTainted(_ as JSONTokener, json) + 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ } - - void 'test JSONObject #method'() { + void 'test JSONObject map constructor'(){ given: + final Map map = new HashMap<>() + map.put("name", "nameTest") + map.put("age", "22") + map.put("city", "chicago") final module = Mock(PropagationModule) InstrumentationBridge.registerIastModule(module) - final json = """{"menu": { - "name": "nameTest" - }}""" - final jsonObject = new JSONObject(json) - final getObject =jsonObject.getJSONObject("menu") when: - final name = getObject."$method"('name') + final jsonObject = new JSONObject(map) + jsonObject.get("name") then: - name == "nameTest" - 1 * module.taintStringIfTainted("nameTest", _ as JSONObject) + 1 * module.taintObjectIfTainted(_ as JSONObject, map) + 2 * module.taintStringIfTainted("nameTest", _ as JSONObject) 0 * _ - - where: - method << ['get', 'getString', 'opt', 'optString'] } } From f994bacb9d07e903f7edc22655e5c297c40cb77d Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 11 Feb 2025 15:33:05 +0100 Subject: [PATCH 27/32] product trace source should generate two-character case-insensitive hexadecimal string and be able to parse masks of at least 32 bits to ensure forward compatibility --- .../core/propagation/ptags/PTagsCodec.java | 20 +++++++--- .../DatadogPropagationTagsTest.groovy | 19 ++++++---- .../propagation/W3CPropagationTagsTest.groovy | 6 ++- .../datadog/trace/api/ProductTraceSource.java | 37 ++++++++++++++++--- 4 files changed, 61 insertions(+), 21 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java index eb083f054c7..2dde4f84a87 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java @@ -230,11 +230,17 @@ private static boolean validateTraceId(TagValue value) { } private static boolean validateTraceSourceTagValue(TagValue value) { - // Ensure the string is not null, has a length of 2, and matches the hex pattern - return value != null - && value.length() == 2 - && isHexDigit(value.charAt(0)) - && isHexDigit(value.charAt(1)); + // Ensure the string is not null and has a length between 2 and 8 + if (value == null || value.length() < 2 || value.length() > 8) { + return false; + } + for (int i = 0; i < value.length(); i++) { + // Ensure each character is a valid hex digit + if (!isHexDigitCaseInsensitive(value.charAt(i))) { + return false; + } + } + return true; } protected static boolean isDigit(char c) { @@ -244,4 +250,8 @@ protected static boolean isDigit(char c) { protected static boolean isHexDigit(char c) { return c >= 'a' && c <= 'f' || isDigit(c); } + + protected static boolean isHexDigitCaseInsensitive(char c) { + return isHexDigit(c) || c >= 'A' && c <= 'F'; + } } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy index 636b8b412b8..60a56900a36 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/DatadogPropagationTagsTest.groovy @@ -154,16 +154,19 @@ class DatadogPropagationTagsTest extends DDCoreSpecification { propagationTags.createTagMap() == tags where: - originalTagSet | product | expectedHeaderValue | tags + originalTagSet | product | expectedHeaderValue | tags // keep the existing dm tag as is - "" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=00" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] - "_dd.p.ts=02" | ProductTraceSource.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] + "" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=00" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=FFC00000" | ProductTraceSource.ASM | "_dd.p.ts=02" | ["_dd.p.ts": "02"] + "_dd.p.ts=02" | ProductTraceSource.DBM | "_dd.p.ts=12" | ["_dd.p.ts": "12"] //Invalid input - "_dd.p.ts=" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=0" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=GG" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] - "_dd.p.ts=foo" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=0" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=0G" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=GG" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=foo" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] + "_dd.p.ts=000000002" | ProductTraceSource.UNSET | null | ["_dd.propagation_error": "decoding_error"] } def extractionLimitExceeded() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index f65859922e2..6f6540249cb 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -311,7 +311,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { propagationTags.createTagMap() == tags where: - headerValue | priority | mechanism | origin | expectedHeaderValue | tags + headerValue | priority | mechanism | origin | expectedHeaderValue | tags 'dd=s:0;o:some;t.dm:934086a686-4' | PrioritySampling.SAMPLER_KEEP | SamplingMechanism.DEFAULT | "other" | 'dd=s:0;o:other;t.dm:934086a686-4' | ['_dd.p.dm': '934086a686-4'] 'dd=s:0;o:some;x:unknown' | PrioritySampling.USER_KEEP | SamplingMechanism.LOCAL_USER_RULE | "same" | 'dd=s:2;o:same;t.dm:-3;x:unknown' | ['_dd.p.dm': '-3'] 'dd=s:0;o:some;x:unknown' | PrioritySampling.USER_DROP | SamplingMechanism.MANUAL | null | 'dd=s:-1;x:unknown' | [:] @@ -339,9 +339,11 @@ class W3CPropagationTagsTest extends DDCoreSpecification { propagationTags.createTagMap() == tags where: - headerValue | product | expectedHeaderValue | tags + headerValue | product | expectedHeaderValue | tags 'dd=x:unknown' | ProductTraceSource.ASM | 'dd=t.ts:02;x:unknown' | ['_dd.p.ts': '02'] 'dd=t.ts:02;x:unknown' | ProductTraceSource.DBM | 'dd=t.ts:12;x:unknown' | ['_dd.p.ts': '12'] + "dd=t.ts:00" | ProductTraceSource.ASM | 'dd=t.ts:02' | ["_dd.p.ts": "02"] + "dd=t.ts:FFC00000" | ProductTraceSource.ASM | 'dd=t.ts:02' | ["_dd.p.ts": "02"] } static private String toLcAlpha(String cs) { diff --git a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java index 7933609a5b2..792829dde3a 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java +++ b/internal-api/src/main/java/datadog/trace/api/ProductTraceSource.java @@ -1,5 +1,18 @@ package datadog.trace.api; +/** + * Represents a bitfield-based trace source for tracking product propagation tags. + * + *

The bitfield is encoded as an **8-bit hexadecimal mask** (ranging from `00` to `FF`), where + * each bit corresponds to a specific product. This class provides utility methods for updating, + * checking, and parsing these product bitfields. + * + *

    + *
  • MUST use a two-character, case-insensitive hexadecimal string (e.g., "00" to "FF"). + *
  • MUST support parsing masks of at least 32 bits to ensure forward compatibility. + *
  • Each bit corresponds to a specific product: + *
+ */ public class ProductTraceSource { public static final int UNSET = 0; @@ -10,26 +23,38 @@ public class ProductTraceSource { public static final int DJM = 0x08; public static final int DBM = 0x10; - // Update (set) the bitfield for a specific product + /** Updates the bitfield by setting the bit corresponding to a specific product. */ public static int updateProduct(int bitfield, int product) { return bitfield |= product; // Set the bit for the given product } - // Check if the bitfield is marked for a specific product + /** Checks if the bitfield is marked for a specific product. */ public static boolean isProductMarked(final int bitfield, int product) { return (bitfield & product) != 0; // Check if the bit is set } - // Get the current bitfield as a hexadecimal string + /** + * Converts the current bitfield to a two-character hexadecimal string. + * + *

This method ensures the output follows the **00 to FF** format, padding with leading zeros + * if necessary. + */ public static String getBitfieldHex(final int bitfield) { - return String.format("%02x", bitfield); // Convert to 2-character hex + String hex = Integer.toHexString(bitfield & 0xFF); + return hex.length() == 1 ? "0" + hex : hex; // Ensure two characters } - // Parse a hexadecimal string back to an integer + /** + * Parses a hexadecimal string back into an integer bitfield. + * + *

This method allows for **at least 32-bit parsing**, ensuring forward compatibility with + * potential future expansions. + */ public static int parseBitfieldHex(final String hexString) { if (hexString == null || hexString.isEmpty()) { return 0; // Return 0 if the string is empty } - return Integer.parseInt(hexString, 16); // Parse the string as a base-16 number + // Need to support unsigned parsing + return (int) Long.parseUnsignedLong(hexString, 16); // Parse the string as a base-16 number } } From 92ff4721a2b7671cdfcc1c510e0829b54faf2fcc Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 12 Feb 2025 10:11:10 +0100 Subject: [PATCH 28/32] special standalone sampler should only be used when tracing is disabled and appsec/iast are enabled --- ...Sampler.java => AsmStandaloneSampler.java} | 19 +++++---- .../trace/common/sampling/Sampler.java | 7 +++- ...groovy => AsmStandaloneSamplerTest.groovy} | 4 +- .../trace/common/sampling/SamplerTest.groovy | 41 ++++++++++++++++++- 4 files changed, 56 insertions(+), 15 deletions(-) rename dd-trace-core/src/main/java/datadog/trace/common/sampling/{ApmTracingDisabledSampler.java => AsmStandaloneSampler.java} (63%) rename dd-trace-core/src/test/groovy/datadog/trace/common/sampling/{ApmTracingDisabledSamplerTest.groovy => AsmStandaloneSamplerTest.groovy} (92%) diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java similarity index 63% rename from dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java rename to dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java index 18b9c233417..54a2d750700 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/ApmTracingDisabledSampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/AsmStandaloneSampler.java @@ -11,20 +11,21 @@ import org.slf4j.LoggerFactory; /** - * This class is designed to only allow 1 APM trace per minute for apm tracing disabled. The service - * catalog and the billing need a continuous ingestion of at least at 1 trace per minute to consider - * a service as being live and billable. In the absence of other products events, no APM traces must - * be sent, so we need to let some regular APM traces go through. + * This class is designed to only allow 1 APM trace per minute as standalone ASM is only interested + * in the traces containing ASM events. But the service catalog and the billing need a continuous + * ingestion of at least at 1 trace per minute to consider a service as being live and billable. In + * the absence of ASM events, no APM traces must be sent, so we need to let some regular APM traces + * go through, even in the absence of ASM events. */ -public class ApmTracingDisabledSampler implements Sampler, PrioritySampler { +public class AsmStandaloneSampler implements Sampler, PrioritySampler { - private static final Logger log = LoggerFactory.getLogger(ApmTracingDisabledSampler.class); + private static final Logger log = LoggerFactory.getLogger(AsmStandaloneSampler.class); private static final int RATE_IN_MILLISECONDS = 60000; // 1 minute private final AtomicLong lastSampleTime; private final Clock clock; - public ApmTracingDisabledSampler(final Clock clock) { + public AsmStandaloneSampler(final Clock clock) { this.clock = clock; this.lastSampleTime = new AtomicLong(clock.millis() - RATE_IN_MILLISECONDS); } @@ -41,10 +42,10 @@ public > void setSamplingPriority(final T span) { if (shouldSample()) { log.debug("Set SAMPLER_KEEP for span {}", span.getSpanId()); - span.setSamplingPriority(SAMPLER_KEEP, SamplingMechanism.DEFAULT); + span.setSamplingPriority(SAMPLER_KEEP, SamplingMechanism.APPSEC); } else { log.debug("Set SAMPLER_DROP for span {}", span.getSpanId()); - span.setSamplingPriority(SAMPLER_DROP, SamplingMechanism.DEFAULT); + span.setSamplingPriority(SAMPLER_DROP, SamplingMechanism.APPSEC); } } diff --git a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java index e591bfbbe7f..eb255b66328 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/sampling/Sampler.java @@ -4,6 +4,7 @@ import static datadog.trace.bootstrap.instrumentation.api.SamplerConstants.KEEP; import datadog.trace.api.Config; +import datadog.trace.api.ProductActivation; import datadog.trace.api.TraceConfig; import datadog.trace.api.config.TracerConfig; import datadog.trace.api.sampling.PrioritySampling; @@ -35,9 +36,11 @@ final class Builder { public static Sampler forConfig(final Config config, final TraceConfig traceConfig) { Sampler sampler; if (config != null) { - if (!config.isApmTracingEnabled()) { + if (!config.isApmTracingEnabled() + && (config.getAppSecActivation() == ProductActivation.FULLY_ENABLED + || config.getIastActivation() == ProductActivation.FULLY_ENABLED)) { log.debug("APM is disabled. Only 1 trace per minute will be sent."); - return new ApmTracingDisabledSampler(Clock.systemUTC()); + return new AsmStandaloneSampler(Clock.systemUTC()); } final Map serviceRules = config.getTraceSamplingServiceRules(); final Map operationRules = config.getTraceSamplingOperationRules(); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy similarity index 92% rename from dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy rename to dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy index 3a1ea329747..1d83510ec52 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/ApmTracingDisabledSamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/AsmStandaloneSamplerTest.groovy @@ -7,7 +7,7 @@ import datadog.trace.api.sampling.PrioritySampling import java.time.Clock import java.util.concurrent.atomic.AtomicLong -class ApmTracingDisabledSamplerTest extends DDCoreSpecification{ +class AsmStandaloneSamplerTest extends DDCoreSpecification{ def writer = new ListWriter() @@ -19,7 +19,7 @@ class ApmTracingDisabledSamplerTest extends DDCoreSpecification{ current.get() } } - def sampler = new ApmTracingDisabledSampler(clock) + def sampler = new AsmStandaloneSampler(clock) def tracer = tracerBuilder().writer(writer).sampler(sampler).build() when: diff --git a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy index e8c84d886d2..5591b118e5d 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/common/sampling/SamplerTest.groovy @@ -5,15 +5,52 @@ import datadog.trace.test.util.DDSpecification class SamplerTest extends DDSpecification{ - void "test that TimeSampler is selected when experimentalAppSecStandalone is enabled"() { + void "test that AsmStandaloneSampler is selected when apm tracing disabled and appsec enabled is enabled"() { setup: System.setProperty("dd.apm.tracing.enabled", "false") + System.setProperty("dd.appsec.enabled", "true") Config config = new Config() when: Sampler sampler = Sampler.Builder.forConfig(config, null) then: - sampler instanceof ApmTracingDisabledSampler + sampler instanceof AsmStandaloneSampler + } + + void "test that AsmStandaloneSampler is selected when apm tracing disabled and iast enabled is enabled"() { + setup: + System.setProperty("dd.apm.tracing.enabled", "false") + System.setProperty("dd.iast.enabled", "true") + Config config = new Config() + + when: + Sampler sampler = Sampler.Builder.forConfig(config, null) + + then: + sampler instanceof AsmStandaloneSampler + } + + void "test that AsmStandaloneSampler is not selected when apm tracing and asm not enabled"() { + setup: + System.setProperty("dd.apm.tracing.enabled", "false") + Config config = new Config() + + when: + Sampler sampler = Sampler.Builder.forConfig(config, null) + + then: + !(sampler instanceof AsmStandaloneSampler) + } + + void "test that AsmStandaloneSampler is not selected when apm tracing and asm not enabled"() { + setup: + Config config = new Config() + + when: + Sampler sampler = Sampler.Builder.forConfig(config, null) + + then: + !(sampler instanceof AsmStandaloneSampler) } } From 4867759a2f5b32fa6944d5681ccf07b2582d5f85 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 12 Feb 2025 15:11:32 +0100 Subject: [PATCH 29/32] fix merge issues --- .../src/main/java/datadog/trace/core/CoreTracer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 32c31760cbc..668cff57a1f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -86,12 +86,11 @@ import datadog.trace.core.monitor.HealthMetrics; import datadog.trace.core.monitor.MonitoringImpl; import datadog.trace.core.monitor.TracerHealthMetrics; -import datadog.trace.core.propagation.ApmTracionDisabledPropagator; +import datadog.trace.core.propagation.ApmTracingDisabledPropagator; import datadog.trace.core.propagation.CorePropagation; import datadog.trace.core.propagation.ExtractedContext; import datadog.trace.core.propagation.HttpCodec; import datadog.trace.core.propagation.PropagationTags; -import datadog.trace.core.propagation.ApmTracionDisabledPropagator; import datadog.trace.core.propagation.TracingPropagator; import datadog.trace.core.propagation.XRayPropagator; import datadog.trace.core.scopemanager.ContinuableScopeManager; @@ -729,7 +728,8 @@ private CoreTracer( // If disabled, the most common case, use the usual tracing propagator by default. boolean apmTracingDisabled = !config.isApmTracingEnabled(); boolean dsm = config.isDataStreamsEnabled(); - Propagators.register(STANDALONE_ASM_CONCERN, new ApmTracionDisabledPropagator(), apmTracingDisabled); + Propagators.register( + STANDALONE_ASM_CONCERN, new ApmTracingDisabledPropagator(), apmTracingDisabled); Propagators.register(TRACING_CONCERN, tracingPropagator, !apmTracingDisabled); Propagators.register(XRAY_TRACING_CONCERN, new XRayPropagator(config), false); if (dsm) { From b5f1b221c218e4c0681ae08fa886309514c7e067 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 14 Feb 2025 08:01:01 +0100 Subject: [PATCH 30/32] rename updatePropagatedTraceSource method --- .../src/main/java/datadog/trace/core/DDSpanContext.java | 2 +- .../java/datadog/trace/core/taginterceptor/TagInterceptor.java | 2 +- .../datadog/trace/core/taginterceptor/TagInterceptorTest.groovy | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 67b0f20bf00..33574e62f47 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -512,7 +512,7 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void updatePropagatedTraceSource(final int value) { + public void addTraceSource(final int value) { propagationTags.addTraceSource(value); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 09c568dca30..6335db6415b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -114,7 +114,7 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return interceptSamplingPriority(span, value); case Tags.PROPAGATED_TRACE_SOURCE: if (value instanceof Integer) { - span.updatePropagatedTraceSource((Integer) value); + span.addTraceSource((Integer) value); return true; } return false; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 2b9d4b9c3cf..5e76e83274f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -742,6 +742,6 @@ class TagInterceptorTest extends DDCoreSpecification { interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) then: - 1 * context.updatePropagatedTraceSource(ProductTraceSource.ASM) + 1 * context.addTraceSource(ProductTraceSource.ASM) } } From 8eb0d65bf14f43ff2ab34eb8e2892f10638c2a12 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 14 Feb 2025 08:02:23 +0100 Subject: [PATCH 31/32] Revert "rename updatePropagatedTraceSource method" This reverts commit b5f1b221c218e4c0681ae08fa886309514c7e067. --- .../src/main/java/datadog/trace/core/DDSpanContext.java | 2 +- .../java/datadog/trace/core/taginterceptor/TagInterceptor.java | 2 +- .../datadog/trace/core/taginterceptor/TagInterceptorTest.groovy | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 33574e62f47..67b0f20bf00 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -512,7 +512,7 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void addTraceSource(final int value) { + public void updatePropagatedTraceSource(final int value) { propagationTags.addTraceSource(value); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 6335db6415b..09c568dca30 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -114,7 +114,7 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return interceptSamplingPriority(span, value); case Tags.PROPAGATED_TRACE_SOURCE: if (value instanceof Integer) { - span.addTraceSource((Integer) value); + span.updatePropagatedTraceSource((Integer) value); return true; } return false; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 5e76e83274f..2b9d4b9c3cf 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -742,6 +742,6 @@ class TagInterceptorTest extends DDCoreSpecification { interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) then: - 1 * context.addTraceSource(ProductTraceSource.ASM) + 1 * context.updatePropagatedTraceSource(ProductTraceSource.ASM) } } From cb9a2d4b0d22768b246c7261550c998dd6850617 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 14 Feb 2025 08:03:17 +0100 Subject: [PATCH 32/32] rename updatePropagatedTraceSource method --- .../src/main/java/datadog/trace/core/DDSpanContext.java | 2 +- .../java/datadog/trace/core/taginterceptor/TagInterceptor.java | 2 +- .../datadog/trace/core/taginterceptor/TagInterceptorTest.groovy | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 67b0f20bf00..bce7d8800f3 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -512,7 +512,7 @@ private void forceKeepThisSpan(byte samplingMechanism) { } } - public void updatePropagatedTraceSource(final int value) { + public void addPropagatedTraceSource(final int value) { propagationTags.addTraceSource(value); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 09c568dca30..931eca80721 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -114,7 +114,7 @@ public boolean interceptTag(DDSpanContext span, String tag, Object value) { return interceptSamplingPriority(span, value); case Tags.PROPAGATED_TRACE_SOURCE: if (value instanceof Integer) { - span.updatePropagatedTraceSource((Integer) value); + span.addPropagatedTraceSource((Integer) value); return true; } return false; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy index 2b9d4b9c3cf..075ac57a188 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/taginterceptor/TagInterceptorTest.groovy @@ -742,6 +742,6 @@ class TagInterceptorTest extends DDCoreSpecification { interceptor.interceptTag(context, Tags.PROPAGATED_TRACE_SOURCE, ProductTraceSource.ASM) then: - 1 * context.updatePropagatedTraceSource(ProductTraceSource.ASM) + 1 * context.addPropagatedTraceSource(ProductTraceSource.ASM) } }