-
Notifications
You must be signed in to change notification settings - Fork 312
Implement Config Inversion with Default Strictness of Warning
#9181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
fd62009
a756eb2
96e2546
6a87970
b062c96
6769bda
06ed2fd
8dd50e0
9660b1a
f8d2a8c
b68f0b2
dc5d83b
fe2eeb9
229c38d
77ddecd
eb6908a
972eef4
81c12f4
98c57b5
990c0ce
5bdd8cf
52c05b4
5192a8a
f128de8
734f9c7
e9ce233
90f536d
7a1517e
5ca61c0
abc0b64
97db41f
04bde01
9a764c9
e10e0a9
340eeef
ea83e25
4b602c1
4bdb960
962490d
c7aa94d
8068724
47a8f6a
94f392c
6c2a551
fe697ff
1aefa7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Note, in the example above I'm leveraging 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// 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} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: space
Suggested change
|
||||||||||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Since
Suggested change
Note I'm referring to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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( | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Maybe you can also add
Suggested change
|
||||||||
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 {") | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: This doesn;t have any effect really, but it just gives the same prefix to the name.