Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
fd62009
saving work
mhlidd Jul 14, 2025
a756eb2
init: adding System.getenv forbiddenAPI, ConfigHelper, ParseSupported…
mhlidd Jul 15, 2025
96e2546
Fixing java6 incompatibility with EnvironmentVariables component
mhlidd Jul 15, 2025
6a87970
parsing JSON at buildtime
mhlidd Jul 17, 2025
b062c96
fixing invocationtargetexception
mhlidd Jul 17, 2025
6769bda
spotless
mhlidd Jul 17, 2025
06ed2fd
updating linter to include OTEL_
mhlidd Jul 18, 2025
8dd50e0
adding EnvironmentVariables linter
mhlidd Jul 18, 2025
9660b1a
adding flag for strictness
mhlidd Jul 23, 2025
f8d2a8c
updating supported-configurations
mhlidd Jul 24, 2025
b68f0b2
updated all discoverable configs
mhlidd Jul 29, 2025
dc5d83b
updating jmxfetch configs
mhlidd Jul 30, 2025
fe2eeb9
fixing config helper
mhlidd Jul 31, 2025
229c38d
adding unit tests for ConfigHelper
mhlidd Aug 1, 2025
77ddecd
fixing tests pt 1
mhlidd Aug 5, 2025
eb6908a
adding configs and updating specific tests to TEST strictness
mhlidd Aug 5, 2025
972eef4
cleanup and removing system.getenv call
mhlidd Aug 5, 2025
81c12f4
adding more supported-configurations
mhlidd Aug 5, 2025
98c57b5
adding more configs
mhlidd Aug 5, 2025
990c0ce
adding configs and updating tests
mhlidd Aug 6, 2025
5bdd8cf
adding branch coverage
mhlidd Aug 6, 2025
52c05b4
updating code coverage again
mhlidd Aug 6, 2025
5192a8a
test
mhlidd Aug 7, 2025
f128de8
rebasing
mhlidd Aug 7, 2025
734f9c7
adding more configs post-rebase
mhlidd Aug 7, 2025
e9ce233
initialize GeneratedSupportedConfiguratons at buildtime
mhlidd Aug 12, 2025
90f536d
fixing buildtests
mhlidd Aug 12, 2025
7a1517e
adding ConfigInversionStrictStyle
mhlidd Aug 12, 2025
5ca61c0
refactor ConfigHelper to datadog.trace.api
mhlidd Aug 13, 2025
abc0b64
adding configs and updating gradle files to reflect ConfigHelper refa…
mhlidd Aug 13, 2025
97db41f
adding telemetry
mhlidd Aug 13, 2025
04bde01
reverting agent-bootstrap to directly invoke EnvironmentVariables com…
mhlidd Aug 14, 2025
9a764c9
Merge branch 'master' into mhlidd/config_inversion
mhlidd Aug 14, 2025
e10e0a9
updating ConfigHelper to use getAll()
mhlidd Aug 14, 2025
340eeef
adding classes to native build time and adding suppressforbidden
mhlidd Aug 14, 2025
ea83e25
Adding SupportedConfigurationsSource to buildtime
mhlidd Aug 14, 2025
4b602c1
testing
mhlidd Aug 14, 2025
4bdb960
attempt to fix native image
mhlidd Aug 14, 2025
962490d
attempt to fix native build
mhlidd Aug 15, 2025
c7aa94d
cleanup pt 1
mhlidd Aug 18, 2025
8068724
cleanup pt 2
mhlidd Aug 18, 2025
47a8f6a
moving buildtime parser out of environment component
mhlidd Aug 20, 2025
94f392c
updating tests bootstrap constants and spock runner
mhlidd Aug 20, 2025
6c2a551
undo migration to configHelper
mhlidd Aug 25, 2025
fe697ff
pushing changes for new module
mhlidd Aug 26, 2025
1aefa7e
updating gradle tasks to kotlin plugins
mhlidd Aug 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ plugins {
id("de.thetaphi.forbiddenapis") version "3.8"

id("tracer-version")
id("config-inversion-linter")
id("io.github.gradle-nexus.publish-plugin") version "2.0.0"

id("com.gradleup.shadow") version "8.3.6" apply false
Expand Down
13 changes: 13 additions & 0 deletions buildSrc/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ gradlePlugin {
id = "tracer-version"
implementationClass = "datadog.gradle.plugin.version.TracerVersionPlugin"
}
create("config-generation") {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: This doesn;t have any effect really, but it just gives the same prefix to the name.

Suggested change
create("config-generation") {
create("supported-config-generation") {

id = "supported-config-generator"
implementationClass = "datadog.gradle.plugin.config.SupportedConfigPlugin"
}
create("supported-config-linter") {
id = "config-inversion-linter"
implementationClass = "datadog.gradle.plugin.config.ConfigInversionLinter"
}
}
}

Expand All @@ -52,6 +60,11 @@ dependencies {
implementation("com.google.guava", "guava", "20.0")
implementation("org.ow2.asm", "asm", "9.8")
implementation("org.ow2.asm", "asm-tree", "9.8")

implementation(platform("com.fasterxml.jackson:jackson-bom:2.17.2"))
implementation("com.fasterxml.jackson.core:jackson-databind")
implementation("com.fasterxml.jackson.core:jackson-annotations")
implementation("com.fasterxml.jackson.core:jackson-core")
}

tasks.compileKotlin {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package datadog.gradle.plugin.config

import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.GradleException
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.SourceSetContainer
import java.net.URLClassLoader
import java.nio.file.Path

class ConfigInversionLinter : Plugin<Project> {
override fun apply(target: Project) {
registerLogEnvVarUsages(target)
registerCheckEnvironmentVariablesUsage(target)
}
}

/** Registers `logEnvVarUsages` (scan for DD_/OTEL_ tokens and fail if unsupported). */
private fun registerLogEnvVarUsages(target: Project) {
val ownerPath = ":utils:config-utils" // <-- change to the module that contains the generated class
val generatedFile = "datadog.config.GeneratedSupportedConfigurations"
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: I'm thinking this should be configured via an extension, it's a gradle configuration mechanism.

Basically, the idea is to introduce an extension that will hold the configuration for both plugins, and the tasks registered from these plugins will grab the configuration from it.

Something along the lines

open class SupportedTracerConfigurations @Inject constructor(objects: ObjectFactory) {
  val configOwnerPath = objects.property<String>().convention(":utils:config-utils")
  val className = objects.property<String>().convention("datadog.config.GeneratedSupportedConfigurations")

  val jsonFile = objects.fileProperty().convention(project.file("src/main/resources/supported-configurations.json"))

  val destinationDirectory = objects.directoryProperty().convention(project.layout.buildDirectory.dir("generated/supportedConfigurations"))

  // ... other configs
  // maybe excluded files in specific submodules like "internal-api/...", "dd-java-agent/..."
}

Then in the plugin before registering tasks

  override fun apply(target: Project) {
    val extension = target.extensions.create("supportedTracerConfigurations", SupportedTracerConfigurations::class.java)
  }

then access the extension vals as needed.

Note, in the example above I'm leveraging .convention(...) which sets "default" values.

This extension class should hold most of the values that are spread in various places.


// token check that uses the generated class instead of JSON
target.tasks.register("logEnvVarUsages") {
group = "verification"
description = "Scan Java files for DD_/OTEL_ tokens and fail if unsupported (using generated constants)"

val owner = target.project(ownerPath)
val sourceSets = owner.extensions.getByType(SourceSetContainer::class.java)
val main = sourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME)

// ensure the owner project generated/compiled the class
dependsOn(
owner.tasks.named("generateSupportedConfigurations"),
owner.tasks.named("classes") // compiles main (including generated sources)
)
Comment on lines +28 to +36
Copy link
Contributor

@bric3 bric3 Aug 28, 2025

Choose a reason for hiding this comment

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

suggestion: I believe this can be replaced by declaring the config owner output as input of this task. E.g. something like

val mainSourceSetOutput = target.project(configOwnerPath).extensions.getByType<SourceSetContainer>().named(SourceSet.MAIN_SOURCE_SET_NAME).map { it.output } // it's a Provider
inputs.files(mainSourceSetOutput)

I believe using the source set output is enough to load the class. But using the runtimeClasspath instead should work as well.


// inputs for incrementality (your own source files, not the owner’s)
val javaFiles = target.fileTree(target.projectDir) {
include("**/src/main/java/**/*.java")
exclude("**/build/**", "**/dd-smoke-tests/**")
}
inputs.files(javaFiles)
outputs.upToDateWhen { true}
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: space

Suggested change
outputs.upToDateWhen { true}
outputs.upToDateWhen { true }

doLast {
// 1) Build classloader from the owner project’s runtime classpath
val urls = main.runtimeClasspath.files.map { it.toURI().toURL() }.toTypedArray()
URLClassLoader(urls, javaClass.classLoader).use { cl ->
// 2) Load the generated class + read static field
val clazz = Class.forName(generatedFile, true, cl)

@Suppress("UNCHECKED_CAST")
val supported: Set<String> = clazz.getField("SUPPORTED").get(null) as Set<String>
Comment on lines +47 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Since clazz is used only once I suggest to close the use block there.

Suggested change
val urls = main.runtimeClasspath.files.map { it.toURI().toURL() }.toTypedArray()
URLClassLoader(urls, javaClass.classLoader).use { cl ->
// 2) Load the generated class + read static field
val clazz = Class.forName(generatedFile, true, cl)
@Suppress("UNCHECKED_CAST")
val supported: Set<String> = clazz.getField("SUPPORTED").get(null) as Set<String>
val urls = mainSourceSetOutput.get().files.map { it.toURI().toURL() }.toTypedArray()
val supported: Set<String> = URLClassLoader(urls, javaClass.classLoader).use { cl ->
// 2) Load the generated class + read static field
val clazz = Class.forName(generatedFile, true, cl)
@Suppress("UNCHECKED_CAST")
clazz.getField("SUPPORTED").get(null) as Set<String>
}

Note I'm referring to mainSourceSetOutput mentioned in the comment above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option would be to use the json conf instead of loading the generated class. This would allow to skip the generation and the compilation of the generated class ?


// 3) Scan our sources and compare
val repoRoot = target.projectDir.toPath()
val tokenRegex = Regex("\"(?:DD_|OTEL_)[A-Za-z0-9_]+\"")

val violations = mutableListOf<String>()
javaFiles.files.forEach { f ->
val rel = repoRoot.relativize(f.toPath()).toString()
var inBlock = false
f.readLines().forEachIndexed { i, raw ->
val trimmed = raw.trim()
if (trimmed.startsWith("//")) return@forEachIndexed
if (!inBlock && trimmed.contains("/*")) inBlock = true
if (inBlock) {
if (trimmed.contains("*/")) inBlock = false
return@forEachIndexed
}
tokenRegex.findAll(raw).forEach { m ->
val token = m.value.trim('"')
if (token !in supported) violations += "$rel:${i + 1} -> Unsupported token '$token'"
}
}
}

if (violations.isNotEmpty()) {
violations.forEach { target.logger.error(it) }
throw GradleException("Unsupported DD_/OTEL_ tokens found! See errors above.")
} else {
target.logger.lifecycle("All DD_/OTEL_ tokens are supported.")
}
}
}
}
}

/** Registers `checkEnvironmentVariablesUsage` (forbid EnvironmentVariables.get(...)). */
private fun registerCheckEnvironmentVariablesUsage(project: Project) {
project.tasks.register("checkEnvironmentVariablesUsage") {
group = "verification"
description = "Scans src/main/java for direct usages of EnvironmentVariables.get(...)"

doLast {
val repoRoot: Path = project.projectDir.toPath()
val javaFiles = project.fileTree(project.projectDir) {
include("**/src/main/java/**/*.java")
exclude("**/build/**")
exclude("internal-api/src/main/java/datadog/trace/api/ConfigHelper.java")
exclude("dd-java-agent/agent-bootstrap/**")
exclude("dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java")
}

val pattern = Regex("""EnvironmentVariables\.get\s*\(""")
val matches = mutableListOf<String>()

javaFiles.forEach { f ->
val relative = repoRoot.relativize(f.toPath())
f.readLines().forEachIndexed { idx, line ->
if (pattern.containsMatchIn(line)) {
matches += "$relative:${idx + 1} -> ${line.trim()}"
}
}
}

if (matches.isNotEmpty()) {
project.logger.lifecycle("\nFound forbidden usages of EnvironmentVariables.get(...):")
matches.forEach { project.logger.lifecycle(it) }
throw GradleException("Forbidden usage of EnvironmentVariables.get(...) found in Java files.")
} else {
project.logger.lifecycle("No forbidden EnvironmentVariables.get(...) usages found in src/main/java.")
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package datadog.gradle.plugin.config

import org.gradle.api.DefaultTask
import org.gradle.api.model.ObjectFactory
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFile
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction
import com.fasterxml.jackson.core.type.TypeReference
import com.fasterxml.jackson.databind.ObjectMapper
import java.io.File
import java.io.FileInputStream
import java.io.PrintWriter
import javax.inject.Inject

abstract class ParseSupportedConfigurationsTask @Inject constructor(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Maybe you can also add @CacheableTask

Suggested change
abstract class ParseSupportedConfigurationsTask @Inject constructor(
@CacheableTask
abstract class ParseSupportedConfigurationsTask @Inject constructor(

private val objects: ObjectFactory
) : DefaultTask() {
@InputFile
val jsonFile = objects.fileProperty()

@get:OutputDirectory
val destinationDirectory = objects.directoryProperty()

@Input
val className = objects.property(String::class.java)

@TaskAction
fun generate() {
val input = jsonFile.get().asFile
val outputDir = destinationDirectory.get().asFile
val fqcn = className.get()
outputDir.mkdirs()

// Read JSON (directly from the file, not classpath)
val mapper = ObjectMapper()
val fileData: Map<String, Any?> = FileInputStream(input).use { inStream ->
mapper.readValue(inStream, object : TypeReference<Map<String, Any?>>() {})
}

@Suppress("UNCHECKED_CAST")
val supported = fileData["supportedConfigurations"] as Map<String, List<String>>
@Suppress("UNCHECKED_CAST")
val aliases = fileData["aliases"] as Map<String, List<String>>
@Suppress("UNCHECKED_CAST")
val deprecated = (fileData["deprecations"] as? Map<String, String>) ?: emptyMap()

val aliasMapping = mutableMapOf<String, String>()
for ((canonical, alist) in aliases) {
for (alias in alist) aliasMapping[alias] = canonical
}

// Build the output .java path from the fully-qualified class name
val pkgPath = fqcn.substringBeforeLast('.', "").replace('.', File.separatorChar)
val simpleName = fqcn.substringAfterLast('.')
val pkgDir = if (pkgPath.isEmpty()) outputDir else File(outputDir, pkgPath).also { it.mkdirs() }
val generatedFile = File(pkgDir, "$simpleName.java").absolutePath

// Call your existing generator (same signature as in your Java code)
generateJavaFile(
generatedFile,
supported.keys,
aliases,
aliasMapping,
deprecated
)
}

private fun generateJavaFile(
outputPath: String,
supportedKeys: Set<String>,
aliases: Map<String, List<String>>,
aliasMapping: Map<String, String>,
deprecated: Map<String, String>
) {
val outFile = File(outputPath)
outFile.parentFile?.mkdirs()

PrintWriter(outFile).use { out ->
// NOTE: adjust these if you want to match task's className
out.println("package datadog.config;")
out.println()
out.println("import java.util.*;")
out.println()
out.println("public final class GeneratedSupportedConfigurations {")
Copy link
Contributor

Choose a reason for hiding this comment

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

note: Shouldn't this be coming from the className property ?

out.println()
out.println(" public static final Set<String> SUPPORTED;")
out.println()
out.println(" public static final Map<String, List<String>> ALIASES;")
out.println()
out.println(" public static final Map<String, String> ALIAS_MAPPING;")
out.println()
out.println(" public static final Map<String, String> DEPRECATED;")
out.println()
out.println(" static {")
out.println()

// SUPPORTED
out.print(" Set<String> supportedSet = new HashSet<>(Arrays.asList(")
val supportedIter = supportedKeys.toSortedSet().iterator()
while (supportedIter.hasNext()) {
val key = supportedIter.next()
out.print("\"${esc(key)}\"")
if (supportedIter.hasNext()) out.print(", ")
}
out.println("));")
out.println(" SUPPORTED = Collections.unmodifiableSet(supportedSet);")
out.println()

// ALIASES
out.println(" Map<String, List<String>> aliasesMap = new HashMap<>();")
for ((canonical, list) in aliases.toSortedMap()) {
out.printf(
" aliasesMap.put(\"%s\", Collections.unmodifiableList(Arrays.asList(%s)));\n",
esc(canonical),
quoteList(list)
)
}
out.println(" ALIASES = Collections.unmodifiableMap(aliasesMap);")
out.println()

// ALIAS_MAPPING
out.println(" Map<String, String> aliasMappingMap = new HashMap<>();")
for ((alias, target) in aliasMapping.toSortedMap()) {
out.printf(" aliasMappingMap.put(\"%s\", \"%s\");\n", esc(alias), esc(target))
}
out.println(" ALIAS_MAPPING = Collections.unmodifiableMap(aliasMappingMap);")
out.println()

// DEPRECATED
out.println(" Map<String, String> deprecatedMap = new HashMap<>();")
for ((oldKey, note) in deprecated.toSortedMap()) {
out.printf(" deprecatedMap.put(\"%s\", \"%s\");\n", esc(oldKey), esc(note))
}
out.println(" DEPRECATED = Collections.unmodifiableMap(deprecatedMap);")
out.println()
out.println(" }")
out.println("}")
}
}

private fun quoteList(list: List<String>): String =
list.joinToString(", ") { "\"${esc(it)}\"" }

private fun esc(s: String): String =
s.replace("\\", "\\\\").replace("\"", "\\\"")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package datadog.gradle.plugin.config

import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.SourceSetContainer

class SupportedConfigPlugin : Plugin<Project> {
override fun apply(targetProject: Project) {
generateSupportedConfigurations(targetProject)
}

private fun generateSupportedConfigurations(targetProject: Project) {
val generateTask =
targetProject.tasks.register("generateSupportedConfigurations", ParseSupportedConfigurationsTask::class.java) {
jsonFile.set(project.file("src/main/resources/supported-configurations.json"))
destinationDirectory.set(project.layout.buildDirectory.dir("generated/supportedConfigurations"))
className.set("datadog.config.GeneratedSupportedConfigurations")
}

// Ensure Java compilation depends on the generated sources

val sourceset = targetProject.extensions.getByType(SourceSetContainer::class.java).named(SourceSet.MAIN_SOURCE_SET_NAME)
sourceset.configure {
java.srcDir(generateTask)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import datadog.communication.monitor.DDAgentStatsDClientManager;
import datadog.communication.monitor.Monitoring;
import datadog.communication.monitor.Recording;
import datadog.config.util.Strings;
import datadog.trace.api.telemetry.LogCollector;
import datadog.trace.util.Strings;
import java.nio.ByteBuffer;
import java.security.NoSuchAlgorithmException;
import java.util.HashSet;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import datadog.communication.serialization.EncodingCache;
import datadog.communication.serialization.ValueWriter;
import datadog.communication.serialization.Writable;
import datadog.trace.util.Strings;
import datadog.config.util.Strings;
import datadog.trace.util.stacktrace.StackTraceFrame;

public class StackTraceEventFrameWriter implements ValueWriter<StackTraceFrame> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import datadog.communication.serialization.EncodingCache;
import datadog.communication.serialization.ValueWriter;
import datadog.communication.serialization.Writable;
import datadog.trace.util.Strings;
import datadog.config.util.Strings;
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Is it needed to move Strings ?

Also, need to check with bootstrap there, as some packages maybe relocated.

import datadog.trace.util.stacktrace.StackTraceEvent;

public class StackTraceEventWriter implements ValueWriter<StackTraceEvent> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package datadog.communication.ddagent
import datadog.common.container.ContainerInfo
import datadog.communication.monitor.Monitoring
import datadog.trace.test.util.DDSpecification
import datadog.trace.util.Strings
import Strings
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: This feels wrong

import okhttp3.Call
import okhttp3.Headers
import okhttp3.HttpUrl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;

import de.thetaphi.forbiddenapis.SuppressForbidden;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -36,6 +37,7 @@ private EnvironmentVariables() {}
* @return The environment variable value, {@code defaultValue} if missing, can't be retrieved or
* the environment variable name is {@code null}.
*/
@SuppressForbidden
public static String getOrDefault(String name, String defaultValue) {
if (name == null) {
return defaultValue;
Expand All @@ -54,6 +56,7 @@ public static String getOrDefault(String name, String defaultValue) {
* @return All environment variables captured in an unmodifiable {@link Map}, or an empty {@link
* Map} if they can't be retrieved.
*/
@SuppressForbidden
public static Map<String, String> getAll() {
try {
return unmodifiableMap(new HashMap<>(System.getenv()));
Expand Down
Loading