Avoid resetting the sidecar session on fork

And add a few more log lines Also fix some ordering assumptions around unrwap(). Signed-off-by: Bob Weinand <[email protected]>
DataDog · Feb 3, 2025 · d76b7d7 · d76b7d7
1 parent 2af23c4
commit d76b7d7
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 34 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Makefile b/Makefile
@@ -1046,6 +1046,53 @@ TEST_WEB_83 := \
 	test_web_custom \
 	test_web_zend_1_21
 
+TEST_INTEGRATIONS_84 := \
+	test_integrations_amqp2 \
+	test_integrations_amqp_latest \
+	test_integrations_curl \
+	test_integrations_deferred_loading \
+	test_integrations_kafka \
+	test_integrations_laminaslog2 \
+	test_integrations_memcache \
+	test_integrations_memcached \
+	test_integrations_mongodb_latest \
+	test_integrations_monolog1 \
+	test_integrations_monolog2 \
+	test_integrations_monolog_latest \
+	test_integrations_mysqli \
+	test_integrations_openai_latest \
+	test_opentelemetry_1 \
+	test_integrations_googlespanner_latest \
+	test_integrations_guzzle_latest \
+	test_integrations_pcntl \
+	test_integrations_pdo \
+	test_integrations_elasticsearch7 \
+	test_integrations_elasticsearch_latest \
+	test_integrations_phpredis5 \
+	test_integrations_predis_latest \
+	test_integrations_frankenphp \
+	test_integrations_roadrunner \
+	test_integrations_sqlsrv \
+	test_integrations_swoole_5 \
+	test_opentracing_10
+
+TEST_WEB_84 := \
+	test_metrics \
+	test_web_cakephp_latest \
+	test_web_codeigniter_22 \
+	test_web_codeigniter_31 \
+	test_web_laravel_octane_latest \
+	test_web_lumen_100 \
+	test_web_nette_latest \
+	test_web_slim_312 \
+	test_web_symfony_latest \
+	test_web_wordpress_59 \
+	test_web_wordpress_61 \
+	test_web_custom \
+	test_web_zend_1_21
+
+# to check: test_web_drupal_95, test_web_laravel_latest, test_web_slim_latest
+
 FILTER ?= .
 MAX_RETRIES := 3
 

diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h
@@ -194,7 +194,8 @@ ddog_MaybeError ddog_sidecar_session_set_config(struct ddog_SidecarTransport **t
                                                 const enum ddog_RemoteConfigProduct *remote_config_products,
                                                 uintptr_t remote_config_products_count,
                                                 const enum ddog_RemoteConfigCapabilities *remote_config_capabilities,
-                                                uintptr_t remote_config_capabilities_count);
+                                                uintptr_t remote_config_capabilities_count,
+                                                bool is_fork);
 
 /**
  * Sends a trace to the sidecar via shared memory.

diff --git a/ext/sidecar.c b/ext/sidecar.c
@@ -36,7 +36,17 @@ static void ddtrace_set_resettable_sidecar_globals(void) {
     ddtrace_sidecar_instance_id = ddog_sidecar_instanceId_build(session_id, runtime_id);
 }
 
-ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
+static inline void dd_set_endpoint_test_token(ddog_Endpoint *endpoint) {
+    if (zai_config_is_initialized()) {
+        if (ZSTR_LEN(get_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) {
+            ddog_endpoint_set_test_token(endpoint, dd_zend_string_to_CharSlice(get_DD_TRACE_AGENT_TEST_SESSION_TOKEN()));
+        }
+    } else if (ZSTR_LEN(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) {
+        ddog_endpoint_set_test_token(endpoint, dd_zend_string_to_CharSlice(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN()));
+    }
+}
+
+static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) {
     // Should not happen, unless the agent url is malformed
     if (!ddtrace_endpoint) {
         return NULL;
@@ -51,9 +61,7 @@ ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
         free(dogstatsd_url);
     }
 
-    if (ZSTR_LEN(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) {
-        ddog_endpoint_set_test_token(dogstatsd_endpoint, dd_zend_string_to_CharSlice(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN()));
-    }
+    dd_set_endpoint_test_token(dogstatsd_endpoint);
 
 #ifdef _WIN32
     DDOG_PHP_FUNCTION = (const uint8_t *)zend_hash_func;
@@ -87,7 +95,8 @@ ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
                                     DDTRACE_REMOTE_CONFIG_PRODUCTS.ptr,
                                     DDTRACE_REMOTE_CONFIG_PRODUCTS.len,
                                     DDTRACE_REMOTE_CONFIG_CAPABILITIES.ptr,
-                                    DDTRACE_REMOTE_CONFIG_CAPABILITIES.len);
+                                    DDTRACE_REMOTE_CONFIG_CAPABILITIES.len,
+                                    is_fork);
 
     ddog_endpoint_drop(dogstatsd_endpoint);
 
@@ -98,6 +107,10 @@ ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
     return sidecar_transport;
 }
 
+ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
+    return dd_sidecar_connection_factory_ex(false);
+}
+
 bool ddtrace_sidecar_maybe_enable_appsec(bool *appsec_features, bool *appsec_config) {
     *appsec_features = false;
     *appsec_config = false;
@@ -165,10 +178,12 @@ void ddtrace_reset_sidecar(void) {
 
     if (ddtrace_sidecar) {
         ddog_sidecar_transport_drop(ddtrace_sidecar);
-        ddtrace_sidecar = dd_sidecar_connection_factory();
+        ddtrace_sidecar = dd_sidecar_connection_factory_ex(true);
         if (!ddtrace_sidecar && ddtrace_endpoint) { // Something went wrong
             ddog_endpoint_drop(ddtrace_endpoint);
             ddtrace_endpoint = NULL;
+        } else {
+            ddtrace_sidecar_submit_root_span_data();
         }
     }
 }
@@ -187,9 +202,8 @@ ddog_Endpoint *ddtrace_sidecar_agent_endpoint(void) {
         free(agent_url);
     }
 
-
-    if (agent_endpoint && ZSTR_LEN(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN())) {
-        ddog_endpoint_set_test_token(agent_endpoint, dd_zend_string_to_CharSlice(get_global_DD_TRACE_AGENT_TEST_SESSION_TOKEN()));
+    if (agent_endpoint) {
+        dd_set_endpoint_test_token(agent_endpoint);
     }
 
     return agent_endpoint;
@@ -440,6 +454,7 @@ void ddtrace_sidecar_rshutdown(void) {
 bool ddtrace_alter_test_session_token(zval *old_value, zval *new_value, zend_string *new_str) {
     UNUSED(old_value, new_str);
     if (ddtrace_sidecar) {
+        ddog_endpoint_set_test_token(ddtrace_endpoint, dd_zend_string_to_CharSlice(Z_STR_P(new_value)));
         ddog_CharSlice session_id = (ddog_CharSlice) {.ptr = (char *) dd_sidecar_formatted_session_id, .len = sizeof(dd_sidecar_formatted_session_id)};
         ddtrace_ffi_try("Failed updating test session token",
                         ddog_sidecar_set_test_session_token(&ddtrace_sidecar, session_id, dd_zend_string_to_CharSlice(Z_STR_P(new_value))));

diff --git a/libdatadog b/libdatadog
diff --git a/tests/Common/CLITestCase.php b/tests/Common/CLITestCase.php
@@ -92,9 +92,6 @@ public function executeCommand($arguments = '', $overrideEnvs = [])
         $arguments = escapeshellarg($arguments);
         $commandToExecute = "$envs " . PHP_BINARY . " $inis $script $arguments";
         `$commandToExecute`;
-        if (\dd_trace_env_config("DD_TRACE_SIDECAR_TRACE_SENDER")) {
-            \dd_trace_synchronous_flush();
-        }
     }
 
     /**

diff --git a/tests/Common/TracerTestTrait.php b/tests/Common/TracerTestTrait.php
@@ -316,10 +316,6 @@ public function tracesFromWebRequest($fn, $tracer = null, callable $until = null
         self::putEnv('DD_TRACE_SHUTDOWN_TIMEOUT');
         self::putEnv('DD_TRACE_AGENT_RETRIES');
 
-        if (\dd_trace_env_config("DD_TRACE_SIDECAR_TRACE_SENDER")) {
-            \dd_trace_synchronous_flush();
-        }
-
         return $this->parseTracesFromDumpedData($until);
     }
 
@@ -401,18 +397,24 @@ private function parseTracesFromDumpedData(callable $until = null, $throw = fals
                     $dumps = array_merge($dumps, $this->parseRawDumpedTraces(json_decode($dump['body'], true)));
                 }
             }
+        } else {
+            $uniqueRequest = $loaded[0];
 
-            return $dumps;
-        }
-
-        $uniqueRequest = $loaded[0];
+            if (!isset($uniqueRequest['body'])) {
+                return [];
+            }
 
-        if (!isset($uniqueRequest['body'])) {
-            return [];
+            $rawTraces = json_decode($uniqueRequest['body'], true);
+            $dumps = $this->parseRawDumpedTraces($rawTraces);
         }
 
-        $rawTraces = json_decode($uniqueRequest['body'], true);
-        return $this->parseRawDumpedTraces($rawTraces);
+        // Ensure stable sorting; sort order isn't guaranteed with sidecar trace sender
+        // Sorting by end of root span
+        usort($dumps, function ($a, $b) {
+            return $a[0]["start"] + $a[0]["duration"] <=> $b[0]["start"] + $b[0]["duration"];
+        });
+
+        return $dumps;
     }
 
     public function parseMultipleRequestsFromDumpedData()
@@ -460,6 +462,10 @@ public function retrieveAnyDumpedData(callable $until = null, $throw, $metrics =
         // and actually sent. While we should find a smart way to tackle this, for now we do it quick and dirty, in a
         // for loop.
         for ($attemptNumber = 1; $attemptNumber <= 50; $attemptNumber++) {
+            if (\dd_trace_env_config("DD_TRACE_SIDECAR_TRACE_SENDER")) {
+                \dd_trace_synchronous_flush();
+            }
+
             $curl = curl_init(self::$agentRequestDumperUrl . '/replay' . ($metrics ? '-metrics' : ''));
             curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
             curl_setopt($curl, CURLOPT_HTTPHEADER, ['x-datadog-test-session-token: ' . ini_get("datadog.trace.agent_test_session_token")]);
@@ -493,7 +499,8 @@ public function retrieveAnyDumpedData(callable $until = null, $throw, $metrics =
         return $allResponses;
     }
 
-    public function retrieveDumpedTraceData(callable $until = null, $throw = false)
+    /** @param callable|null $until */
+    public function retrieveDumpedTraceData($until = null, $throw = false)
     {
         return array_values(array_filter($this->retrieveDumpedData($until, $throw), function ($request) {
             // Filter telemetry requests

diff --git a/tests/Integrations/AMQP/V2/AMQPTest.php b/tests/Integrations/AMQP/V2/AMQPTest.php
@@ -913,7 +913,7 @@ public function testDistributedTracing()
 
         foreach ($receiveTraces as $receiveTrace) {
             // Spans: connect -> queue_declare -> basic_consume & basic_consume_ok -> basic_deliver
-            if ($receiveTrace[0]["name"] == "amqp.basic.deliver") {
+            if ($receiveTrace[0]["name"] == "amqp.basic.deliver" && isset($receiveTrace[0]["parent_id"])) {
                 $basicDeliverSpan = $receiveTrace[0];
                 break;
             }

diff --git a/tests/Integrations/PCNTL/PCNTLTest.php b/tests/Integrations/PCNTL/PCNTLTest.php
@@ -96,7 +96,9 @@ public function testCliShortRunningTracingWhenEnabled()
             $this->untilNumberOfTraces(2)
         );
 
-        $this->assertCount(2, $requests);
+        // ignore curl noise from unrelated stuff
+        $requests = array_filter($requests, function ($request) { return $request[0]["name"] == "synthetic.php"; });
+
         $this->assertFlameGraph([$requests[1]], [
             SpanAssertion::exists('synthetic.php')->withChildren([
                 SpanAssertion::exists('pcntl_fork'),

diff --git a/tests/Integrations/Swoole/CommonScenariosTest.php b/tests/Integrations/Swoole/CommonScenariosTest.php
@@ -95,7 +95,7 @@ public function provideSpecs()
                         'http.method' => 'GET',
                         'http.url' => 'http://localhost/error?key=value&<redacted>',
                         'http.status_code' => '500',
-                        'error.stack' => "#0 [internal function]: {closure}()\n#1 {main}",
+                        'error.stack' => PHP_VERSION_ID >= 80400 ? "#0 [internal function]: {closure:" . dirname(__DIR__, 2) . "/Frameworks/Swoole/index.php:9}()" : "#0 [internal function]: {closure}()\n#1 {main}",
                         Tag::SPAN_KIND => 'server',
                         Tag::COMPONENT => 'swoole'
                     ])->setError('Exception', 'Uncaught Exception: Error page'),

diff --git a/tests/OpenTelemetry/composer-1.json b/tests/OpenTelemetry/composer-1.json
@@ -1,9 +1,9 @@
 {
     "name": "datadog/dd-trace-tests",
     "require": {
-        "open-telemetry/sdk": "1.0.*",
-        "open-telemetry/extension-propagator-b3": "1.0.*",
-        "open-telemetry/opentelemetry-logger-monolog": "1.0.*"
+        "open-telemetry/sdk": "1.0.*||1.1.*",
+        "open-telemetry/extension-propagator-b3": "1.0.*||1.1.*",
+        "open-telemetry/opentelemetry-logger-monolog": "1.0.*||1.1.*"
     },
     "minimum-stability": "stable"
 }
diff --git a/zend_abstract_interface/config/config.h b/zend_abstract_interface/config/config.h
@@ -101,4 +101,6 @@ bool zai_config_get_id_by_name(zai_str name, zai_config_id *id);
 // Adds name to name<->id mapping. Id may be present multiple times.
 void zai_config_register_config_id(zai_config_name *name, zai_config_id id);
 
+bool zai_config_is_initialized(void);
+
 #endif  // ZAI_CONFIG_H
+43 −40		remote-config/src/fetch/multitarget.rs
+2 −0		sidecar-ffi/src/lib.rs
+2 −0		sidecar-ffi/tests/sidecar.rs
+10 −9		sidecar/src/entry.rs
+4 −5		sidecar/src/log.rs
+2 −0		sidecar/src/service/blocking.rs
+1 −0		sidecar/src/service/sidecar_interface.rs
+14 −2		sidecar/src/service/sidecar_server.rs
+1 −1		sidecar/src/service/tracing/trace_flusher.rs
+8 −1		tinybytes/src/bytes_string.rs