Skip to content

Commit

Permalink
Revert "Enable sidecar by default on PHP 8.3 (#2680)"
Browse files Browse the repository at this point in the history
This reverts commit c5f2dcb.
  • Loading branch information
pierotibou committed Jun 10, 2024
1 parent 8d07c87 commit c43f712
Show file tree
Hide file tree
Showing 36 changed files with 179 additions and 534 deletions.
4 changes: 2 additions & 2 deletions .circleci/continue_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ aliases:
DD_REQUEST_DUMPER_FILE: dump.json

- &IMAGE_DOCKER_DD_TEST_AGENT
image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:latest
image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.11.0
name: test-agent
environment:
LOG_LEVEL: DEBUG
Expand Down Expand Up @@ -435,7 +435,7 @@ commands:
-e SNAPSHOT_DIR=/snapshots \
-p "127.0.0.1:9126:9126" \
-v $(pwd)/tests/snapshots:/snapshots \
--name test-agent ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:latest
--name test-agent ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.11.0
retry_docker run --detach --rm --net net \
--name httpbin_integration kong/httpbin
retry_docker run --detach --rm --net net \
Expand Down
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ test_extension_ci: $(SO_FILE) $(TEST_FILES) $(TEST_STUB_FILES)
\
export TEST_PHP_JUNIT=$(JUNIT_RESULTS_DIR)/valgrind-extension-test.xml; \
export TEST_PHP_OUTPUT=$(JUNIT_RESULTS_DIR)/valgrind-run-tests.out; \
export DD_TRACE_SIDECAR_TRACE_SENDER=0; \
$(RUN_TESTS_CMD) -d extension=$(SO_FILE) -m -s $$TEST_PHP_OUTPUT $(BUILD_DIR)/$(TESTS) && ! grep -e 'LEAKED TEST SUMMARY' $$TEST_PHP_OUTPUT; \
)

Expand Down Expand Up @@ -970,7 +969,7 @@ TEST_WEB_83 := \
test_web_custom \
test_web_zend_1_21

FILTER ?= .
FILTER := .
MAX_RETRIES := 3

# Note: The "composer show" command below outputs a csv with pairs of dependency;version such as "phpunit/phpunit;9.6.17"
Expand All @@ -985,7 +984,7 @@ define run_composer_with_retry
endef

define run_tests_without_coverage
$(TEST_EXTRA_ENV) $(ENV_OVERRIDE) php $(TEST_EXTRA_INI) -d datadog.instrumentation_telemetry_enabled=0 -d datadog.trace.sidecar_trace_sender=$(shell test $(PHP_MAJOR_MINOR) -ge 83 && echo 1 || echo 0) $(TRACER_SOURCES_INI) $(PHPUNIT) $(1) --filter=$(FILTER)
$(TEST_EXTRA_ENV) $(ENV_OVERRIDE) php $(TEST_EXTRA_INI) -d datadog.instrumentation_telemetry_enabled=0 $(TRACER_SOURCES_INI) $(PHPUNIT) $(1) --filter=$(FILTER)
endef

define run_tests_with_coverage
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ services:
- .:/app

test-agent:
image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:latest
image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.11.0
ports:
- "127.0.0.1:9126:8126"
volumes:
Expand Down
2 changes: 1 addition & 1 deletion ext/auto_flush.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ ZEND_RESULT_CODE ddtrace_flush_tracer(bool force_on_startup, bool collect_cycles
size_t size_hint = written;
zend_long n_requests = get_global_DD_TRACE_AGENT_FLUSH_AFTER_N_REQUESTS();
if (n_requests) {
size_hint = MAX(get_global_DD_TRACE_BUFFER_SIZE() / n_requests + 1, size_hint);
size_hint = MAX(get_global_DD_TRACE_BUFFER_SIZE() / n_requests, size_hint);
}
ddog_MaybeError send_error = ddog_sidecar_send_trace_v04_shm(&ddtrace_sidecar, ddtrace_sidecar_instance_id, shm, size_hint, &tags);
do {
Expand Down
93 changes: 5 additions & 88 deletions ext/coms.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,10 @@ static void _dd_at_exit_hook() {
}
}

bool ddtrace_coms_minit(size_t initial_stack_size, size_t max_stack_size, size_t max_backlog_size, char *bgs_fallback_telemetry_service) {
bool ddtrace_coms_minit(size_t initial_stack_size, size_t max_stack_size, size_t max_backlog_size) {
ddtrace_coms_globals.initial_stack_size = initial_stack_size;
ddtrace_coms_globals.max_payload_size = max_stack_size;
ddtrace_coms_globals.max_backlog_size = max_backlog_size;
ddtrace_coms_globals.bgs_fallback_telemetry = bgs_fallback_telemetry_service != NULL;
if (bgs_fallback_telemetry_service) {
strncpy(ddtrace_coms_globals.initial_service_name, bgs_fallback_telemetry_service, sizeof(ddtrace_coms_globals.initial_service_name) - 1);
}

atomic_store(&ddtrace_coms_globals.stack_size, initial_stack_size);

Expand Down Expand Up @@ -739,31 +735,23 @@ void ddtrace_curl_set_connect_timeout(CURL *curl) {
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS, timeout);
}

static void ddtrace_curl_set_hostname_generic(CURL *curl, const char *path) {
void ddtrace_curl_set_hostname(CURL *curl) {
char *url = ddtrace_agent_url();
if (url && url[0]) {
char *http_url = url;
if (strlen(url) > 7 && strncmp(url, "unix://", 7) == 0) {
curl_easy_setopt(curl, CURLOPT_UNIX_SOCKET_PATH, url + 7);
http_url = "http://localhost";
}
size_t agent_url_len = strlen(http_url) + strlen(path) + 1;
size_t agent_url_len = strlen(http_url) + sizeof(TRACE_PATH_STR);
char *agent_url = malloc(agent_url_len);
sprintf(agent_url, "%s%s", http_url, path);
sprintf(agent_url, "%s%s", http_url, TRACE_PATH_STR);
curl_easy_setopt(curl, CURLOPT_URL, agent_url);
free(agent_url);
}
free(url);
}

void ddtrace_curl_set_hostname(CURL *curl) {
ddtrace_curl_set_hostname_generic(curl, TRACE_PATH_STR);
}

void ddtrace_curl_set_telemetry_url(CURL *curl) {
ddtrace_curl_set_hostname_generic(curl, "/telemetry/proxy/api/v2/apmtelemetry");
}

static struct timespec _dd_deadline_in_ms(uint32_t ms) {
struct timespec deadline;
struct timeval now;
Expand Down Expand Up @@ -1004,77 +992,6 @@ static void *_dd_writer_loop(void *_) {

bool running = true;
_dd_signal_writer_started(writer);

if (ddtrace_coms_globals.bgs_fallback_telemetry) {
ddtrace_coms_globals.bgs_fallback_telemetry = false;
uint8_t runtime_id[36];
ddtrace_format_runtime_id(&runtime_id);
char hostname[101];
hostname[100] = 0;
gethostname(hostname, 100);
char *payload;
asprintf(&payload, "{\n"
" \"api_version\": \"v2\",\n"
" \"request_type\": \"generate-metrics\",\n"
" \"seq_id\": 1,\n"
" \"runtime_id\": \"%.36s\",\n"
" \"tracer_time\": %ld,\n"
" \"payload\": {\n"
" \"namespace\": \"tracers\",\n"
" \"series\": [\n"
" {\n"
" \"metric\": \"exporter_fallback\",\n"
" \"tags\": [\n"
" \"reason:instrumentation_telemetry_disabled\"\n"
" ],\n"
" \"points\": [\n"
" [\n"
" %ld,\n"
" 1\n"
" ]\n"
" ],\n"
" \"type\": \"count\",\n"
" \"common\": true\n"
" }\n"
" ]\n"
" },\n"
" \"application\": {\n"
" \"service_name\": \"%s\",\n"
" \"tracer_version\": \"%s\",\n"
" \"language_name\": \"php\",\n"
" \"language_version\": \"%s\"\n"
" },\n"
" \"host\": {\n"
" \"hostname\": \"%s\"\n"
" }\n"
"}",
(char *)runtime_id,
time(NULL),
time(NULL),
ddtrace_coms_globals.initial_service_name,
PHP_DDTRACE_VERSION,
ZSTR_VAL(ddtrace_php_version),
hostname
);

writer->curl = curl_easy_init();
curl_easy_setopt(writer->curl, CURLOPT_WRITEFUNCTION, _dd_dummy_write_callback);
curl_easy_setopt(writer->curl, CURLOPT_NOSIGNAL, 1);
curl_easy_setopt(writer->curl, CURLOPT_POSTFIELDS, payload);
ddtrace_curl_set_timeout(writer->curl);
ddtrace_curl_set_connect_timeout(writer->curl);
struct curl_slist *headers = curl_slist_append(NULL, "Content-Type: application/json");
curl_easy_setopt(writer->curl, CURLOPT_HTTPHEADER, headers);
ddtrace_curl_set_telemetry_url(writer->curl);
curl_easy_perform(writer->curl);

free(payload);
CURL *curl = writer->curl;
writer->curl = NULL;
curl_easy_cleanup(curl);
curl_slist_free_all(headers);
}

do {
atomic_fetch_add(&writer->writer_cycle, 1);
uint32_t interval = atomic_load(&writer->flush_interval);
Expand Down Expand Up @@ -1239,7 +1156,7 @@ void ddtrace_coms_clean_background_sender_after_fork(void) {
_dd_unsafe_cleanup_dirty_stack_area();
_dd_coms_stack_shutdown();
global_writer = (struct _writer_loop_data_t){0};
ddtrace_coms_minit(ddtrace_coms_globals.initial_stack_size, ddtrace_coms_globals.max_payload_size, ddtrace_coms_globals.max_backlog_size, NULL);
ddtrace_coms_minit(ddtrace_coms_globals.initial_stack_size, ddtrace_coms_globals.max_payload_size, ddtrace_coms_globals.max_backlog_size);
}

bool ddtrace_coms_on_pid_change(void) {
Expand Down
6 changes: 1 addition & 5 deletions ext/coms.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ typedef struct ddtrace_coms_state_t {
* The maximum backlog size, from DD_TRACE_AGENT_MAX_BACKLOG_SIZE
*/
size_t max_backlog_size;

/* Whether to send fallback telemetry. */
bool bgs_fallback_telemetry;
char initial_service_name[100];
} ddtrace_coms_state_t;

inline bool ddtrace_coms_is_stack_unused(ddtrace_coms_stack_t *stack) { return atomic_load(&stack->refcount) == 0; }
Expand All @@ -56,7 +52,7 @@ inline bool ddtrace_coms_is_stack_free(ddtrace_coms_stack_t *stack) {
/* Is called by the PHP thread to buffer a payload in order to send it. It is non-blocking on the request to the agent.
*/
bool ddtrace_coms_buffer_data(uint32_t group_id, const char *data, size_t size);
bool ddtrace_coms_minit(size_t initial_stack_size, size_t max_payload_size, size_t max_backlog_size, char *bgs_fallback_telemetry_service);
bool ddtrace_coms_minit(size_t initial_stack_size, size_t max_payload_size, size_t max_backlog_size);
void ddtrace_coms_mshutdown(void);
void ddtrace_coms_curl_shutdown(void);
void ddtrace_coms_rshutdown(void);
Expand Down
14 changes: 4 additions & 10 deletions ext/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ enum ddtrace_sampling_rules_format {
#define DD_INTEGRATION_ANALYTICS_ENABLED_DEFAULT false
#define DD_INTEGRATION_ANALYTICS_SAMPLE_RATE_DEFAULT 1

#if PHP_VERSION_ID >= 80300 || defined(_WIN32)
#define DD_SIDECAR_TRACE_SENDER_DEFAULT true
#else
#define DD_SIDECAR_TRACE_SENDER_DEFAULT false
#endif

#if _BUILD_FROM_PECL_
#define DD_DEFAULT_SOURCES_PATH "@php_dir@/datadog_trace/src/"
#else
Expand Down Expand Up @@ -206,11 +200,11 @@ enum ddtrace_sampling_rules_format {
DD_INTEGRATIONS

#ifndef _WIN32
# define DD_CONFIGURATION \
CONFIG(BOOL, DD_TRACE_SIDECAR_TRACE_SENDER, DD_CFG_EXPSTR(DD_SIDECAR_TRACE_SENDER_DEFAULT), .ini_change = zai_config_system_ini_change) \
DD_CONFIGURATION_ALL
#define DD_CONFIGURATION \
CONFIG(BOOL, DD_TRACE_SIDECAR_TRACE_SENDER, "false", .ini_change = zai_config_system_ini_change) \
DD_CONFIGURATION_ALL
#else
# define DD_CONFIGURATION DD_CONFIGURATION_ALL
#define DD_CONFIGURATION DD_CONFIGURATION_ALL
#endif

#define CALIAS CONFIG
Expand Down
49 changes: 16 additions & 33 deletions ext/ddtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,20 +398,6 @@ 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
// Only disable sidecar sender when explicitly disabled
bool bgs_fallback = DD_SIDECAR_TRACE_SENDER_DEFAULT && get_global_DD_TRACE_SIDECAR_TRACE_SENDER() && zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_SIDECAR_TRACE_SENDER].name_index < 0 && !get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED();
zend_string *bgs_service = NULL;
if (bgs_fallback) {
// 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
ddtrace_change_default_ini(DDTRACE_CONFIG_DD_TRACE_SIDECAR_TRACE_SENDER, (zai_str) ZAI_STR_FROM_CSTR("0"));
if ((bgs_service = get_DD_SERVICE())) {
zend_string_addref(bgs_service);
} else {
bgs_service = ddtrace_default_service_name();
}
}
if (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER())
#endif
{
Expand All @@ -420,25 +406,6 @@ static void dd_activate_once(void) {
ddtrace_sidecar_setup();
PG(modules_activated) = modules_activated;
}
#ifndef _WIN32
if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
if (get_global_DD_TRACE_AGENT_FLUSH_AFTER_N_REQUESTS() == 0) {
// Set the default to 10 so that BGS flushes faster. With sidecar it's not needed generally.
ddtrace_change_default_ini(DDTRACE_CONFIG_DD_TRACE_AGENT_FLUSH_AFTER_N_REQUESTS, (zai_str) ZAI_STR_FROM_CSTR("10"));
}
if (get_DD_TRACE_AGENT_FLUSH_INTERVAL() == DD_TRACE_AGENT_FLUSH_INTERVAL_VAL) {
// Set the default to 5000 so that BGS does not flush too often. The sidecar can flush more often, but the BGS is per process. Keep it higher to avoid too much load on the agent.
ddtrace_change_default_ini(DDTRACE_CONFIG_DD_TRACE_AGENT_FLUSH_INTERVAL, (zai_str) ZAI_STR_FROM_CSTR("5000"));
}
ddtrace_coms_minit(get_global_DD_TRACE_AGENT_STACK_INITIAL_SIZE(),
get_global_DD_TRACE_AGENT_MAX_PAYLOAD_SIZE(),
get_global_DD_TRACE_AGENT_STACK_BACKLOG(),
bgs_fallback ? ZSTR_VAL(bgs_service) : NULL);
if (bgs_fallback) {
zend_string_release(bgs_service);
}
}
#endif
}
}

Expand Down Expand Up @@ -1150,6 +1117,14 @@ static PHP_MINIT_FUNCTION(ddtrace) {

ddtrace_engine_hooks_minit();

#ifndef _WIN32
if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
ddtrace_coms_minit(get_global_DD_TRACE_AGENT_STACK_INITIAL_SIZE(),
get_global_DD_TRACE_AGENT_MAX_PAYLOAD_SIZE(),
get_global_DD_TRACE_AGENT_STACK_BACKLOG());
}
#endif

ddtrace_integrations_minit();
dd_ip_extraction_startup();
ddtrace_serializer_startup();
Expand Down Expand Up @@ -1227,6 +1202,14 @@ static void dd_rinit_once(void) {
#ifndef _WIN32
ddtrace_signals_first_rinit();
if (!get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
if (get_global_DD_TRACE_AGENT_FLUSH_AFTER_N_REQUESTS() == 0) {
// Set the default to 10 so that BGS flushes faster. With sidecar it's not needed generally.
ddtrace_change_default_ini(DDTRACE_CONFIG_DD_TRACE_AGENT_FLUSH_AFTER_N_REQUESTS, (zai_str) ZAI_STR_FROM_CSTR("10"));
}
if (get_DD_TRACE_AGENT_FLUSH_INTERVAL() == DD_TRACE_AGENT_FLUSH_INTERVAL_VAL) {
// Set the default to 5000 so that BGS does not flush too often. The sidecar can flush more often, but the BGS is per process. Keep it higher to avoid too much load on the agent.
ddtrace_change_default_ini(DDTRACE_CONFIG_DD_TRACE_AGENT_FLUSH_INTERVAL, (zai_str) ZAI_STR_FROM_CSTR("5000"));
}
ddtrace_coms_init_and_start_writer();
}
#endif
Expand Down
23 changes: 8 additions & 15 deletions ext/serializer.c
Original file line number Diff line number Diff line change
Expand Up @@ -811,19 +811,6 @@ void ddtrace_inherit_span_properties(ddtrace_span_data *span, ddtrace_span_data
ZVAL_COPY(prop_env, env);
}

zend_string *ddtrace_default_service_name(void) {
if (strcmp(sapi_module.name, "cli") != 0) {
return zend_string_init(ZEND_STRL("web.request"), 0);
}

const char *script_name;
if (SG(request_info).argc > 0 && (script_name = SG(request_info).argv[0]) && script_name[0] != '\0') {
return php_basename(script_name, strlen(script_name), NULL, 0);
} else {
return zend_string_init(ZEND_STRL("cli.command"), 0);
}
}

void ddtrace_set_root_span_properties(ddtrace_root_span_data *span) {
ddtrace_update_root_id_properties(span);

Expand Down Expand Up @@ -894,12 +881,18 @@ void ddtrace_set_root_span_properties(ddtrace_root_span_data *span) {
if (strcmp(sapi_module.name, "cli") == 0) {
zval_ptr_dtor(prop_type);
ZVAL_STR(prop_type, zend_string_init(ZEND_STRL("cli"), 0));
const char *script_name;
zval_ptr_dtor(prop_name);
ZVAL_STR(prop_name,
(SG(request_info).argc > 0 && (script_name = SG(request_info).argv[0]) && script_name[0] != '\0')
? php_basename(script_name, strlen(script_name), NULL, 0)
: zend_string_init(ZEND_STRL("cli.command"), 0));
} else {
zval_ptr_dtor(prop_type);
ZVAL_STR(prop_type, zend_string_init(ZEND_STRL("web"), 0));
zval_ptr_dtor(prop_name);
ZVAL_STR(prop_name, zend_string_init(ZEND_STRL("web.request"), 0));
}
zval_ptr_dtor(prop_name);
ZVAL_STR(prop_name, ddtrace_default_service_name());
zval_ptr_dtor(prop_service);
ZVAL_STR_COPY(prop_service, ZSTR_LEN(get_DD_SERVICE()) ? get_DD_SERVICE() : Z_STR_P(prop_name));

Expand Down
1 change: 0 additions & 1 deletion ext/serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ void ddtrace_set_global_span_properties(ddtrace_span_data *span);
void ddtrace_set_root_span_properties(ddtrace_root_span_data *span);
void ddtrace_update_root_id_properties(ddtrace_root_span_data *span);
void ddtrace_inherit_span_properties(ddtrace_span_data *span, ddtrace_span_data *parent);
zend_string *ddtrace_default_service_name(void);

void ddtrace_initialize_span_sampling_limiter(void);
void ddtrace_shutdown_span_sampling_limiter(void);
Expand Down
2 changes: 0 additions & 2 deletions ext/startup_logging.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ 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 @@ -182,7 +181,6 @@ 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
Loading

0 comments on commit c43f712

Please sign in to comment.