Skip to content

Commit c59bfda

Browse files
committed
apply feedbacks
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
1 parent 88442ce commit c59bfda

5 files changed

Lines changed: 71 additions & 267 deletions

File tree

components-rs/sidecar.h

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -92,70 +92,14 @@ void ddog_sidecar_transport_drop(struct ddog_SidecarTransport*);
9292
*/
9393
ddog_MaybeError ddog_sidecar_connect(struct ddog_SidecarTransport **connection);
9494

95-
/**
96-
* Start master listener thread for thread-based connections (Unix only).
97-
*
98-
* This spawns a listener thread that accepts worker connections. Only one
99-
* master listener can be active per process.
100-
*
101-
* # Arguments
102-
* * `pid` - Process ID that owns this listener
103-
*
104-
* # Returns
105-
* * `MaybeError::None` on success
106-
* * `MaybeError::Some(Error)` on failure
107-
*/
10895
ddog_MaybeError ddog_sidecar_connect_master(int32_t pid);
10996

110-
/**
111-
* Connect as worker to master listener thread (Unix only).
112-
*
113-
* Establishes a connection to the master listener for the given PID.
114-
*
115-
* # Arguments
116-
* * `pid` - Process ID of the master process
117-
* * `connection` - Output parameter for the connection
118-
*
119-
* # Returns
120-
* * `MaybeError::None` on success
121-
* * `MaybeError::Some(Error)` on failure
122-
*/
12397
ddog_MaybeError ddog_sidecar_connect_worker(int32_t pid, struct ddog_SidecarTransport **connection);
12498

125-
/**
126-
* Shutdown the master listener thread (Unix only).
127-
*
128-
* Sends shutdown signal and joins the listener thread. This is blocking.
129-
*
130-
* # Returns
131-
* * `MaybeError::None` on success
132-
* * `MaybeError::Some(Error)` on failure
133-
*/
13499
ddog_MaybeError ddog_sidecar_shutdown_master_listener(void);
135100

136-
/**
137-
* Check if master listener is active for the given PID (Unix only).
138-
*
139-
* Used for fork detection.
140-
*
141-
* # Arguments
142-
* * `pid` - Process ID to check
143-
*
144-
* # Returns
145-
* * `true` if listener is active for this PID
146-
* * `false` otherwise
147-
*/
148101
bool ddog_sidecar_is_master_listener_active(int32_t pid);
149102

150-
/**
151-
* Clear inherited master listener state in child after fork (Unix only).
152-
*
153-
* Child processes must call this to avoid using the parent's listener.
154-
*
155-
* # Returns
156-
* * `MaybeError::None` on success
157-
* * `MaybeError::Some(Error)` on failure
158-
*/
159103
ddog_MaybeError ddog_sidecar_clear_inherited_listener(void);
160104

161105
ddog_MaybeError ddog_sidecar_ping(struct ddog_SidecarTransport **transport);

ext/ddtrace.c

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -454,10 +454,7 @@ static void dd_activate_once(void) {
454454
}
455455

456456
// if we're to enable appsec, we need to enable sidecar
457-
bool enable_sidecar = ddtrace_sidecar_maybe_enable_appsec(&appsec_activation, &appsec_config);
458-
if (!enable_sidecar) {
459-
enable_sidecar = get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER();
460-
}
457+
bool enable_sidecar = ddtrace_sidecar_should_enable(&appsec_activation, &appsec_config);
461458

462459
if (enable_sidecar)
463460
#endif
@@ -1616,11 +1613,7 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {
16161613

16171614
ddtrace_user_req_shutdown();
16181615

1619-
// Only shutdown sidecar in MSHUTDOWN for non-CLI SAPIs
1620-
// CLI SAPI shuts down in RSHUTDOWN to allow thread joins before ASAN checks
1621-
if (strcmp(sapi_module.name, "cli") != 0) {
1622-
ddtrace_sidecar_shutdown();
1623-
}
1616+
ddtrace_sidecar_shutdown();
16241617

16251618
ddtrace_live_debugger_mshutdown();
16261619
ddtrace_process_tags_mshutdown();
@@ -2666,30 +2659,9 @@ void dd_internal_handle_fork(void) {
26662659
ddtrace_coms_curl_shutdown();
26672660
ddtrace_coms_clean_background_sender_after_fork();
26682661
}
2669-
2670-
// Handle thread mode after fork
2671-
int32_t current_pid = (int32_t)getpid();
2672-
bool is_child_process = (ddtrace_sidecar_master_pid != 0 &&
2673-
current_pid != ddtrace_sidecar_master_pid);
2674-
2675-
if (is_child_process && ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
2676-
// Clear inherited master listener state (child doesn't own it)
2677-
ddtrace_ffi_try("Failed clearing inherited listener state",
2678-
ddog_sidecar_clear_inherited_listener());
2679-
2680-
// Attempt to reconnect child to parent's master listener
2681-
bool appsec_activation = false;
2682-
bool appsec_config = false;
2683-
bool enable_sidecar = ddtrace_sidecar_maybe_enable_appsec(&appsec_activation, &appsec_config);
2684-
if (!enable_sidecar) {
2685-
enable_sidecar = get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER();
2686-
}
2687-
2688-
if (enable_sidecar && !ddtrace_sidecar_reconnect_after_fork(appsec_activation, appsec_config)) {
2689-
LOG(WARN, "Child process after fork with thread mode: failed to reconnect to parent's listener");
2690-
}
2691-
}
26922662
#endif
2663+
2664+
ddtrace_sidecar_handle_fork();
26932665
if (DDTRACE_G(agent_config_reader)) {
26942666
ddog_agent_remote_config_reader_drop(DDTRACE_G(agent_config_reader));
26952667
DDTRACE_G(agent_config_reader) = NULL;
@@ -2704,11 +2676,6 @@ void dd_internal_handle_fork(void) {
27042676
}
27052677
ddtrace_seed_prng();
27062678
ddtrace_generate_runtime_id();
2707-
// Thread mode already handled sidecar reconnection above (lines 2648-2664)
2708-
// Only reset for subprocess mode
2709-
if (ddtrace_sidecar_active_mode != DD_SIDECAR_CONNECTION_THREAD) {
2710-
ddtrace_reset_sidecar();
2711-
}
27122679
if (!get_DD_TRACE_FORKED_PROCESS()) {
27132680
ddtrace_disable_tracing_in_current_request();
27142681
}

0 commit comments

Comments
 (0)