From 341e015d83ce3007b2ee754468c7aa69e7436672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Mouch=C3=A8re?= <4822814+wmouchere@users.noreply.github.com> Date: Fri, 14 Mar 2025 10:05:36 +0100 Subject: [PATCH 1/4] Extract git tags from embedded git.properties and datadog_git.properties --- .../java/datadog/trace/bootstrap/Agent.java | 8 +++ .../trace/api/git/EmbeddedGitInfoBuilder.java | 52 ++++++++++++++----- .../api/git/EmbeddedGitInfoBuilderTest.groovy | 6 +-- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index e55d6e99ef9..0157a64dd1d 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -31,6 +31,8 @@ import datadog.trace.api.config.UsmConfig; import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.api.gateway.SubscriptionService; +import datadog.trace.api.git.EmbeddedGitInfoBuilder; +import datadog.trace.api.git.GitInfoProvider; import datadog.trace.api.profiling.ProfilingEnablement; import datadog.trace.api.scopemanager.ScopeListener; import datadog.trace.bootstrap.benchmark.StaticEventLogger; @@ -225,6 +227,12 @@ public static void start( } } + // Enable automatic fetching of git tags from datadog_git.properties only if CI Visibility is + // not enabled + if (!ciVisibilityEnabled) { + GitInfoProvider.INSTANCE.registerGitInfoBuilder(new EmbeddedGitInfoBuilder()); + } + boolean dataJobsEnabled = isFeatureEnabled(AgentFeature.DATA_JOBS); if (dataJobsEnabled) { log.info("Data Jobs Monitoring enabled, enabling spark integrations"); diff --git a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java index b3cbb3180d9..36d41295916 100644 --- a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java +++ b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java @@ -2,6 +2,8 @@ import java.io.IOException; import java.io.InputStream; +import java.util.Arrays; +import java.util.List; import java.util.Properties; import javax.annotation.Nullable; import org.slf4j.Logger; @@ -11,29 +13,55 @@ public class EmbeddedGitInfoBuilder implements GitInfoBuilder { private static final Logger log = LoggerFactory.getLogger(EmbeddedGitInfoBuilder.class); - private static final String EMBEDDED_GIT_PROPERTIES_FILE_NAME = "git.properties"; + // Spring boot fat jars and wars should have the git.properties file in a specific path, + // guaranteeing that it's not coming from a dependency + private static final String SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = + "BOOT-INF/classes/git.properties"; + private static final String SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = + "BOOT-INF/classes/datadog_git.properties"; + private static final String WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = + "WEB-INF/classes/git.properties"; + private static final String WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = + "WEB-INF/classes/datadog_git.properties"; + // If we can't find the files above, probably because we're not in a spring context, we can look + // at the root of the classpath. + // Since it could be tainted by dependencies, we're looking for a specific datadog_git.properties + // file. + private static final String EMBEDDED_DATADOOG_GIT_PROPERTIES_FILE_NAME = "datadog_git.properties"; - private final String resourceName; + private final List resourceNames; public EmbeddedGitInfoBuilder() { - this(EMBEDDED_GIT_PROPERTIES_FILE_NAME); + // Order is important here, from the most reliable sources to the least reliable ones + this( + Arrays.asList( + SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, + SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, + WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, + WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, + EMBEDDED_DATADOOG_GIT_PROPERTIES_FILE_NAME)); } - EmbeddedGitInfoBuilder(String resourceName) { - this.resourceName = resourceName; + EmbeddedGitInfoBuilder(List resourceNames) { + this.resourceNames = resourceNames; } @Override public GitInfo build(@Nullable String repositoryPath) { Properties gitProperties = new Properties(); - try (InputStream is = ClassLoader.getSystemResourceAsStream(resourceName)) { - if (is != null) { - gitProperties.load(is); - } else { - log.debug("Could not find embedded Git properties resource: {}", resourceName); + + for (String resourceName : resourceNames) { + try (InputStream is = ClassLoader.getSystemResourceAsStream(resourceName)) { + if (is != null) { + gitProperties.load(is); + // stop at the first git properties file found + break; + } else { + log.debug("Could not find embedded Git properties resource: {}", resourceName); + } + } catch (IOException e) { + log.error("Error reading embedded Git properties from {}", resourceName, e); } - } catch (IOException e) { - log.error("Error reading embedded Git properties from {}", resourceName, e); } String commitSha = gitProperties.getProperty("git.commit.id"); diff --git a/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy index 222c1715b07..84f1bc0300a 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy @@ -7,7 +7,7 @@ class EmbeddedGitInfoBuilderTest extends Specification { def "test no embedded git info"() { when: - def gitInfo = new EmbeddedGitInfoBuilder("non-existent-git.properties").build(null) + def gitInfo = new EmbeddedGitInfoBuilder(["non-existent-git.properties"]).build(null) then: gitInfo.isEmpty() @@ -16,7 +16,7 @@ class EmbeddedGitInfoBuilderTest extends Specification { def "test maven-plugin-generated git info"() { when: def mavenGitProperties = "datadog/trace/bootstrap/git/maven-git.properties" - def gitInfo = new EmbeddedGitInfoBuilder(mavenGitProperties).build(null) + def gitInfo = new EmbeddedGitInfoBuilder([mavenGitProperties]).build(null) then: gitInfo.repositoryURL == "git@github.com:DataDog/ciapp-test-resources.git" @@ -35,7 +35,7 @@ class EmbeddedGitInfoBuilderTest extends Specification { def "test gradle-plugin-generated git info"() { when: def gradleGitProperties = "datadog/trace/bootstrap/git/gradle-git.properties" - def gitInfo = new EmbeddedGitInfoBuilder(gradleGitProperties).build(null) + def gitInfo = new EmbeddedGitInfoBuilder([gradleGitProperties]).build(null) then: gitInfo.repositoryURL == "git@github.com:DataDog/ciapp-test-resources.git" From f440bd37aeed9b4321d4f5acdea69dfbc06b167f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Mouch=C3=A8re?= <4822814+wmouchere@users.noreply.github.com> Date: Fri, 14 Mar 2025 13:38:05 +0100 Subject: [PATCH 2/4] Prioritize datadog_git.properties even in spring boot context --- .../trace/api/git/EmbeddedGitInfoBuilder.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java index 36d41295916..fbf9d031194 100644 --- a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java +++ b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java @@ -15,14 +15,14 @@ public class EmbeddedGitInfoBuilder implements GitInfoBuilder { // Spring boot fat jars and wars should have the git.properties file in a specific path, // guaranteeing that it's not coming from a dependency - private static final String SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = - "BOOT-INF/classes/git.properties"; private static final String SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = "BOOT-INF/classes/datadog_git.properties"; - private static final String WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = - "WEB-INF/classes/git.properties"; + private static final String SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = + "BOOT-INF/classes/git.properties"; private static final String WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = "WEB-INF/classes/datadog_git.properties"; + private static final String WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = + "WEB-INF/classes/git.properties"; // If we can't find the files above, probably because we're not in a spring context, we can look // at the root of the classpath. // Since it could be tainted by dependencies, we're looking for a specific datadog_git.properties @@ -35,10 +35,10 @@ public EmbeddedGitInfoBuilder() { // Order is important here, from the most reliable sources to the least reliable ones this( Arrays.asList( - SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, - WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, + SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, + WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, EMBEDDED_DATADOOG_GIT_PROPERTIES_FILE_NAME)); } From b3bf09beeb1219e69d285eb62146bb22f15407a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Mouch=C3=A8re?= <4822814+wmouchere@users.noreply.github.com> Date: Tue, 18 Mar 2025 11:19:01 +0100 Subject: [PATCH 3/4] Add test for ordering --- .../trace/api/git/EmbeddedGitInfoBuilderTest.groovy | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy index 84f1bc0300a..69b04a2192b 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/git/EmbeddedGitInfoBuilderTest.groovy @@ -50,4 +50,13 @@ class EmbeddedGitInfoBuilderTest extends Specification { gitInfo.commit.committer.email == "nikita.tkachenko@datadoghq.com" gitInfo.commit.committer.iso8601Date == "2023-03-22T14:43:21+0100" } + + def "test embedded gitinfo has a lower priority than user supplied gitinfo"() { + when: + def embeddedGitInfoBuilder = new EmbeddedGitInfoBuilder() + def userSuppliedGitInfoBuilder = new UserSuppliedGitInfoBuilder() + + then: + embeddedGitInfoBuilder.order() > userSuppliedGitInfoBuilder.order() + } } From d63f22047a54531ecff4c5d5b8d0d058ba4bbd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Mouch=C3=A8re?= <4822814+wmouchere@users.noreply.github.com> Date: Thu, 27 Mar 2025 09:31:14 +0100 Subject: [PATCH 4/4] PR review: inline constants --- .../trace/api/git/EmbeddedGitInfoBuilder.java | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java index fbf9d031194..c388f5471d0 100644 --- a/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java +++ b/internal-api/src/main/java/datadog/trace/api/git/EmbeddedGitInfoBuilder.java @@ -13,33 +13,22 @@ public class EmbeddedGitInfoBuilder implements GitInfoBuilder { private static final Logger log = LoggerFactory.getLogger(EmbeddedGitInfoBuilder.class); - // Spring boot fat jars and wars should have the git.properties file in a specific path, - // guaranteeing that it's not coming from a dependency - private static final String SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = - "BOOT-INF/classes/datadog_git.properties"; - private static final String SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = - "BOOT-INF/classes/git.properties"; - private static final String WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME = - "WEB-INF/classes/datadog_git.properties"; - private static final String WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME = - "WEB-INF/classes/git.properties"; - // If we can't find the files above, probably because we're not in a spring context, we can look - // at the root of the classpath. - // Since it could be tainted by dependencies, we're looking for a specific datadog_git.properties - // file. - private static final String EMBEDDED_DATADOOG_GIT_PROPERTIES_FILE_NAME = "datadog_git.properties"; - private final List resourceNames; public EmbeddedGitInfoBuilder() { // Order is important here, from the most reliable sources to the least reliable ones this( Arrays.asList( - SPRING_BOOT_JAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, - SPRING_BOOT_JAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, - WAR_EMBEDDED_DATADOG_GIT_PROPERTIES_FILE_NAME, - WAR_EMBEDDED_GIT_PROPERTIES_FILE_NAME, - EMBEDDED_DATADOOG_GIT_PROPERTIES_FILE_NAME)); + // Spring boot fat jars and wars should have the git.properties file in the following + // specific paths, guaranteeing that it's not coming from a dependency + "BOOT-INF/classes/datadog_git.properties", + "BOOT-INF/classes/git.properties", + "WEB-INF/classes/datadog_git.properties", + "WEB-INF/classes/git.properties", + // If we can't find the files above, probably because we're not in a spring context, we + // can look at the root of the classpath. Since it could be tainted by dependencies, + // we're looking for a specific datadog_git.properties file. + "datadog_git.properties")); } EmbeddedGitInfoBuilder(List resourceNames) {