Config Consistency Round 2 (#8489)

mhlidd · web-flow · commit 3c989a4ab1b5 · 2025-03-19T13:55:27.000-04:00
* trigger ci

* fixing tests

* fixing smoke test

* running CI

* fixing more smoke tests

* cleanup

* testing islogging128bittraceidenabled migration to config

* fixing small error

* adding to toString

* updating experimental features from list to set

* updating PR comments

* Adding test to verify behavior for configs under feature falg

* updating test

* updating config value when not under breaking_changed feature flag

* cleanup and nit responses

* nit pt 2
diff --git a/dd-java-agent/instrumentation/jboss-logmanager/src/main/java/datadog/trace/instrumentation/jbosslogmanager/ExtLogRecordInstrumentation.java b/dd-java-agent/instrumentation/jboss-logmanager/src/main/java/datadog/trace/instrumentation/jbosslogmanager/ExtLogRecordInstrumentation.java
@@ -15,7 +15,6 @@
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -113,8 +112,7 @@ public static void getMdcValue(
         case "dd.trace_id":
           if (context != null) {
             DDTraceId traceId = context.getTraceId();
-            if (traceId.toHighOrderLong() != 0
-                && InstrumenterConfig.get().isLogs128bTraceIdEnabled()) {
+            if (traceId.toHighOrderLong() != 0 && Config.get().isLogs128bitTraceIdEnabled()) {
               value = traceId.toHexString();
             } else {
               value = traceId.toString();
@@ -155,7 +153,7 @@ public static void onExit(
       if (context != null) {
         DDTraceId traceId = context.getTraceId();
         String traceIdValue =
-            InstrumenterConfig.get().isLogs128bTraceIdEnabled() && traceId.toHighOrderLong() != 0
+            Config.get().isLogs128bitTraceIdEnabled() && traceId.toHighOrderLong() != 0
                 ? traceId.toHexString()
                 : traceId.toString();
         correlationValues.put(CorrelationIdentifier.getTraceIdKey(), traceIdValue);
diff --git a/dd-java-agent/instrumentation/log4j-2.7/src/main/java/datadog/trace/instrumentation/log4j27/SpanDecoratingContextDataInjector.java b/dd-java-agent/instrumentation/log4j-2.7/src/main/java/datadog/trace/instrumentation/log4j27/SpanDecoratingContextDataInjector.java
@@ -11,7 +11,6 @@
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 import java.util.List;
@@ -57,7 +56,7 @@ public StringMap injectContextData(List<Property> list, StringMap reusable) {
     if (span != null) {
       DDTraceId traceId = span.context().getTraceId();
       String traceIdValue =
-          InstrumenterConfig.get().isLogs128bTraceIdEnabled() && traceId.toHighOrderLong() != 0
+          Config.get().isLogs128bitTraceIdEnabled() && traceId.toHighOrderLong() != 0
               ? traceId.toHexString()
               : traceId.toString();
       newContextData.putValue(CorrelationIdentifier.getTraceIdKey(), traceIdValue);
diff --git a/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/LoggingEventInstrumentation.java b/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/LoggingEventInstrumentation.java
@@ -13,7 +13,6 @@
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -95,8 +94,7 @@ public static void getMdcValue(
         case "dd.trace_id":
           if (context != null) {
             DDTraceId traceId = context.getTraceId();
-            if (traceId.toHighOrderLong() != 0
-                && InstrumenterConfig.get().isLogs128bTraceIdEnabled()) {
+            if (traceId.toHighOrderLong() != 0 && Config.get().isLogs128bitTraceIdEnabled()) {
               value = traceId.toHexString();
             } else {
               value = traceId.toString();
@@ -157,7 +155,7 @@ public static void onExit(
       if (context != null) {
         DDTraceId traceId = context.getTraceId();
         String traceIdValue =
-            InstrumenterConfig.get().isLogs128bTraceIdEnabled() && traceId.toHighOrderLong() != 0
+            Config.get().isLogs128bitTraceIdEnabled() && traceId.toHighOrderLong() != 0
                 ? traceId.toHexString()
                 : traceId.toString();
         mdc.put(CorrelationIdentifier.getTraceIdKey(), traceIdValue);
diff --git a/dd-java-agent/instrumentation/logback-1/src/main/java/datadog/trace/instrumentation/logback/LoggingEventInstrumentation.java b/dd-java-agent/instrumentation/logback-1/src/main/java/datadog/trace/instrumentation/logback/LoggingEventInstrumentation.java
@@ -15,7 +15,6 @@
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -90,7 +89,7 @@ public static void onExit(
       if (context != null) {
         DDTraceId traceId = context.getTraceId();
         String traceIdValue =
-            InstrumenterConfig.get().isLogs128bTraceIdEnabled() && traceId.toHighOrderLong() != 0
+            Config.get().isLogs128bitTraceIdEnabled() && traceId.toHighOrderLong() != 0
                 ? traceId.toHexString()
                 : traceId.toString();
         correlationValues.put(CorrelationIdentifier.getTraceIdKey(), traceIdValue);
diff --git a/dd-java-agent/instrumentation/tinylog-2/src/main/java/datadog/trace/instrumentation/tinylog2/LogEntryInstrumentation.java b/dd-java-agent/instrumentation/tinylog-2/src/main/java/datadog/trace/instrumentation/tinylog2/LogEntryInstrumentation.java
@@ -13,7 +13,6 @@
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.bootstrap.InstrumentationContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -83,7 +82,7 @@ public static void onExit(
       if (context != null) {
         DDTraceId traceId = context.getTraceId();
         String traceIdValue =
-            InstrumenterConfig.get().isLogs128bTraceIdEnabled() && traceId.toHighOrderLong() != 0
+            Config.get().isLogs128bitTraceIdEnabled() && traceId.toHighOrderLong() != 0
                 ? traceId.toHexString()
                 : traceId.toString();
         correlationValues.put(CorrelationIdentifier.getTraceIdKey(), traceIdValue);
diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/log/injection/LogContextInjectionTestBase.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/log/injection/LogContextInjectionTestBase.groovy
@@ -34,6 +34,10 @@ abstract class LogContextInjectionTestBase extends AgentTestRunner {
 
   abstract Map<String, Object> getMap()
 
+  def setup() {
+    injectSysConfig("logs.injection.enabled", "true")
+  }
+
   def "Log context shows trace and span ids for active scope"() {
     when:
     put("foo", "bar")
diff --git a/dd-smoke-tests/log-injection/src/test/groovy/datadog/smoketest/LogInjectionSmokeTest.groovy b/dd-smoke-tests/log-injection/src/test/groovy/datadog/smoketest/LogInjectionSmokeTest.groovy
@@ -41,7 +41,7 @@ abstract class LogInjectionSmokeTest extends AbstractSmokeTest {
   boolean noTags = false
 
   @Shared
-  boolean trace128bits = false
+  boolean trace128bits = true
 
   @Shared
   @AutoCleanup
@@ -88,9 +88,9 @@ abstract class LogInjectionSmokeTest extends AbstractSmokeTest {
       command.add("-Ddd.version=" as String)
       command.add("-Ddd.service.name=" as String)
     }
-    if (trace128bits) {
-      command.add("-Ddd.$TRACE_128_BIT_TRACEID_GENERATION_ENABLED=true" as String)
-      command.add("-Ddd.$TRACE_128_BIT_TRACEID_LOGGING_ENABLED=true" as String)
+    if (!trace128bits) {
+      command.add("-Ddd.$TRACE_128_BIT_TRACEID_GENERATION_ENABLED=false" as String)
+      command.add("-Ddd.$TRACE_128_BIT_TRACEID_LOGGING_ENABLED=false" as String)
     }
     if (supportsDirectLogSubmission()) {
       command.add("-Ddd.$GeneralConfig.AGENTLESS_LOG_SUBMISSION_ENABLED=true" as String)
diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java
@@ -104,6 +104,8 @@ public final class ConfigDefaults {
   static final boolean DEFAULT_PERF_METRICS_ENABLED = false;
   // No default constants for metrics statsd support -- falls back to jmxfetch values
 
+  // Change value to be false in new release. Until then, manually set logs_injection default
+  // value to false if config is under breaking changes flag
   static final boolean DEFAULT_LOGS_INJECTION_ENABLED = true;
 
   static final String DEFAULT_APPSEC_ENABLED = "inactive";
@@ -237,7 +239,7 @@ public final class ConfigDefaults {
   static final Set<String> DEFAULT_TRACE_EXPERIMENTAL_FEATURES_ENABLED = new HashSet<>();
 
   static final boolean DEFAULT_TRACE_128_BIT_TRACEID_GENERATION_ENABLED = true;
-  static final boolean DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = false;
+  static final boolean DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED = true;
   static final boolean DEFAULT_SECURE_RANDOM = false;
 
   public static final int DEFAULT_TRACE_X_DATADOG_TAGS_MAX_LENGTH = 512;
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
@@ -29,7 +29,6 @@
 import datadog.trace.api.DynamicConfig;
 import datadog.trace.api.EndpointTracker;
 import datadog.trace.api.IdGenerationStrategy;
-import datadog.trace.api.InstrumenterConfig;
 import datadog.trace.api.StatsDClient;
 import datadog.trace.api.TraceConfig;
 import datadog.trace.api.TracePropagationBehaviorExtract;
@@ -608,7 +607,7 @@ private CoreTracer(
             .setTracingTags(config.getMergedSpanTags())
             .apply();
 
-    this.logs128bTraceIdEnabled = InstrumenterConfig.get().isLogs128bTraceIdEnabled();
+    this.logs128bTraceIdEnabled = Config.get().isLogs128bitTraceIdEnabled();
     this.partialFlushMinSpans = partialFlushMinSpans;
     this.idGenerationStrategy =
         null == idGenerationStrategy
diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java
@@ -484,6 +484,7 @@ public static String getHostName() {
   private final boolean secureRandom;
 
   private final boolean trace128bitTraceIdGenerationEnabled;
+  private final boolean logs128bitTraceIdEnabled;
 
   private final Set<String> grpcIgnoredInboundMethods;
   private final Set<String> grpcIgnoredOutboundMethods;
@@ -665,6 +666,11 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
         configProvider.getBoolean(
             TRACE_128_BIT_TRACEID_GENERATION_ENABLED,
             DEFAULT_TRACE_128_BIT_TRACEID_GENERATION_ENABLED);
+
+    logs128bitTraceIdEnabled =
+        configProvider.getBoolean(
+            TRACE_128_BIT_TRACEID_LOGGING_ENABLED, DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED);
+
     if (secureRandom) {
       strategyName = "SECURE_RANDOM";
     }
@@ -1059,9 +1065,15 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
 
     clockSyncPeriod = configProvider.getInteger(CLOCK_SYNC_PERIOD, DEFAULT_CLOCK_SYNC_PERIOD);
 
-    logsInjectionEnabled =
-        configProvider.getBoolean(
-            LOGS_INJECTION_ENABLED, DEFAULT_LOGS_INJECTION_ENABLED, LOGS_INJECTION);
+    if (experimentalFeaturesEnabled.contains(
+        propertyNameToEnvironmentVariableName(LOGS_INJECTION))) {
+      logsInjectionEnabled =
+          configProvider.getBoolean(LOGS_INJECTION_ENABLED, false, LOGS_INJECTION);
+    } else {
+      logsInjectionEnabled =
+          configProvider.getBoolean(
+              LOGS_INJECTION_ENABLED, DEFAULT_LOGS_INJECTION_ENABLED, LOGS_INJECTION);
+    }
 
     dogStatsDNamedPipe = configProvider.getString(DOGSTATSD_NAMED_PIPE);
 
@@ -3490,6 +3502,10 @@ public boolean isTrace128bitTraceIdGenerationEnabled() {
     return trace128bitTraceIdGenerationEnabled;
   }
 
+  public boolean isLogs128bitTraceIdEnabled() {
+    return logs128bitTraceIdEnabled;
+  }
+
   public Set<String> getGrpcIgnoredInboundMethods() {
     return grpcIgnoredInboundMethods;
   }
@@ -4742,6 +4758,8 @@ public String toString() {
         + idGenerationStrategy
         + ", trace128bitTraceIdGenerationEnabled="
         + trace128bitTraceIdGenerationEnabled
+        + ", logs128bitTraceIdEnabled="
+        + logs128bitTraceIdEnabled
         + ", grpcIgnoredInboundMethods="
         + grpcIgnoredInboundMethods
         + ", grpcIgnoredOutboundMethods="
diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java
@@ -11,7 +11,6 @@
 import static datadog.trace.api.ConfigDefaults.DEFAULT_RUNTIME_CONTEXT_FIELD_INJECTION;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_SERIALVERSIONUID_FIELD_INJECTION;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_TELEMETRY_ENABLED;
-import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_ANNOTATIONS;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_ANNOTATION_ASYNC;
 import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_ENABLED;
@@ -52,7 +51,6 @@
 import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_USE_URL_CACHES;
 import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION;
 import static datadog.trace.api.config.TraceInstrumentationConfig.SERIALVERSIONUID_FIELD_INJECTION;
-import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_128_BIT_TRACEID_LOGGING_ENABLED;
 import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ANNOTATIONS;
 import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ANNOTATION_ASYNC;
 import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_CLASSES_EXCLUDE;
@@ -107,7 +105,6 @@ public class InstrumenterConfig {
   private final boolean codeOriginEnabled;
   private final boolean traceEnabled;
   private final boolean traceOtelEnabled;
-  private final boolean logs128bTraceIdEnabled;
   private final ProfilingEnablement profilingEnabled;
   private final boolean ciVisibilityEnabled;
   private final ProductActivation appSecActivation;
@@ -185,9 +182,7 @@ private InstrumenterConfig() {
             CODE_ORIGIN_FOR_SPANS_ENABLED, DEFAULT_CODE_ORIGIN_FOR_SPANS_ENABLED);
     traceEnabled = configProvider.getBoolean(TRACE_ENABLED, DEFAULT_TRACE_ENABLED);
     traceOtelEnabled = configProvider.getBoolean(TRACE_OTEL_ENABLED, DEFAULT_TRACE_OTEL_ENABLED);
-    logs128bTraceIdEnabled =
-        configProvider.getBoolean(
-            TRACE_128_BIT_TRACEID_LOGGING_ENABLED, DEFAULT_TRACE_128_BIT_TRACEID_LOGGING_ENABLED);
+
     profilingEnabled =
         ProfilingEnablement.of(
             configProvider.getString(PROFILING_ENABLED, String.valueOf(PROFILING_ENABLED_DEFAULT)));
@@ -342,10 +337,6 @@ public boolean isTraceOtelEnabled() {
     return traceOtelEnabled;
   }
 
-  public boolean isLogs128bTraceIdEnabled() {
-    return logs128bTraceIdEnabled;
-  }
-
   public boolean isProfilingEnabled() {
     return profilingEnabled.isActive();
   }
@@ -567,8 +558,6 @@ public String toString() {
         + traceEnabled
         + ", traceOtelEnabled="
         + traceOtelEnabled
-        + ", logs128bTraceIdEnabled="
-        + logs128bTraceIdEnabled
         + ", profilingEnabled="
         + profilingEnabled
         + ", ciVisibilityEnabled="
diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy
@@ -1899,16 +1899,17 @@ class ConfigTest extends DDSpecification {
 
   def "verify behavior of features under DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED"() {
     setup:
-    environmentVariables.set("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "DD_TAGS")
+    environmentVariables.set("DD_TRACE_EXPERIMENTAL_FEATURES_ENABLED", "DD_LOGS_INJECTION, DD_TAGS")
     environmentVariables.set("DD_TAGS", "env:test,aKey:aVal bKey:bVal cKey:")
 
     when:
     def config = new Config()
 
     then:
-    config.experimentalFeaturesEnabled == ["DD_TAGS"].toSet()
+    config.experimentalFeaturesEnabled == ["DD_LOGS_INJECTION", "DD_TAGS"].toSet()
 
     //verify expected behavior enabled under feature flag
+    config.logsInjectionEnabled == false
     config.globalTags == [env: "test", aKey: "aVal bKey:bVal cKey:"]
   }
 
@@ -1923,6 +1924,7 @@ class ConfigTest extends DDSpecification {
     config.experimentalFeaturesEnabled == [].toSet()
 
     //verify expected behavior when not enabled under feature flag
+    config.logsInjectionEnabled == true
     config.globalTags == [env:"test", aKey:"aVal", bKey:"bVal"]
   }
 
