Skip to content

Commit 2eb0609

Browse files
committed
fix: compilation error
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
1 parent ab050fe commit 2eb0609

File tree

5 files changed

+46
-89
lines changed

5 files changed

+46
-89
lines changed

ext/sidecar.c

Lines changed: 29 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#include <php.h>
2+
#include <main/SAPI.h>
13
#include "ddtrace.h"
24
#include "auto_flush.h"
35
#include "compat_string.h"
@@ -162,7 +164,6 @@ static void dd_sidecar_on_reconnect(ddog_SidecarTransport *transport) {
162164

163165
}
164166

165-
// Subprocess connection mode - current default behavior
166167
ddog_SidecarTransport *ddtrace_sidecar_connect_subprocess(void) {
167168
if (!ddtrace_endpoint) {
168169
return NULL;
@@ -193,13 +194,11 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_subprocess(void) {
193194

194195
dd_sidecar_post_connect(&sidecar_transport, false, logpath);
195196

196-
// Set active mode
197197
ddtrace_sidecar_active_mode = DD_SIDECAR_CONNECTION_SUBPROCESS;
198198

199199
return sidecar_transport;
200200
}
201201

202-
// Thread connection mode - fallback when subprocess fails
203202
ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void) {
204203
if (!ddtrace_endpoint) {
205204
return NULL;
@@ -211,12 +210,10 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void) {
211210
bool is_master = (ddtrace_sidecar_master_pid == 0 || current_pid == ddtrace_sidecar_master_pid);
212211

213212
if (is_master) {
214-
// Set master PID
215213
if (ddtrace_sidecar_master_pid == 0) {
216214
ddtrace_sidecar_master_pid = current_pid;
217215
}
218216

219-
// Start master listener thread (only if not already running)
220217
if (!ddog_sidecar_is_master_listener_active(ddtrace_sidecar_master_pid)) {
221218
if (!ddtrace_ffi_try("Failed starting master listener thread",
222219
ddog_sidecar_connect_master(ddtrace_sidecar_master_pid))) {
@@ -228,7 +225,6 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void) {
228225
}
229226
}
230227

231-
// Connect as worker to master listener
232228
ddog_SidecarTransport *sidecar_transport;
233229
if (!ddtrace_ffi_try("Failed connecting to master listener (thread mode)",
234230
ddog_sidecar_connect_worker(ddtrace_sidecar_master_pid, &sidecar_transport))) {
@@ -244,7 +240,6 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void) {
244240

245241
dd_sidecar_post_connect(&sidecar_transport, false, logpath);
246242

247-
// Set active mode
248243
ddtrace_sidecar_active_mode = DD_SIDECAR_CONNECTION_THREAD;
249244

250245
return sidecar_transport;
@@ -255,15 +250,25 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void) {
255250
#endif
256251
}
257252

258-
// Auto-fallback connection logic
259-
ddog_SidecarTransport *ddtrace_sidecar_connect_with_fallback(void) {
253+
ddog_SidecarTransport *ddtrace_sidecar_connect(bool is_fork) {
254+
if (is_fork && ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
255+
LOG(WARN, "Thread mode sidecar cannot be reset after fork, sidecar unavailable");
256+
return NULL;
257+
}
258+
259+
if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_SUBPROCESS) {
260+
return ddtrace_sidecar_connect_subprocess();
261+
} else if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
262+
return ddtrace_sidecar_connect_thread();
263+
}
264+
260265
zend_long mode = get_global_DD_TRACE_SIDECAR_CONNECTION_MODE();
261266
ddog_SidecarTransport *transport = NULL;
262267

263268
switch (mode) {
264269
case DD_TRACE_SIDECAR_CONNECTION_MODE_SUBPROCESS:
265270
// Force subprocess only
266-
LOG(INFO, "Sidecar connection mode: subprocess (forced)");
271+
LOG(DEBUG, "Sidecar connection mode: subprocess (forced)");
267272
transport = ddtrace_sidecar_connect_subprocess();
268273
if (!transport) {
269274
LOG(ERROR, "Subprocess connection failed (mode=subprocess, no fallback)");
@@ -272,7 +277,7 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_with_fallback(void) {
272277

273278
case DD_TRACE_SIDECAR_CONNECTION_MODE_THREAD:
274279
// Force thread only
275-
LOG(INFO, "Sidecar connection mode: thread (forced)");
280+
LOG(DEBUG, "Sidecar connection mode: thread (forced)");
276281
transport = ddtrace_sidecar_connect_thread();
277282
if (!transport) {
278283
LOG(ERROR, "Thread connection failed (mode=thread, no fallback)");
@@ -281,14 +286,17 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_with_fallback(void) {
281286

282287
case DD_TRACE_SIDECAR_CONNECTION_MODE_AUTO:
283288
default:
284-
// Try subprocess first
285-
LOG(INFO, "Sidecar connection mode: auto (trying subprocess first)");
289+
// Try subprocess first, fallback to thread if needed
290+
LOG(DEBUG, "Sidecar connection mode: auto (trying subprocess first)");
286291
transport = ddtrace_sidecar_connect_subprocess();
287292

288293
if (transport) {
289-
LOG(INFO, "Connected to sidecar via subprocess");
294+
LOG(DEBUG, "Connected to sidecar via subprocess");
295+
} else if (!ddtrace_endpoint) {
296+
// Don't try fallback if endpoint is invalid - both modes need a valid endpoint
297+
// The "Invalid DD_TRACE_AGENT_URL" error was already logged during endpoint creation
290298
} else {
291-
// Fallback to thread mode
299+
// Subprocess failed but endpoint is valid - try thread mode fallback
292300
LOG(WARN, "Subprocess connection failed, falling back to thread mode");
293301
transport = ddtrace_sidecar_connect_thread();
294302

@@ -304,51 +312,8 @@ ddog_SidecarTransport *ddtrace_sidecar_connect_with_fallback(void) {
304312
return transport;
305313
}
306314

307-
static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork) {
308-
// Should not happen, unless the agent url is malformed
309-
if (!ddtrace_endpoint) {
310-
return NULL;
311-
}
312-
ZEND_ASSERT(dogstatsd_endpoint != NULL);
313-
314-
dd_set_endpoint_test_token(dogstatsd_endpoint);
315-
316-
#ifdef _WIN32
317-
DDOG_PHP_FUNCTION = (const uint8_t *)zend_hash_func;
318-
#endif
319-
320-
char logpath[MAXPATHLEN];
321-
int error_fd = atomic_load(&ddtrace_error_log_fd);
322-
if (error_fd == -1 || ddtrace_get_fd_path(error_fd, logpath) < 0) {
323-
*logpath = 0;
324-
}
325-
326-
ddog_SidecarTransport *sidecar_transport;
327-
if (!ddtrace_ffi_try("Failed connecting to the sidecar", ddog_sidecar_connect_php(&sidecar_transport, logpath, dd_zend_string_to_CharSlice(get_global_DD_TRACE_LOG_LEVEL()), get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), dd_sidecar_on_reconnect, ddtrace_endpoint))) {
328-
dd_free_endpoints();
329-
return NULL;
330-
}
331-
332-
dd_sidecar_post_connect(&sidecar_transport, is_fork, logpath);
333-
334-
return sidecar_transport;
335-
}
336-
337-
ddog_SidecarTransport *dd_sidecar_connection_factory(void) {
338-
// Reconnect using the same mode that succeeded initially
339-
switch (ddtrace_sidecar_active_mode) {
340-
case DD_SIDECAR_CONNECTION_SUBPROCESS:
341-
return ddtrace_sidecar_connect_subprocess();
342-
343-
case DD_SIDECAR_CONNECTION_THREAD:
344-
return ddtrace_sidecar_connect_thread();
345-
346-
case DD_SIDECAR_CONNECTION_NONE:
347-
default:
348-
// Shouldn't happen, but fall back to auto mode
349-
LOG(WARN, "Reconnection attempted with no active mode, using fallback logic");
350-
return ddtrace_sidecar_connect_with_fallback();
351-
}
315+
static ddog_SidecarTransport *ddtrace_sidecar_connect_callback(void) {
316+
return ddtrace_sidecar_connect(false);
352317
}
353318

354319
bool ddtrace_sidecar_maybe_enable_appsec(bool *appsec_activation, bool *appsec_config) {
@@ -381,8 +346,7 @@ void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) {
381346

382347
ddog_init_remote_config(get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED(), appsec_activation, appsec_config);
383348

384-
// Use fallback connection logic
385-
ddtrace_sidecar = ddtrace_sidecar_connect_with_fallback();
349+
ddtrace_sidecar = ddtrace_sidecar_connect(false);
386350
if (!ddtrace_sidecar) { // Something went wrong
387351
if (ddtrace_endpoint) {
388352
dd_free_endpoints();
@@ -405,7 +369,7 @@ void ddtrace_sidecar_minit(void) {
405369

406370
void ddtrace_sidecar_ensure_active(void) {
407371
if (ddtrace_sidecar) {
408-
ddtrace_sidecar_reconnect(&ddtrace_sidecar, dd_sidecar_connection_factory);
372+
ddtrace_sidecar_reconnect(&ddtrace_sidecar, ddtrace_sidecar_connect_callback);
409373
}
410374
}
411375

@@ -482,19 +446,8 @@ void ddtrace_reset_sidecar(void) {
482446
ddog_sidecar_transport_drop(ddtrace_sidecar);
483447
ddtrace_sidecar = NULL;
484448

485-
// Don't reconnect in thread mode after fork (Option A: documented incompatibility)
486-
if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
487-
// Sidecar unavailable in child process after fork
488-
LOG(WARN, "Thread mode sidecar cannot be reset after fork, sidecar unavailable");
489-
if (ddtrace_endpoint) {
490-
dd_free_endpoints();
491-
}
492-
return;
493-
}
494-
495-
// For subprocess mode, reconnect with is_fork=true
496-
ddtrace_sidecar = dd_sidecar_connection_factory_ex(true);
497-
if (!ddtrace_sidecar) { // Something went wrong
449+
ddtrace_sidecar = ddtrace_sidecar_connect(true);
450+
if (!ddtrace_sidecar) {
498451
if (ddtrace_endpoint) {
499452
dd_free_endpoints();
500453
}

ext/sidecar.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ DDTRACE_PUBLIC struct telemetry_rc_info ddtrace_get_telemetry_rc_info(void);
3232
// Connection functions
3333
ddog_SidecarTransport *ddtrace_sidecar_connect_subprocess(void);
3434
ddog_SidecarTransport *ddtrace_sidecar_connect_thread(void);
35-
ddog_SidecarTransport *ddtrace_sidecar_connect_with_fallback(void);
35+
ddog_SidecarTransport *ddtrace_sidecar_connect(bool is_fork);
3636

3737
// Lifecycle functions
3838
void ddtrace_sidecar_minit(void);

tests/ext/span_on_close.phpt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ $span->onClose = [
2424

2525
?>
2626
--EXPECTF--
27+
[ddtrace] [debug] Sidecar connection mode: auto (trying subprocess first)
28+
[ddtrace] [debug] Connected to sidecar via subprocess
2729
Second
2830
First
2931
[ddtrace] [span] Encoding span: Span { service: %s, name: root span, resource: root span, type: cli, trace_id: %d, span_id: %d, parent_id: %d, start: %d, duration: %d, error: %d, meta: %s, metrics: %s, meta_struct: %s, span_links: %s, span_events: %s }

tests/ext/startup_logging_json_config.phpt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ dd_dump_startup_logs($logs, [
5252
]);
5353
?>
5454
--EXPECT--
55+
[ddtrace] [debug] Sidecar connection mode: auto (trying subprocess first)
56+
[ddtrace] [debug] Connected to sidecar via subprocess
5557
Sanity check
5658
env: "my-env"
5759
service: "my-service"

tests/ext/telemetry/config.phpt

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,53 +73,53 @@ Array
7373
[name] => trace.agent_url
7474
[value] => file://%s/config-telemetry.out
7575
[origin] => env_var
76-
[config_id] =>
77-
[seq_id] =>
76+
[config_id] =>
77+
[seq_id] =>
7878
)
7979

8080
[1] => Array
8181
(
8282
[name] => instrumentation_telemetry_enabled
8383
[value] => 1
8484
[origin] => env_var
85-
[config_id] =>
86-
[seq_id] =>
85+
[config_id] =>
86+
[seq_id] =>
8787
)
8888

8989
[2] => Array
9090
(
9191
[name] => trace.ignore_agent_sampling_rates
9292
[value] => 1
9393
[origin] => env_var
94-
[config_id] =>
95-
[seq_id] =>
94+
[config_id] =>
95+
[seq_id] =>
9696
)
9797

9898
[3] => Array
9999
(
100100
[name] => trace.generate_root_span
101101
[value] => 0
102102
[origin] => env_var
103-
[config_id] =>
104-
[seq_id] =>
103+
[config_id] =>
104+
[seq_id] =>
105105
)
106106

107107
[4] => Array
108108
(
109109
[name] => trace.git_metadata_enabled
110110
[value] => 0
111111
[origin] => env_var
112-
[config_id] =>
113-
[seq_id] =>
112+
[config_id] =>
113+
[seq_id] =>
114114
)
115115

116116
[5] => Array
117117
(
118118
[name] => ssi_forced_injection_enabled
119119
[value] => False
120120
[origin] => env_var
121-
[config_id] =>
122-
[seq_id] =>
121+
[config_id] =>
122+
[seq_id] =>
123123
)
124124

125125
)

0 commit comments

Comments
 (0)