Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import static datadog.trace.util.AgentThreadFactory.AgentThread.PROFILER_STARTUP;
import static datadog.trace.util.AgentThreadFactory.AgentThread.TRACE_STARTUP;
import static datadog.trace.util.AgentThreadFactory.newAgentThread;
import static datadog.trace.util.Strings.propertyNameToSystemPropertyName;
import static datadog.trace.util.Strings.toEnvVar;
import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName;
import static datadog.trace.util.ConfigStrings.toEnvVar;

import datadog.environment.EnvironmentVariables;
import datadog.environment.JavaVirtualMachine;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import datadog.trace.api.Config
import datadog.trace.api.civisibility.config.TestFQN
import datadog.trace.api.config.CiVisibilityConfig
import datadog.trace.api.config.GeneralConfig
import datadog.trace.util.Strings
import spock.lang.Specification
import spock.util.environment.Jvm

import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName

abstract class CiVisibilitySmokeTest extends Specification {
static final List<String> SMOKE_IGNORED_TAGS = ["content.meta.['_dd.integration']"]

Expand Down Expand Up @@ -73,7 +74,7 @@ abstract class CiVisibilitySmokeTest extends Specification {
argMap.put(CiVisibilityConfig.CIVISIBILITY_DEBUG_PORT, "5055")
}

String agentArgs = argMap.collect { k, v -> "${Strings.propertyNameToSystemPropertyName(k)}=${v}" }.join(",")
String agentArgs = argMap.collect { k, v -> "${propertyNameToSystemPropertyName(k)}=${v}" }.join(",")
arguments += "-javaagent:${AGENT_JAR}=${agentArgs}".toString()

return arguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_HOST;
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_PORT;
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.extractContextAndGetSpanContext;
import static datadog.trace.util.Strings.propertyNameToSystemPropertyName;
import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName;

import datadog.environment.SystemProperties;
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.civisibility.domain.buildsystem;

import static datadog.context.propagation.Propagators.defaultPropagator;
import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName;

import datadog.communication.ddagent.TracerVersion;
import datadog.context.propagation.CarrierSetter;
Expand Down Expand Up @@ -31,7 +32,6 @@
import datadog.trace.civisibility.ipc.SignalType;
import datadog.trace.civisibility.source.LinesResolver;
import datadog.trace.civisibility.source.SourcePathResolver;
import datadog.trace.util.Strings;
import java.net.InetSocketAddress;
import java.nio.file.Path;
import java.util.Collection;
Expand Down Expand Up @@ -148,81 +148,72 @@ private Map<String, String> getPropertiesPropagatedToChildProcess(
}

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED),
Boolean.toString(executionSettings.isItrEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_ENABLED),
Boolean.toString(executionSettings.isCodeCoverageEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_TEST_SKIPPING_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_TEST_SKIPPING_ENABLED),
Boolean.toString(executionSettings.isTestSkippingEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_FLAKY_RETRY_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_FLAKY_RETRY_ENABLED),
Boolean.toString(executionSettings.isFlakyTestRetriesEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_IMPACTED_TESTS_DETECTION_ENABLED),
Boolean.toString(executionSettings.isImpactedTestsDetectionEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_EARLY_FLAKE_DETECTION_ENABLED),
Boolean.toString(executionSettings.getEarlyFlakeDetectionSettings().isEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.TEST_MANAGEMENT_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.TEST_MANAGEMENT_ENABLED),
Boolean.toString(executionSettings.getTestManagementSettings().isEnabled()));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.TEST_FAILED_TEST_REPLAY_ENABLED),
propertyNameToSystemPropertyName(CiVisibilityConfig.TEST_FAILED_TEST_REPLAY_ENABLED),
Boolean.toString(executionSettings.isFailedTestReplayEnabled()));

// explicitly disable build instrumentation in child processes,
// because some projects run "embedded" Maven/Gradle builds as part of their integration tests,
// and we don't want to show those as if they were regular build executions
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_BUILD_INSTRUMENTATION_ENABLED),
Boolean.toString(false));

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_INJECTED_TRACER_VERSION),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_INJECTED_TRACER_VERSION),
TracerVersion.TRACER_VERSION);

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(GeneralConfig.SERVICE_NAME), serviceName);
propertyNameToSystemPropertyName(GeneralConfig.SERVICE_NAME), serviceName);
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(GeneralConfig.SERVICE_NAME_SET_BY_USER),
propertyNameToSystemPropertyName(GeneralConfig.SERVICE_NAME_SET_BY_USER),
String.valueOf(userProvidedServiceName));
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_MODULE_NAME),
moduleName);
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_MODULE_NAME), moduleName);
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_TEST_COMMAND),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_TEST_COMMAND),
startCommand);

propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_HOST),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_HOST),
signalServerAddress != null ? signalServerAddress.getHostName() : null);
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_PORT),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_SIGNAL_SERVER_PORT),
String.valueOf(signalServerAddress != null ? signalServerAddress.getPort() : 0));

List<String> coverageIncludedPackages = sessionSettings.getCoverageIncludedPackages();
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_INCLUDES),
propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_INCLUDES),
String.join(":", coverageIncludedPackages));

if (jacocoAgent != null && !config.isCiVisibilityCoverageLinesDisabled()) {
Expand All @@ -234,7 +225,7 @@ private Map<String, String> getPropertiesPropagatedToChildProcess(
// This setting is only relevant if per-test code coverage is enabled,
// otherwise it has no effect.
propagatedSystemProperties.put(
Strings.propertyNameToSystemPropertyName(
propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_LINES_ENABLED),
Boolean.toString(true));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import datadog.trace.civisibility.ci.env.CiEnvironmentImpl
import datadog.trace.civisibility.git.CILocalGitInfoBuilder
import datadog.trace.civisibility.git.CIProviderGitInfoBuilder
import datadog.trace.civisibility.git.tree.GitClient
import datadog.trace.util.Strings
import org.junit.Rule
import org.junit.contrib.java.lang.system.EnvironmentVariables
import org.junit.contrib.java.lang.system.RestoreSystemProperties
Expand All @@ -17,6 +16,8 @@ import spock.lang.Specification
import java.nio.file.Path
import java.nio.file.Paths

import static datadog.trace.util.ConfigStrings.propertyNameToEnvironmentVariableName

abstract class CITagsProviderTest extends Specification {

static final CI_WORKSPACE_PATH_FOR_TESTS = "ci/ci_workspace_for_tests"
Expand Down Expand Up @@ -70,7 +71,7 @@ abstract class CITagsProviderTest extends Specification {
environmentVariables.set(it.key, it.value)
}

environmentVariables.set(Strings.propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_COMMIT_SHA), "1234567890123456789012345678901234567890")
environmentVariables.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_COMMIT_SHA), "1234567890123456789012345678901234567890")

when:
CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
Expand All @@ -89,7 +90,7 @@ abstract class CITagsProviderTest extends Specification {
environmentVariables.set(it.key, it.value)
}

environmentVariables.set(Strings.propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_REPOSITORY_URL), "local supplied repo url")
environmentVariables.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_REPOSITORY_URL), "local supplied repo url")

when:
CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import datadog.trace.core.PendingTrace
import datadog.trace.core.datastreams.DefaultDataStreamsMonitoring
import datadog.trace.test.util.DDSpecification
import datadog.trace.util.AgentTaskScheduler
import datadog.trace.util.Strings
import datadog.trace.util.ConfigStrings
import de.thetaphi.forbiddenapis.SuppressForbidden
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
import groovy.transform.stc.ClosureParams
Expand Down Expand Up @@ -117,7 +117,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
StringBuilder ddEnvVars = new StringBuilder()
for (Map.Entry<Object, Object> entry : System.getProperties().entrySet()) {
if (entry.getKey().toString().startsWith("dd.")) {
ddEnvVars.append(Strings.systemPropertyNameToEnvironmentVariableName(entry.getKey().toString()))
ddEnvVars.append(ConfigStrings.systemPropertyNameToEnvironmentVariableName(entry.getKey().toString()))
.append("=").append(entry.getValue()).append(",")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[
+ "datadog.trace.api.ResolverCacheConfig$5:build_time,"
+ "datadog.trace.api.TracePropagationStyle:build_time,"
+ "datadog.trace.api.TracePropagationBehaviorExtract:build_time,"
+ "datadog.trace.api.telemetry.OtelEnvMetricCollector:build_time,"
+ "datadog.trace.api.telemetry.OtelEnvMetricCollectorImpl:build_time,"
+ "datadog.trace.api.profiling.ProfilingEnablement:build_time,"
+ "datadog.trace.bootstrap.config.provider.ConfigConverter:build_time,"
+ "datadog.trace.bootstrap.config.provider.ConfigConverter$ValueOfLookup:build_time,"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import datadog.trace.api.Config
import datadog.trace.api.civisibility.domain.BuildSessionSettings
import datadog.trace.api.config.CiVisibilityConfig
import datadog.trace.bootstrap.DatadogClassLoader
import datadog.trace.util.ConfigStrings
import datadog.trace.util.Strings
import org.gradle.api.Project
import org.gradle.api.Task
Expand Down Expand Up @@ -73,7 +74,7 @@ class GradleProjectConfigurator {
}

String taskPath = task.getPath()
jvmArgs.add("-D" + Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_MODULE_NAME) + '=' + taskPath)
jvmArgs.add("-D" + ConfigStrings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_MODULE_NAME) + '=' + taskPath)

jvmArgs.add("-javaagent:" + config.ciVisibilityAgentJarFile.toPath())

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package datadog.trace.instrumentation.gradle;

import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_INJECTED_TRACER_VERSION;
import static datadog.trace.util.Strings.propertyNameToSystemPropertyName;
import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName;

import datadog.environment.SystemProperties;
import datadog.trace.api.Config;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package datadog.smoketest


import datadog.trace.api.civisibility.CIConstants
import datadog.trace.api.config.CiVisibilityConfig
import datadog.trace.api.config.GeneralConfig
Expand All @@ -20,7 +19,11 @@ import spock.util.environment.Jvm

import javax.xml.parsers.DocumentBuilder
import javax.xml.parsers.DocumentBuilderFactory
import java.nio.file.*
import java.nio.file.FileVisitResult
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.SimpleFileVisitor
import java.nio.file.attribute.BasicFileAttributes
import java.util.concurrent.TimeUnit
import java.util.concurrent.TimeoutException
Expand Down
1 change: 1 addition & 0 deletions internal-api/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ dependencies {
api(project(":components:environment"))
api(project(":components:json"))
api(project(":components:yaml"))
api(project(":utils:config-utils"))
api(project(":utils:time-utils"))

// has to be loaded by system classloader:
Expand Down
12 changes: 10 additions & 2 deletions internal-api/src/main/java/datadog/trace/api/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@
import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
import static datadog.trace.util.CollectionUtils.tryMakeImmutableList;
import static datadog.trace.util.CollectionUtils.tryMakeImmutableSet;
import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;
import static datadog.trace.util.ConfigStrings.propertyNameToEnvironmentVariableName;

import datadog.environment.EnvironmentVariables;
import datadog.environment.JavaVirtualMachine;
Expand All @@ -652,11 +652,14 @@
import datadog.trace.api.profiling.ProfilingEnablement;
import datadog.trace.api.rum.RumInjectorConfig;
import datadog.trace.api.rum.RumInjectorConfig.PrivacyLevel;
import datadog.trace.api.telemetry.OtelEnvMetricCollectorImpl;
import datadog.trace.api.telemetry.OtelEnvMetricCollectorProvider;
import datadog.trace.bootstrap.config.provider.CapturedEnvironmentConfigSource;
import datadog.trace.bootstrap.config.provider.ConfigProvider;
import datadog.trace.bootstrap.config.provider.SystemPropertiesConfigSource;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.context.TraceScope;
import datadog.trace.util.ConfigStrings;
import datadog.trace.util.PidHelper;
import datadog.trace.util.RandomUtils;
import datadog.trace.util.Strings;
Expand Down Expand Up @@ -1239,6 +1242,11 @@ public static String getHostName() {

private final RumInjectorConfig rumInjectorConfig;

static {
// Bind telemetry collector to config module before initializing ConfigProvider
OtelEnvMetricCollectorProvider.register(OtelEnvMetricCollectorImpl.getInstance());
}

Comment on lines +1245 to +1249
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, is there a reason that you chose to register the instance here instead of when it is initialized in OtelEnvMetricCollectorImpl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

instead of when it is initialized in OtelEnvMetricCollectorImpl

There is nobody to initialize it. The code that needs it can't have a dependency on it (to avoid circular dependency).
So the initialization should come from the :internal-api module.

// Read order: System Properties -> Env Variables, [-> properties file], [-> default value]
private Config() {
this(ConfigProvider.createDefault());
Expand Down Expand Up @@ -5246,7 +5254,7 @@ static String initHostName() {
} catch (Throwable t) {
// Ignore
}
possibleHostname = Strings.trim(possibleHostname);
possibleHostname = ConfigStrings.trim(possibleHostname);
if (!possibleHostname.isEmpty()) {
log.debug("Determined hostname from file {}", hostNameFile);
return possibleHostname;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLE_RATE;
import static datadog.trace.api.config.TracerConfig.TRACE_SAMPLING_RULES;
import static datadog.trace.util.CollectionUtils.tryMakeImmutableMap;
import static datadog.trace.util.ConfigStrings.normalizedHeaderTag;
import static datadog.trace.util.ConfigStrings.trim;

import datadog.trace.api.sampling.SamplingRule.SpanSamplingRule;
import datadog.trace.api.sampling.SamplingRule.TraceSamplingRule;
import datadog.trace.util.Strings;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -248,11 +249,11 @@ static Map<String, String> cleanMapping(
}

static String key(Map.Entry<String, String> association) {
return Strings.trim(association.getKey());
return trim(association.getKey());
}

static String value(Map.Entry<String, String> association) {
return Strings.trim(association.getValue());
return trim(association.getValue());
}

static String lowerKey(Map.Entry<String, String> association) {
Expand All @@ -263,7 +264,7 @@ static String requestTag(Map.Entry<String, String> association) {
String requestTag = value(association);
if (requestTag.isEmpty()) {
// normalization is only applied when generating default tag names; see ConfigConverter
requestTag = "http.request.headers." + Strings.normalizedHeaderTag(association.getKey());
requestTag = "http.request.headers." + normalizedHeaderTag(association.getKey());
}
return requestTag;
}
Expand All @@ -272,7 +273,7 @@ static String responseTag(Map.Entry<String, String> association) {
String responseTag = value(association);
if (responseTag.isEmpty()) {
// normalization is only applied when generating default tag names; see ConfigConverter
responseTag = "http.response.headers." + Strings.normalizedHeaderTag(association.getKey());
responseTag = "http.response.headers." + normalizedHeaderTag(association.getKey());
}
return responseTag;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@
import static datadog.trace.util.CollectionUtils.tryMakeImmutableSet;

import datadog.trace.api.profiling.ProfilingEnablement;
import datadog.trace.api.telemetry.OtelEnvMetricCollectorImpl;
import datadog.trace.api.telemetry.OtelEnvMetricCollectorProvider;
import datadog.trace.bootstrap.config.provider.ConfigProvider;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -172,6 +174,11 @@ public class InstrumenterConfig {

private final boolean rumEnabled;

static {
// Bind telemetry collector to config module before initializing ConfigProvider
OtelEnvMetricCollectorProvider.register(OtelEnvMetricCollectorImpl.getInstance());
}
Comment on lines +177 to +180
Copy link
Contributor

Choose a reason for hiding this comment

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

question: For my understanding this registration code is called here and in Config class, why is that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two modules are tightly coupled together.
I inverted the dependency and needed somewhere for the parent module to register the service implementation.
I did it into the two main entry points as object lifecycle are screwed due to heavy use of static and singleton.


private InstrumenterConfig() {
this(ConfigProvider.createDefault());
}
Expand Down
Loading