Skip to content

Commit

Permalink
Enable sidecar by default, only when telemetry is enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
pierotibou committed May 29, 2024
1 parent 78871c1 commit f7c26ee
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 4 deletions.
14 changes: 10 additions & 4 deletions ext/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,17 @@ enum ddtrace_sampling_rules_format {
DD_INTEGRATIONS

#ifndef _WIN32
#define DD_CONFIGURATION \
CONFIG(BOOL, DD_TRACE_SIDECAR_TRACE_SENDER, "false", .ini_change = zai_config_system_ini_change) \
DD_CONFIGURATION_ALL
# if PHP_VERSION_ID >= 80300
# define DD_CONFIGURATION \
CONFIG(BOOL, DD_TRACE_SIDECAR_TRACE_SENDER, "true", .ini_change = zai_config_system_ini_change) \
DD_CONFIGURATION_ALL
# else
# define DD_CONFIGURATION \
CONFIG(BOOL, DD_TRACE_SIDECAR_TRACE_SENDER, "false", .ini_change = zai_config_system_ini_change) \
DD_CONFIGURATION_ALL
# endif
#else
#define DD_CONFIGURATION DD_CONFIGURATION_ALL
# define DD_CONFIGURATION DD_CONFIGURATION_ALL
#endif

#define CALIAS CONFIG
Expand Down
17 changes: 17 additions & 0 deletions ext/ddtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,15 @@ static void dd_activate_once(void) {

// must run before the first zai_hook_activate as ddtrace_telemetry_setup installs a global hook
if (!ddtrace_disable) {

#ifndef _WIN32
if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && !get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED()){
// We enabled sending traces through the sidecar by default
// That said a few customers have explicitly disabled telemetry to disable the sidecar
// So if telemetry is disabled, we will disable the sidecar and send a one shot telemetry call
disable_sidecar_sending();
// TODO: report the fallback in telemetry
}
if (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER())
#endif
{
Expand All @@ -410,6 +418,15 @@ static void dd_activate_once(void) {
}
}

#ifndef _WIN32
void disable_sidecar_sending(void){
zend_string *zero = zend_string_init("0", 1, 0);
zend_alter_ini_entry(zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_SIDECAR_TRACE_SENDER].ini_entries[0]->name, zero,
ZEND_INI_SYSTEM, ZEND_INI_STAGE_RUNTIME);
zend_string_release(zero);
}
#endif

static pthread_once_t dd_activate_once_control = PTHREAD_ONCE_INIT;

static void ddtrace_activate(void) {
Expand Down
3 changes: 3 additions & 0 deletions ext/ddtrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ bool ddtrace_alter_default_propagation_style(zval *old_value, zval *new_value);
bool ddtrace_alter_dd_env(zval *old_value, zval *new_value);
bool ddtrace_alter_dd_version(zval *old_value, zval *new_value);
void dd_force_shutdown_tracing(void);
#ifndef _WIN32
void disable_sidecar_sending(void);
#endif

typedef struct {
int type;
Expand Down
2 changes: 2 additions & 0 deletions ext/startup_logging.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ static void _dd_get_startup_config(HashTable *ht) {
_dd_add_assoc_zstring(ht, ZEND_STRL("dd_version"), zend_string_copy(get_DD_VERSION()));
// "health_metrics_enabled" N/A for PHP
_dd_add_assoc_zstring(ht, ZEND_STRL("architecture"), php_get_uname('m'));
_dd_add_assoc_bool(ht, ZEND_STRL("instrumentation_telemetry_enabled"), get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED());

// PHP-specific values
_dd_add_assoc_string(ht, ZEND_STRL("sapi"), sapi_module.name);
Expand All @@ -181,6 +182,7 @@ static void _dd_get_startup_config(HashTable *ht) {
_dd_implode_keys(get_DD_TRACE_TRACED_INTERNAL_FUNCTIONS()));
_dd_add_assoc_bool(ht, ZEND_STRL("enabled_from_env"), get_DD_TRACE_ENABLED());
_dd_add_assoc_string(ht, ZEND_STRL("opcache.file_cache"), _dd_get_ini(ZEND_STRL("opcache.file_cache")));
_dd_add_assoc_bool(ht, ZEND_STRL("sidecar_trace_sender"), get_global_DD_TRACE_SIDECAR_TRACE_SENDER());
}

#ifndef _WIN32
Expand Down
22 changes: 22 additions & 0 deletions tests/ext/sidecar_disabled_when_telemetry_disabled.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Sidecar should be disabled when telemetry is disabled
--SKIPIF--
<?php include 'startup_logging_skipif_unix.inc'; ?>
--FILE--
<?php
include_once 'startup_logging.inc';

$logs = dd_get_startup_logs([], [
'DD_TRACE_DEBUG' => '1',
'DD_INSTRUMENTATION_TELEMETRY_ENABLED' => '0',
]);

dd_dump_startup_logs($logs, [
'instrumentation_telemetry_enabled',
'sidecar_trace_sender',
]);

?>
--EXPECT--
instrumentation_telemetry_enabled: false
sidecar_trace_sender: false
3 changes: 3 additions & 0 deletions tests/ext/startup_logging.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ $logs = dd_get_startup_logs(['-ddatadog.trace.sources_path='], ['DD_TRACE_DEBUG'

// Ignore any Agent connection errors for now
unset($logs['agent_error']);
// Ignore sidecar config as it depends on specific versions of PHP for now
unset($logs['sidecar_trace_sender']);

dd_dump_startup_logs($logs);
?>
Expand All @@ -34,6 +36,7 @@ service_mapping: []
distributed_tracing_enabled: true
dd_version: null
architecture: "%s"
instrumentation_telemetry_enabled: true
sapi: "cgi-fcgi"
datadog.trace.sources_path: null
open_basedir_configured: false
Expand Down
3 changes: 3 additions & 0 deletions tests/ext/startup_logging_json.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ $logs = json_decode(\DDTrace\startup_logs(), true);

// Ignore any Agent connection errors for now
unset($logs['agent_error']);
// Ignore sidecar config as it depends on specific versions of PHP for now
unset($logs['sidecar_trace_sender']);

dd_dump_startup_logs($logs);
?>
Expand All @@ -33,6 +35,7 @@ service_mapping: []
distributed_tracing_enabled: true
dd_version: null
architecture: "%s"
instrumentation_telemetry_enabled: true
sapi: "cli"
datadog.trace.sources_path: null
open_basedir_configured: false
Expand Down
6 changes: 6 additions & 0 deletions tests/ext/startup_logging_skipif_unix.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

include_once 'startup_logging.inc';
if (!dd_get_php_cgi()) die('skip: php-cgi SAPI required');
if (strncasecmp(PHP_OS, "WIN", 3) == 0) die('skip: There is no background sender on Windows');
// if ( version_compare(phpversion(),'8.3.0', '<')) die('skip: this test works only on PHP 8.3');

0 comments on commit f7c26ee

Please sign in to comment.