Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ext/datadog.c
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,6 @@ static PHP_RSHUTDOWN_FUNCTION(datadog) {
bool fast_shutdown = is_zend_mm() && !EG(full_tables_cleanup);
#endif

if (DATADOG_G(remote_config_state)) {
datadog_rshutdown_remote_config();
}

if (!datadog_disable) {
dd_shutdown_observer();
}
Expand All @@ -666,6 +662,11 @@ static PHP_RSHUTDOWN_FUNCTION(datadog) {

datadog_sidecar_finalize(true);
DATADOG_G(request_initialized) = false;
/* A signal may have queued a Remote Config reread during RSHUTDOWN. */
DATADOG_G(reread_remote_configuration) = 0;
if (DATADOG_G(remote_config_state)) {
datadog_rshutdown_remote_config();
}

datadog_telemetry_rshutdown();
datadog_sidecar_rshutdown();
Expand Down
12 changes: 8 additions & 4 deletions ext/remote_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ static void dd_vm_interrupt(zend_execute_data *execute_data) {
if (dd_prev_interrupt_function) {
dd_prev_interrupt_function(execute_data);
}
if (DATADOG_G(remote_config_state) && DATADOG_G(reread_remote_configuration)) {
LOG(INFO, "Rereading remote configurations after interrupt");
if (DATADOG_G(reread_remote_configuration)) {
DATADOG_G(reread_remote_configuration) = 0;
ddog_process_remote_configs(DATADOG_G(remote_config_state));
if (DATADOG_G(request_initialized) && DATADOG_G(remote_config_state)) {
Comment thread
morrisonlevi marked this conversation as resolved.
LOG(INFO, "Rereading remote configurations after interrupt");
ddog_process_remote_configs(DATADOG_G(remote_config_state));
}
}
}

Expand Down Expand Up @@ -53,7 +55,9 @@ DATADOG_PUBLIC void datadog_set_all_thread_vm_interrupt(void) {
}

void datadog_check_for_new_config_now(void) {
if (DATADOG_G(remote_config_state) && !DATADOG_G(reread_remote_configuration) && ddog_process_remote_configs(DATADOG_G(remote_config_state))) {
if (DATADOG_G(request_initialized) && DATADOG_G(remote_config_state) &&
!DATADOG_G(reread_remote_configuration) &&
ddog_process_remote_configs(DATADOG_G(remote_config_state))) {
// If we blocked the signal, notify the other threads too
datadog_set_all_thread_vm_interrupt();
}
Expand Down
4 changes: 2 additions & 2 deletions ext/sidecar.c
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ void ddtrace_sidecar_submit_span_data_direct(ddog_SidecarTransport **transport,
const ddog_Vec_Tag *process_tags = datadog_process_tags_get_vec();

bool changed = true;
if (DATADOG_G(remote_config_state)) {
if (DATADOG_G(request_initialized) && DATADOG_G(remote_config_state)) {
changed = ddog_remote_configs_service_env_change(DATADOG_G(remote_config_state), service_slice, env_slice, version_slice, &DATADOG_G(active_global_tags), process_tags);
}

Expand Down Expand Up @@ -853,7 +853,7 @@ void ddtrace_sidecar_submit_span_data_direct(ddog_SidecarTransport **transport,
ddog_sidecar_telemetry_filter_flush(transport, datadog_sidecar_instance_id, &DATADOG_G(sidecar_queue_id), datadog_telemetry_buffer(), datadog_telemetry_cache(), service_slice, env_slice));
}

if (DATADOG_G(remote_config_state)) {
if (DATADOG_G(request_initialized) && DATADOG_G(remote_config_state)) {
// Must happen after ddog_sidecar_set_universal_service_tags (session state fully initialized)
ddog_process_remote_configs(DATADOG_G(remote_config_state));
}
Expand Down
60 changes: 60 additions & 0 deletions tests/ext/live-debugger/debugger_rshutdown_remote_config.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
--TEST--
Remote Config and live debugger hooks remain valid through tracer request shutdown
--SKIPIF--
<?php include __DIR__ . '/../includes/skipif_no_dev_env.inc'; ?>
--ENV--
DD_AGENT_HOST=request-replayer
DD_TRACE_AGENT_PORT=80
DD_TRACE_GENERATE_ROOT_SPAN=0
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS=0.1
DD_TRACE_AGENT_TEST_SESSION_TOKEN=live-debugger/rshutdown_remote_config
--INI--
datadog.trace.enabled=1
datadog.autofinish_spans=1
--FILE--
<?php

require __DIR__ . "/live_debugger.inc";

reset_request_replayer();
ini_set('datadog.logs_injection', '0');

function instrumented(): void {}

final class StartSpanDuringShutdown
{
public function __destruct()
{
$span = \DDTrace\start_span();
$span->onClose[] = function () {
// The span is closed from tracer RSHUTDOWN. Remote Config and its
// live debugger subscriber must still be active.
var_dump(count(dd_trace_internal_fn('get_loaded_remote_configs')));
echo "onClose completed\n";
};
}
}

put_dynamic_config_file([
"log_injection_enabled" => true,
"dynamic_instrumentation_enabled" => true,
]);

await_probe_installation(function () {
build_span_probe(["where" => ["methodName" => "instrumented"]]);
});

$trigger = new StartSpanDuringShutdown();

echo "request body complete\n";

?>
--CLEAN--
<?php
require __DIR__ . "/live_debugger.inc";
reset_request_replayer();
?>
--EXPECT--
request body complete
int(2)
onClose completed
64 changes: 64 additions & 0 deletions tests/ext/remote_config/dynamic_config_late_shutdown.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
--TEST--
Remote config is not reapplied after its request shutdown cleanup
--SKIPIF--
<?php
include __DIR__ . '/../includes/skipif_no_dev_env.inc';
?>
--ENV--
DD_AGENT_HOST=request-replayer
DD_TRACE_AGENT_PORT=80
DD_TRACE_GENERATE_ROOT_SPAN=0
DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS=0.1
DD_TRACE_AGENT_TEST_SESSION_TOKEN=remote-config/dynamic_config_late_shutdown
--FILE--
<?php

require __DIR__ . "/remote_config.inc";
include __DIR__ . '/../includes/request_replayer.inc';

final class LateRemoteConfigWrapper
{
public $context;

public function stream_open($path, $mode, $options, &$opened_path)
{
return true;
}

public function stream_close()
{
// Resource destruction runs after module RSHUTDOWN. Process Remote
// Config synchronously so this lifecycle boundary is deterministic.
dd_trace_internal_fn('process_remote_config');
echo "late close completed\n";
}
}

reset_request_replayer();
ini_set('datadog.logs_injection', '0');

$path = put_dynamic_config_file([
'log_injection_enabled' => true,
]);

\DDTrace\start_span();
if (ini_get('datadog.logs_injection') !== '1') {
dd_trace_internal_fn('await_remote_config');
}
var_dump(ini_get('datadog.logs_injection'));

stream_wrapper_register('late-remote-config', LateRemoteConfigWrapper::class);
$trigger = fopen('late-remote-config://trigger', 'r');

echo "request body complete\n";

?>
--CLEAN--
<?php
require __DIR__ . "/remote_config.inc";
reset_request_replayer();
?>
--EXPECT--
string(1) "1"
request body complete
late close completed
2 changes: 1 addition & 1 deletion tracer/ddtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,6 @@ void ddtrace_rshutdown(bool fast_shutdown) {

ddtrace_clean_git_object();
ddtrace_weak_resources_rshutdown();
ddtrace_live_debugger_rshutdown();
}

void ddtrace_post_deactivate(void) {
Expand All @@ -637,6 +636,7 @@ void ddtrace_post_deactivate(void) {
zai_interceptor_deactivate();

// we can only actually free our hooks hashtables in post_deactivate, as within RSHUTDOWN some user code may still run
ddtrace_live_debugger_rshutdown();
zai_hook_rshutdown();
zai_uhook_rshutdown();
}
Expand Down
4 changes: 4 additions & 0 deletions tracer/functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -2215,6 +2215,10 @@ PHP_FUNCTION(dd_trace_internal_fn) {
} else {
array_init(return_value);
}
} else if (FUNCTION_NAME_MATCHES("process_remote_config")) {
// Test/debug helper for exercising Remote Config at precise lifecycle points.
datadog_check_for_new_config_now();
RETVAL_TRUE;
} else if (FUNCTION_NAME_MATCHES("await_remote_config")) {
uint32_t timeout_sec = 10;
if (params_count == 1) {
Expand Down
Loading