Make DD_LOG_BACKTRACE disabled by default again + make it mutually exclusive with DD_CRASHTRACKING_ENABLED

iamluc · iamluc · commit 0d088b1e4624 · 2024-09-13T17:57:18.000+02:00
diff --git a/ext/configuration.h b/ext/configuration.h
@@ -91,10 +91,8 @@ enum ddtrace_sampling_rules_format {
     "(?i)(?:(?:\"|%22)?)(?:(?:old[-_]?|new[-_]?)?p(?:ass)?w(?:or)?d(?:1|2)?|pass(?:[-_]?phrase)?|secret|(?:api[-_]?|private[-_]?|public[-_]?|access[-_]?|secret[-_]?|app(?:lication)?[-_]?)key(?:[-_]?id)?|token|consumer[-_]?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)(?:(?:\\s|%20)*(?:=|%3D)[^&]+|(?:\"|%22)(?:\\s|%20)*(?::|%3A)(?:\\s|%20)*(?:\"|%22)(?:%2[^2]|%[^2]|[^\"%])+(?:\"|%22))|(?:bearer(?:\\s|%20)+[a-z0-9._\\-]+|token(?::|%3A)[a-z0-9]{13}|gh[opsu]_[0-9a-zA-Z]{36}|ey[I-L](?:[\\w=-]|%3D)+\\.ey[I-L](?:[\\w=-]|%3D)+(?:\\.(?:[\\w.+/=-]|%3D|%2F|%2B)+)?|-{5}BEGIN(?:[a-z\\s]|%20)+PRIVATE(?:\\s|%20)KEY-{5}[^\\-]+-{5}END(?:[a-z\\s]|%20)+PRIVATE(?:\\s|%20)KEY(?:-{5})?(?:\\n|%0A)?|(?:ssh-(?:rsa|dss)|ecdsa-[a-z0-9]+-[a-z0-9]+)(?:\\s|%20|%09)+(?:[a-z0-9/.+]|%2F|%5C|%2B){100,}(?:=|%3D)*(?:(?:\\s|%20|%09)+[a-z0-9._-]+)?)"
 
 #ifdef __SANITIZE_ADDRESS__
-#define DD_LOG_BACKTRACE_DEFAULT "false"
 #define DD_CRASHTRACKING_ENABLED_DEFAULT "false"
 #else
-#define DD_LOG_BACKTRACE_DEFAULT "true"
 #define DD_CRASHTRACKING_ENABLED_DEFAULT "true"
 #endif
 
@@ -171,7 +169,7 @@ enum ddtrace_sampling_rules_format {
     CONFIG(INT, DD_TRACE_AGENT_CONNECT_TIMEOUT, DD_CFG_EXPSTR(DD_TRACE_AGENT_CONNECT_TIMEOUT_VAL),             \
            .ini_change = zai_config_system_ini_change)                                                         \
     CONFIG(INT, DD_TRACE_DEBUG_PRNG_SEED, "-1", .ini_change = ddtrace_reseed_seed_change)                      \
-    CONFIG(BOOL, DD_LOG_BACKTRACE, DD_LOG_BACKTRACE_DEFAULT)                                                   \
+    CONFIG(BOOL, DD_LOG_BACKTRACE, "false")                                                                    \
     CONFIG(BOOL, DD_CRASHTRACKING_ENABLED, DD_CRASHTRACKING_ENABLED_DEFAULT)                                   \
     CONFIG(BOOL, DD_TRACE_GENERATE_ROOT_SPAN, "true", .ini_change = ddtrace_span_alter_root_span_config)       \
     CONFIG(INT, DD_TRACE_SPANS_LIMIT, "1000")                                                                  \
diff --git a/ext/signals.c b/ext/signals.c
@@ -169,8 +169,13 @@ void ddtrace_signals_first_rinit(void) {
         return;
     }
 
-    bool install_handler = get_DD_TRACE_HEALTH_METRICS_ENABLED();
+    bool crashtracker_installed = false;
+    if (get_DD_INSTRUMENTATION_TELEMETRY_ENABLED() && get_DD_CRASHTRACKING_ENABLED()) {
+        crashtracker_installed = true;
+        ddtrace_init_crashtracker();
+    }
 
+    bool install_handler = get_DD_TRACE_HEALTH_METRICS_ENABLED();
 #if DDTRACE_HAVE_BACKTRACE
     install_handler |= get_DD_LOG_BACKTRACE();
 #endif
@@ -180,6 +185,11 @@ void ddtrace_signals_first_rinit(void) {
      * stack overflows.
      */
     if (install_handler) {
+        if (crashtracker_installed) {
+            LOG(WARN, "Settings 'datadog.log_backtrace' and 'datadog.crashtracking_enabled' are mutually exclusive. Cannot enable the backtrace.");
+            return;
+        }
+
         size_t stack_size = SIGSTKSZ < MIN_STACKSZ ? MIN_STACKSZ : SIGSTKSZ;
         if ((ddtrace_altstack.ss_sp = malloc(stack_size))) {
             ddtrace_altstack.ss_size = stack_size;
@@ -192,10 +202,6 @@ void ddtrace_signals_first_rinit(void) {
             }
         }
     }
-
-    if (get_DD_INSTRUMENTATION_TELEMETRY_ENABLED() && get_DD_CRASHTRACKING_ENABLED()) {
-        ddtrace_init_crashtracker();
-    }
 }
 
 void ddtrace_signals_mshutdown(void) {
diff --git a/tests/ext/crashtracker_and_backtrace_are_mutually_exclusive.phpt b/tests/ext/crashtracker_and_backtrace_are_mutually_exclusive.phpt
@@ -0,0 +1,22 @@
+--TEST--
+Settings 'datadog.log_backtrace' and 'datadog.crashtracking_enabled' are mutually exclusive
+--SKIPIF--
+<?php
+if (PHP_OS != "Linux") die('skip: Crashtracker/backtrace are only available on Linux');
+if (getenv('DD_TRACE_CLI_ENABLED') === '0') die("skip: tracer is disabled");
+?>
+--ENV--
+DD_TRACE_LOG_LEVEL=warn,span=off,startup=off
+DD_LOG_BACKTRACE=1
+DD_CRASHTRACKING_ENABLED=1
+--INI--
+datadog.trace.log_file=file://stdout
+--FILE--
+<?php
+
+print_r(1);
+
+?>
+--EXPECTF--
+[ddtrace] [warning] Settings 'datadog.log_backtrace' and 'datadog.crashtracking_enabled' are mutually exclusive. Cannot enable the backtrace.
+1
diff --git a/tests/ext/crashtracker_segfault_disabled.phpt b/tests/ext/crashtracker_segfault_disabled.phpt
@@ -50,5 +50,6 @@ $rr->waitForRequest(function ($request) {
 
 ?>
 --EXPECTF--
+%A
 Fatal error: Uncaught Exception: wait for replay timeout in %s
 %A
