From f41d953a0f71dd8225cbb9f7925cc0f41be40204 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 11:00:12 -0400 Subject: [PATCH 01/16] add http status error configuration --- ext/configuration.c | 45 ++++++ ext/configuration.h | 6 +- ext/serializer.c | 129 ++++++++++++++++++ .../http_client_error_status_configured.phpt | 20 +++ .../http_server_error_status_configured.phpt | 19 +++ ...server_status_with_explicit_error_set.phpt | 44 ++++++ .../http_server_vs_client_status.diff | 12 ++ .../http_server_vs_client_status.exp | 7 + .../http_server_vs_client_status.out | 6 + .../http_server_vs_client_status.php | 37 +++++ .../http_server_vs_client_status.phpt | 51 +++++++ .../http_server_vs_client_status.sh | 73 ++++++++++ ...http_status_mixed_range_configuration.phpt | 41 ++++++ ...tatus_no_configuration_with_exception.phpt | 47 +++++++ 14 files changed, 536 insertions(+), 1 deletion(-) create mode 100644 tests/ext/serialization/http_client_error_status_configured.phpt create mode 100644 tests/ext/serialization/http_server_error_status_configured.phpt create mode 100644 tests/ext/serialization/http_server_status_with_explicit_error_set.phpt create mode 100644 tests/ext/serialization/http_server_vs_client_status.diff create mode 100644 tests/ext/serialization/http_server_vs_client_status.exp create mode 100644 tests/ext/serialization/http_server_vs_client_status.out create mode 100644 tests/ext/serialization/http_server_vs_client_status.php create mode 100644 tests/ext/serialization/http_server_vs_client_status.phpt create mode 100755 tests/ext/serialization/http_server_vs_client_status.sh create mode 100644 tests/ext/serialization/http_status_mixed_range_configuration.phpt create mode 100644 tests/ext/serialization/http_status_no_configuration_with_exception.phpt diff --git a/ext/configuration.c b/ext/configuration.c index 11a0c10202f..6e90aa0cacb 100644 --- a/ext/configuration.c +++ b/ext/configuration.c @@ -7,6 +7,7 @@ #include "json/json.h" #include "sidecar.h" #include +#include #include #include "sidecar.h" @@ -156,6 +157,50 @@ static bool dd_parse_tags(zai_str value, zval *decoded_value, bool persistent) { return true; } +static bool dd_parse_http_error_statuses(zai_str value, zval *decoded_value, bool persistent) { + array_init(decoded_value); + + if (value.len == 0) { + return true; + } + + // Add null termination + char *buf = estrndup(value.ptr, value.len); + if (!buf) { + return false; + } + + char *tok, *saveptr = NULL; + + for (tok = strtok_r(buf, ",", &saveptr); tok; tok = strtok_r(NULL, ",", &saveptr)) { + // Trim whitespace + char *start = tok; + char *end = tok + strlen(tok) - 1; + + while (start <= end && isspace((unsigned char)*start)) start++; + while (end > start && isspace((unsigned char)*end)) *end-- = '\0'; + + // Remove quotes if present + if (strlen(start) >= 2 && start[0] == '"' && start[strlen(start)-1] == '"') { + start++; + start[strlen(start)-1] = '\0'; + } + + if (strlen(start) > 0) { + // For SET, the value is the key and the value is boolean true + zval true_val; + ZVAL_TRUE(&true_val); + + // Add to array using string key with boolean true value + zend_hash_str_update(Z_ARR_P(decoded_value), start, strlen(start), &true_val); + } + } + + efree(buf); + + return true; +} + #define INI_CHANGE_DYNAMIC_CONFIG(name, config) \ static bool ddtrace_alter_##name(zval *old_value, zval *new_value, zend_string *new_str) { \ UNUSED(old_value, new_value); \ diff --git a/ext/configuration.h b/ext/configuration.h index 09d6ef0050b..e956c57673d 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -247,7 +247,11 @@ enum ddtrace_sampling_rules_format { CONFIG(SET, DD_DYNAMIC_INSTRUMENTATION_REDACTED_TYPES, "", .ini_change = zai_config_system_ini_change) \ CONFIG(INT, DD_TRACE_BAGGAGE_MAX_ITEMS, "64") \ CONFIG(INT, DD_TRACE_BAGGAGE_MAX_BYTES, "8192") \ - CONFIG(BOOL, DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED, "false") \ + CONFIG(BOOL, DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED, "false") \ + CONFIG(SET, DD_TRACE_HTTP_CLIENT_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change, \ + .parser = dd_parse_http_error_statuses) \ + CONFIG(SET, DD_TRACE_HTTP_SERVER_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change, \ + .parser = dd_parse_http_error_statuses) \ DD_INTEGRATIONS #ifndef _WIN32 diff --git a/ext/serializer.c b/ext/serializer.c index 72cbb9ee14b..0cdc58f8e5f 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1476,6 +1476,98 @@ void transfer_data(zend_array *source, zend_array *destination, const char *key, } } +static bool _dd_should_mark_as_error(ddtrace_span_data *span) { + // Explicitly set errors are the priority + zend_array *meta = ddtrace_property_array(&span->property_meta); + zend_string *key; + zval *zv; + if (meta) { + zval *error_zv = zend_hash_str_find(meta, "error", sizeof("error") - 1); + if (error_zv && Z_TYPE_P(error_zv) == IS_LONG) { + if (Z_LVAL_P(error_zv) == 1) { + return true; + } + if (Z_LVAL_P(error_zv) == 0) { + return false; + } + } + } + + // HTTP status configured + zval *status_zv = meta + ? zend_hash_str_find(meta, "http.status_code", sizeof("http.status_code") - 1) + : NULL; + if (status_zv) { + int status_code = 0; + if (Z_TYPE_P(status_zv) == IS_STRING) { + status_code = atoi(Z_STRVAL_P(status_zv)); + } else if (Z_TYPE_P(status_zv) == IS_LONG) { + status_code = Z_LVAL_P(status_zv); + } + + if (status_code > 0) { + // Determine if client or server span. Check kind first and fallback to type + bool is_client_span = false; + zval *kind_zv = zend_hash_str_find(meta, "span.kind", sizeof("span.kind") - 1); + if (kind_zv && Z_TYPE_P(kind_zv) == IS_STRING && + strcmp(Z_STRVAL_P(kind_zv), "client") == 0) { + is_client_span = true; + } else { + zval *type_zv = zend_hash_str_find(meta, "span.type", sizeof("span.type") - 1); + if (type_zv && Z_TYPE_P(type_zv) == IS_STRING && + (strcmp(Z_STRVAL_P(type_zv), "http") == 0 || + strcmp(Z_STRVAL_P(type_zv), "client") == 0)) { + is_client_span = true; + } + } + + // Get the proper configuration + zend_array *cfg = is_client_span + ? get_DD_TRACE_HTTP_CLIENT_ERROR_STATUSES() + : get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); + size_t cfg_sz = cfg ? zend_hash_num_elements(cfg) : 0; + + if (cfg_sz > 0) { + zend_string *str_key; + zval *entry_zv; + + // For SET, the keys are the status codes/ranges + ZEND_HASH_FOREACH_STR_KEY_VAL(cfg, str_key, entry_zv) { + if (str_key) { + const char *s = ZSTR_VAL(str_key); + char *dash = strchr(s, '-'); + + if (dash) { + // Range like "500-599" + int start, end; + if (sscanf(s, "%d-%d", &start, &end) == 2) { + if (status_code >= start && status_code <= end) { + return true; + } + } + } else { + // Single status code + int code = atoi(s); + if (code == status_code) { + return true; + } + } + } + } ZEND_HASH_FOREACH_END(); + + return false; + } + } + } + + // Exception with no HTTP status configuration + zval *ex_zv = &span->property_exception; + if (Z_TYPE_P(ex_zv) == IS_OBJECT) { + return true; + } + + return false; +} zval *ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { bool is_root_span = span->std.ce == ddtrace_ce_root_span_data; @@ -1834,6 +1926,43 @@ zval *ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { } } + // Figure out if an error should be tracked + bool is_error = _dd_should_mark_as_error(span); + + if (is_error) { + zval zv_error; + ZVAL_LONG(&zv_error, 1); + add_assoc_zval(el, "error", &zv_error); + + // If there's an exception, the existing serializer code will handle it + // We only need to add error metadata for HTTP status code errors (no exception) + zval *exception = &span->property_exception; + if (Z_TYPE_P(exception) != IS_OBJECT) { + // HTTP status code error case + // Add error meta if not already present + zval *serialized_meta = zend_hash_str_find(Z_ARR_P(el), ZEND_STRL("meta")); + if (serialized_meta && Z_TYPE_P(serialized_meta) == IS_ARRAY) { + zend_array *meta_arr = Z_ARR_P(serialized_meta); + if (!zend_hash_str_exists(meta_arr, ZEND_STRL("error.type"))) { + add_assoc_str(serialized_meta, "error.type", zend_string_init("http_error", sizeof("http_error") - 1, 0)); + } + + if (!zend_hash_str_exists(meta_arr, ZEND_STRL("error.msg"))) { + zval *status_code_zv = zend_hash_str_find(meta, ZEND_STRL("http.status_code")); + if (status_code_zv) { + char error_msg[50]; + if (Z_TYPE_P(status_code_zv) == IS_STRING) { + snprintf(error_msg, sizeof(error_msg), "HTTP %s Error", Z_STRVAL_P(status_code_zv)); + } else if (Z_TYPE_P(status_code_zv) == IS_LONG) { + snprintf(error_msg, sizeof(error_msg), "HTTP %ld Error", Z_LVAL_P(status_code_zv)); + } + add_assoc_string(serialized_meta, "error.msg", error_msg); + } + } + } + } + } + if (inferred_span) { zval *serialized_inferred_span = ddtrace_serialize_span_to_array(inferred_span, array); zend_array *serialized_inferred_span_meta = Z_ARR_P(zend_hash_str_find(Z_ARR_P(serialized_inferred_span), ZEND_STRL("meta"))); diff --git a/tests/ext/serialization/http_client_error_status_configured.phpt b/tests/ext/serialization/http_client_error_status_configured.phpt new file mode 100644 index 00000000000..078016f1e9e --- /dev/null +++ b/tests/ext/serialization/http_client_error_status_configured.phpt @@ -0,0 +1,20 @@ +--TEST-- +Client span with configured error status code (basic functionality) +--ENV-- +DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=400-499 +--FILE-- +meta['http.status_code'] = 404; +$span->meta['span.kind'] = 'client'; +\DDTrace\close_span(); + +$serialized = dd_trace_serialize_closed_spans(); +var_dump($serialized[0]['error'] ?? null); +var_dump($serialized[0]['meta']['error.type'] ?? null); +var_dump($serialized[0]['meta']['error.msg'] ?? null); +?> +--EXPECT-- +int(1) +string(10) "http_error" +string(14) "HTTP 404 Error" \ No newline at end of file diff --git a/tests/ext/serialization/http_server_error_status_configured.phpt b/tests/ext/serialization/http_server_error_status_configured.phpt new file mode 100644 index 00000000000..5d038e4bfee --- /dev/null +++ b/tests/ext/serialization/http_server_error_status_configured.phpt @@ -0,0 +1,19 @@ +--TEST-- +Server span with configured error status code (basic functionality) +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 +--FILE-- +meta['http.status_code'] = 500; +\DDTrace\close_span(); + +$serialized = dd_trace_serialize_closed_spans(); +var_dump($serialized[0]['error'] ?? null); +var_dump($serialized[0]['meta']['error.type'] ?? null); +var_dump($serialized[0]['meta']['error.msg'] ?? null); +?> +--EXPECT-- +int(1) +string(10) "http_error" +string(14) "HTTP 500 Error" \ No newline at end of file diff --git a/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt b/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt new file mode 100644 index 00000000000..59563ec552e --- /dev/null +++ b/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt @@ -0,0 +1,44 @@ +--TEST-- +Explicit error flag takes precedence over status code configuration +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 +--FILE-- +meta['http.status_code'] = 500; +$span1->meta['error'] = 0; +$span1->meta['debug_id'] = 'EXPLICIT_NO_ERROR'; +\DDTrace\close_span(); + +// Should be error due to explicit flag = 1, even though status code is 200 +$span2 = \DDTrace\start_span(); +$span2->meta['http.status_code'] = 200; +$span2->meta['error'] = 1; +$span2->meta['debug_id'] = 'EXPLICIT_ERROR'; +\DDTrace\close_span(); + +$serialized = dd_trace_serialize_closed_spans(); + +// Find spans by debug ID +$no_error_span = null; +$error_span = null; +foreach ($serialized as $span) { + if (isset($span['meta']['debug_id'])) { + if ($span['meta']['debug_id'] === 'EXPLICIT_NO_ERROR') { + $no_error_span = $span; + } elseif ($span['meta']['debug_id'] === 'EXPLICIT_ERROR') { + $error_span = $span; + } + } +} + +echo "Explicit error=0 with 500 status: "; +var_dump(isset($no_error_span['error']) ? $no_error_span['error'] : null); + +echo "Explicit error=1 with 200 status: "; +var_dump(isset($error_span['error']) ? $error_span['error'] : null); +?> +--EXPECT-- +Explicit error=0 with 500 status: NULL +Explicit error=1 with 200 status: int(1) \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.diff b/tests/ext/serialization/http_server_vs_client_status.diff new file mode 100644 index 00000000000..37e0c842e7d --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.diff @@ -0,0 +1,12 @@ + === SERVER SPAN WITH 404 === +002- === CLIENT SPAN WITH 404 === +003- === RESULTS === +004- string(17) "Client 404 error:" +005- int(1) +006- string(17) "Server 404 error:" +007- NULL +002+ +003+ Fatal error: Uncaught Error: Call to undefined function DDTrace\start_span() in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php:4 +004+ Stack trace: +005+ #0 {main} +006+ thrown in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php on line 4 diff --git a/tests/ext/serialization/http_server_vs_client_status.exp b/tests/ext/serialization/http_server_vs_client_status.exp new file mode 100644 index 00000000000..e565e9e64d4 --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.exp @@ -0,0 +1,7 @@ +=== SERVER SPAN WITH 404 === +=== CLIENT SPAN WITH 404 === +=== RESULTS === +string(17) "Client 404 error:" +int(1) +string(17) "Server 404 error:" +NULL \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.out b/tests/ext/serialization/http_server_vs_client_status.out new file mode 100644 index 00000000000..6025ec9cf30 --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.out @@ -0,0 +1,6 @@ +=== SERVER SPAN WITH 404 === + +Fatal error: Uncaught Error: Call to undefined function DDTrace\start_span() in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php:4 +Stack trace: +#0 {main} + thrown in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php on line 4 \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.php b/tests/ext/serialization/http_server_vs_client_status.php new file mode 100644 index 00000000000..3391b6623ca --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.php @@ -0,0 +1,37 @@ +meta['http.status_code'] = 404; +$span_server->meta['debug_id'] = 'SERVER_SPAN'; +\DDTrace\close_span(); + +// 2. Client span with 404 - should be error +echo "=== CLIENT SPAN WITH 404 ===\n"; +$span_client = \DDTrace\start_span(); +$span_client->meta['http.status_code'] = 404; +$span_client->meta['span.kind'] = 'client'; +$span_client->meta['debug_id'] = 'CLIENT_SPAN'; +\DDTrace\close_span(); + +// Get serialized spans +$serialized = dd_trace_serialize_closed_spans(); + +// Check results +echo "=== RESULTS ===\n"; +// Find spans by debug ID +$client_span = null; +$server_span = null; +foreach ($serialized as $span) { + if (isset($span['meta']['debug_id'])) { + if ($span['meta']['debug_id'] === 'CLIENT_SPAN') { + $client_span = $span; + } elseif ($span['meta']['debug_id'] === 'SERVER_SPAN') { + $server_span = $span; + } + } +} + +var_dump("Client 404 error:", isset($client_span['error']) ? $client_span['error'] : null); +var_dump("Server 404 error:", isset($server_span['error']) ? $server_span['error'] : null); +?> diff --git a/tests/ext/serialization/http_server_vs_client_status.phpt b/tests/ext/serialization/http_server_vs_client_status.phpt new file mode 100644 index 00000000000..b0f2a53c5a7 --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.phpt @@ -0,0 +1,51 @@ +--TEST-- +Client and server spans use their respective configurations +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 +DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=400-499 +--FILE-- +meta['http.status_code'] = 404; +$span_server->meta['debug_id'] = 'SERVER_SPAN'; +\DDTrace\close_span(); + +// 2. Client span with 404 - should be error +echo "=== CLIENT SPAN WITH 404 ===\n"; +$span_client = \DDTrace\start_span(); +$span_client->meta['http.status_code'] = 404; +$span_client->meta['span.kind'] = 'client'; +$span_client->meta['debug_id'] = 'CLIENT_SPAN'; +\DDTrace\close_span(); + +// Get serialized spans +$serialized = dd_trace_serialize_closed_spans(); + +// Check results +echo "=== RESULTS ===\n"; +// Find spans by debug ID +$client_span = null; +$server_span = null; +foreach ($serialized as $span) { + if (isset($span['meta']['debug_id'])) { + if ($span['meta']['debug_id'] === 'CLIENT_SPAN') { + $client_span = $span; + } elseif ($span['meta']['debug_id'] === 'SERVER_SPAN') { + $server_span = $span; + } + } +} + +var_dump("Client 404 error:", isset($client_span['error']) ? $client_span['error'] : null); +var_dump("Server 404 error:", isset($server_span['error']) ? $server_span['error'] : null); +?> +--EXPECTF-- +=== SERVER SPAN WITH 404 === +=== CLIENT SPAN WITH 404 === +=== RESULTS === +string(17) "Client 404 error:" +int(1) +string(17) "Server 404 error:" +NULL \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.sh b/tests/ext/serialization/http_server_vs_client_status.sh new file mode 100755 index 00000000000..02c7371de28 --- /dev/null +++ b/tests/ext/serialization/http_server_vs_client_status.sh @@ -0,0 +1,73 @@ +#!/bin/sh + +export RUNTIME_DEPS='apache2 apache2-dev ca-certificates clang-format curl debian-goodies git less lsof netbase netcat nginx strace sudo unzip vim xz-utils zip' +export HOSTNAME='9ad8a3847fc6' +export PHP_VERSION='8.3' +export APACHE_CONFDIR='/etc/apache2' +export DATADOG_HAVE_DEV_ENV='1' +export HTTPBIN_HOSTNAME='httpbin_integration' +export PWD='/home/circleci/app' +export PHP_SRC_DIR='/usr/local/src/php' +export RUSTC_BOOTSTRAP='1' +export DD_TRACE_DOCKER_DEBUG='1' +export HOME='/home/circleci' +export LANG='C.UTF-8' +export PHP_IDE_CONFIG='serverName=docker' +export LS_COLORS='rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:' +export CARGO_HOME='/rust/cargo' +export CMAKE_VERSION='3.24.4' +export PHP_INSTALL_DIR='/opt/php' +export GOROOT='/go' +export PHPIZE_DEPS='autoconf bison clang-17 cmake dpkg-dev file gnupg gpg file libc-dev make pkg-config re2c' +export REDIS_HOSTNAME='redis_integration' +export LSAN_OPTIONS='suppressions=/home/circleci/suppr.txt,print_suppressions=0,use_tls=0' +export TERM='xterm' +export ACCEPT_EULA='Y' +export RUSTUP_HOME='/rust/rustup' +export SHLVL='1' +export COMPOSER_MEMORY_LIMIT='-1' +export APACHE_ENVVARS='/etc/apache2/envvars' +export DD_SPAWN_WORKER_USE_EXEC='1' +export PATH='/go/bin:/rust/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin' +export DEVLIBS='libclang-17-dev libclang-rt-17-dev llvm-17-dev lld-17 libbrotli-dev libcurl4-openssl-dev libedit-dev libffi-dev libfreetype6-dev libicu-dev libjpeg-dev libmcrypt-dev libmemcached-dev libodbc1 libonig-dev libpq-dev libpng-dev librabbitmq-dev librdkafka-dev libsodium-dev libsqlite3-dev libssl-dev libwebp-dev libxml2-dev libxslt1-dev libzip-dev libc6-dbg odbcinst1debian2 zlib1g-dev unixodbc-dev unixodbc' +export DDAGENT_HOSTNAME='ddagent_integration' +export DEBIAN_FRONTEND='noninteractive' +export OLDPWD='/home/circleci/app/target/debug/build' +export _='/usr/local/bin/php' +export TEMP='/tmp' +export TEST_PHP_EXECUTABLE='/opt/php/debug/bin/php' +export TEST_PHP_EXECUTABLE_ESCAPED=''\''/opt/php/debug/bin/php'\''' +export TEST_PHP_CGI_EXECUTABLE='/opt/php/debug/bin/php-cgi' +export TEST_PHP_CGI_EXECUTABLE_ESCAPED=''\''/opt/php/debug/bin/php-cgi'\''' +export TEST_PHPDBG_EXECUTABLE='' +export TEST_PHPDBG_EXECUTABLE_ESCAPED=''\'''\''' +export REDIRECT_STATUS='1' +export QUERY_STRING='' +export PATH_TRANSLATED='/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php' +export SCRIPT_FILENAME='/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php' +export REQUEST_METHOD='GET' +export CONTENT_TYPE='' +export CONTENT_LENGTH='' +export TZ='' +export DD_TRACE_HTTP_SERVER_ERROR_STATUSES='500-599' +export DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='400-499' +export TEST_PHP_EXTRA_ARGS=' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0"' +export HTTP_COOKIE='' + +case "$1" in +"gdb") + gdb --args '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 + ;; +"lldb") + lldb -- '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 + ;; +"valgrind") + USE_ZEND_ALLOC=0 valgrind $2 '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 + ;; +"rr") + rr record $2 '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 + ;; +*) + '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 + ;; +esac \ No newline at end of file diff --git a/tests/ext/serialization/http_status_mixed_range_configuration.phpt b/tests/ext/serialization/http_status_mixed_range_configuration.phpt new file mode 100644 index 00000000000..58ce5bfd763 --- /dev/null +++ b/tests/ext/serialization/http_status_mixed_range_configuration.phpt @@ -0,0 +1,41 @@ +--TEST-- +Mixed range configuration with specific status codes +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=400-403,419,500-503 +--FILE-- +meta['http.status_code'] = $code; + $span->meta['debug_id'] = "STATUS_$code"; + \DDTrace\close_span(); +} + +$serialized = dd_trace_serialize_closed_spans(); + +// Check results for each status code +echo "=== RESULTS ===\n"; +foreach ($status_codes as $code) { + $span = null; + foreach ($serialized as $s) { + if (isset($s['meta']['debug_id']) && $s['meta']['debug_id'] === "STATUS_$code") { + $span = $s; + break; + } + } + + echo "Status $code is error: " . (isset($span['error']) ? 'YES' : 'NO') . "\n"; +} +?> +--EXPECT-- +=== RESULTS === +Status 400 is error: YES +Status 403 is error: YES +Status 404 is error: NO +Status 419 is error: YES +Status 500 is error: YES +Status 503 is error: YES +Status 504 is error: NO \ No newline at end of file diff --git a/tests/ext/serialization/http_status_no_configuration_with_exception.phpt b/tests/ext/serialization/http_status_no_configuration_with_exception.phpt new file mode 100644 index 00000000000..788d6435067 --- /dev/null +++ b/tests/ext/serialization/http_status_no_configuration_with_exception.phpt @@ -0,0 +1,47 @@ +--TEST-- +Exception fallback when no configuration is set +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,500-599 +--FILE-- +meta['http.status_code'] = 200; +try { + throw new Exception("Test exception"); +} catch (Exception $e) { + $span1->exception = $e; +} +$span1->meta['debug_id'] = 'EXCEPTION_SPAN'; +\DDTrace\close_span(); + +// Should NOT be error - 404 status code but no configuration +$span2 = \DDTrace\start_span(); +$span2->meta['http.status_code'] = 404; +$span2->meta['debug_id'] = 'NO_EXCEPTION_SPAN'; +\DDTrace\close_span(); + +$serialized = dd_trace_serialize_closed_spans(); + +// Find spans by debug ID +$exception_span = null; +$no_exception_span = null; +foreach ($serialized as $span) { + if (isset($span['meta']['debug_id'])) { + if ($span['meta']['debug_id'] === 'EXCEPTION_SPAN') { + $exception_span = $span; + } elseif ($span['meta']['debug_id'] === 'NO_EXCEPTION_SPAN') { + $no_exception_span = $span; + } + } +} + +echo "200 status with exception: "; +var_dump(isset($exception_span['error']) ? $exception_span['error'] : null); + +echo "404 status without exception or config: "; +var_dump(isset($no_exception_span['error']) ? $no_exception_span['error'] : null); +?> +--EXPECT-- +200 status with exception: int(1) +404 status without exception or config: NULL \ No newline at end of file From 58e23d38b8700b4082e3b25cdb39049a6ec51ec0 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 11:04:40 -0400 Subject: [PATCH 02/16] remove testing artifacts --- .../http_server_vs_client_status.diff | 12 --- .../http_server_vs_client_status.exp | 7 -- .../http_server_vs_client_status.out | 6 -- .../http_server_vs_client_status.php | 37 ---------- .../http_server_vs_client_status.sh | 73 ------------------- 5 files changed, 135 deletions(-) delete mode 100644 tests/ext/serialization/http_server_vs_client_status.diff delete mode 100644 tests/ext/serialization/http_server_vs_client_status.exp delete mode 100644 tests/ext/serialization/http_server_vs_client_status.out delete mode 100644 tests/ext/serialization/http_server_vs_client_status.php delete mode 100755 tests/ext/serialization/http_server_vs_client_status.sh diff --git a/tests/ext/serialization/http_server_vs_client_status.diff b/tests/ext/serialization/http_server_vs_client_status.diff deleted file mode 100644 index 37e0c842e7d..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.diff +++ /dev/null @@ -1,12 +0,0 @@ - === SERVER SPAN WITH 404 === -002- === CLIENT SPAN WITH 404 === -003- === RESULTS === -004- string(17) "Client 404 error:" -005- int(1) -006- string(17) "Server 404 error:" -007- NULL -002+ -003+ Fatal error: Uncaught Error: Call to undefined function DDTrace\start_span() in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php:4 -004+ Stack trace: -005+ #0 {main} -006+ thrown in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php on line 4 diff --git a/tests/ext/serialization/http_server_vs_client_status.exp b/tests/ext/serialization/http_server_vs_client_status.exp deleted file mode 100644 index e565e9e64d4..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.exp +++ /dev/null @@ -1,7 +0,0 @@ -=== SERVER SPAN WITH 404 === -=== CLIENT SPAN WITH 404 === -=== RESULTS === -string(17) "Client 404 error:" -int(1) -string(17) "Server 404 error:" -NULL \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.out b/tests/ext/serialization/http_server_vs_client_status.out deleted file mode 100644 index 6025ec9cf30..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.out +++ /dev/null @@ -1,6 +0,0 @@ -=== SERVER SPAN WITH 404 === - -Fatal error: Uncaught Error: Call to undefined function DDTrace\start_span() in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php:4 -Stack trace: -#0 {main} - thrown in /home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php on line 4 \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.php b/tests/ext/serialization/http_server_vs_client_status.php deleted file mode 100644 index 3391b6623ca..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.php +++ /dev/null @@ -1,37 +0,0 @@ -meta['http.status_code'] = 404; -$span_server->meta['debug_id'] = 'SERVER_SPAN'; -\DDTrace\close_span(); - -// 2. Client span with 404 - should be error -echo "=== CLIENT SPAN WITH 404 ===\n"; -$span_client = \DDTrace\start_span(); -$span_client->meta['http.status_code'] = 404; -$span_client->meta['span.kind'] = 'client'; -$span_client->meta['debug_id'] = 'CLIENT_SPAN'; -\DDTrace\close_span(); - -// Get serialized spans -$serialized = dd_trace_serialize_closed_spans(); - -// Check results -echo "=== RESULTS ===\n"; -// Find spans by debug ID -$client_span = null; -$server_span = null; -foreach ($serialized as $span) { - if (isset($span['meta']['debug_id'])) { - if ($span['meta']['debug_id'] === 'CLIENT_SPAN') { - $client_span = $span; - } elseif ($span['meta']['debug_id'] === 'SERVER_SPAN') { - $server_span = $span; - } - } -} - -var_dump("Client 404 error:", isset($client_span['error']) ? $client_span['error'] : null); -var_dump("Server 404 error:", isset($server_span['error']) ? $server_span['error'] : null); -?> diff --git a/tests/ext/serialization/http_server_vs_client_status.sh b/tests/ext/serialization/http_server_vs_client_status.sh deleted file mode 100755 index 02c7371de28..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.sh +++ /dev/null @@ -1,73 +0,0 @@ -#!/bin/sh - -export RUNTIME_DEPS='apache2 apache2-dev ca-certificates clang-format curl debian-goodies git less lsof netbase netcat nginx strace sudo unzip vim xz-utils zip' -export HOSTNAME='9ad8a3847fc6' -export PHP_VERSION='8.3' -export APACHE_CONFDIR='/etc/apache2' -export DATADOG_HAVE_DEV_ENV='1' -export HTTPBIN_HOSTNAME='httpbin_integration' -export PWD='/home/circleci/app' -export PHP_SRC_DIR='/usr/local/src/php' -export RUSTC_BOOTSTRAP='1' -export DD_TRACE_DOCKER_DEBUG='1' -export HOME='/home/circleci' -export LANG='C.UTF-8' -export PHP_IDE_CONFIG='serverName=docker' -export LS_COLORS='rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:' -export CARGO_HOME='/rust/cargo' -export CMAKE_VERSION='3.24.4' -export PHP_INSTALL_DIR='/opt/php' -export GOROOT='/go' -export PHPIZE_DEPS='autoconf bison clang-17 cmake dpkg-dev file gnupg gpg file libc-dev make pkg-config re2c' -export REDIS_HOSTNAME='redis_integration' -export LSAN_OPTIONS='suppressions=/home/circleci/suppr.txt,print_suppressions=0,use_tls=0' -export TERM='xterm' -export ACCEPT_EULA='Y' -export RUSTUP_HOME='/rust/rustup' -export SHLVL='1' -export COMPOSER_MEMORY_LIMIT='-1' -export APACHE_ENVVARS='/etc/apache2/envvars' -export DD_SPAWN_WORKER_USE_EXEC='1' -export PATH='/go/bin:/rust/cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin' -export DEVLIBS='libclang-17-dev libclang-rt-17-dev llvm-17-dev lld-17 libbrotli-dev libcurl4-openssl-dev libedit-dev libffi-dev libfreetype6-dev libicu-dev libjpeg-dev libmcrypt-dev libmemcached-dev libodbc1 libonig-dev libpq-dev libpng-dev librabbitmq-dev librdkafka-dev libsodium-dev libsqlite3-dev libssl-dev libwebp-dev libxml2-dev libxslt1-dev libzip-dev libc6-dbg odbcinst1debian2 zlib1g-dev unixodbc-dev unixodbc' -export DDAGENT_HOSTNAME='ddagent_integration' -export DEBIAN_FRONTEND='noninteractive' -export OLDPWD='/home/circleci/app/target/debug/build' -export _='/usr/local/bin/php' -export TEMP='/tmp' -export TEST_PHP_EXECUTABLE='/opt/php/debug/bin/php' -export TEST_PHP_EXECUTABLE_ESCAPED=''\''/opt/php/debug/bin/php'\''' -export TEST_PHP_CGI_EXECUTABLE='/opt/php/debug/bin/php-cgi' -export TEST_PHP_CGI_EXECUTABLE_ESCAPED=''\''/opt/php/debug/bin/php-cgi'\''' -export TEST_PHPDBG_EXECUTABLE='' -export TEST_PHPDBG_EXECUTABLE_ESCAPED=''\'''\''' -export REDIRECT_STATUS='1' -export QUERY_STRING='' -export PATH_TRANSLATED='/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php' -export SCRIPT_FILENAME='/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php' -export REQUEST_METHOD='GET' -export CONTENT_TYPE='' -export CONTENT_LENGTH='' -export TZ='' -export DD_TRACE_HTTP_SERVER_ERROR_STATUSES='500-599' -export DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='400-499' -export TEST_PHP_EXTRA_ARGS=' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0"' -export HTTP_COOKIE='' - -case "$1" in -"gdb") - gdb --args '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 - ;; -"lldb") - lldb -- '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 - ;; -"valgrind") - USE_ZEND_ALLOC=0 valgrind $2 '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 - ;; -"rr") - rr record $2 '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 - ;; -*) - '/opt/php/debug/bin/php' -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=0" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "serialize_precision=-1" -d "memory_limit=128M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "zend.assertions=1" -d "zend.exception_ignore_args=0" -d "zend.exception_string_param_max_len=15" -d "short_open_tag=0" -f "/home/circleci/app/tests/ext/serialization/http_server_vs_client_status.php" 2>&1 - ;; -esac \ No newline at end of file From 4e5eed0916b2ba1432f91103ff830e900fee494d Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 11:36:41 -0400 Subject: [PATCH 03/16] remove some unused debug vars --- ext/configuration.c | 1 + ext/serializer.c | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/configuration.c b/ext/configuration.c index 6e90aa0cacb..bf67ad362cd 100644 --- a/ext/configuration.c +++ b/ext/configuration.c @@ -158,6 +158,7 @@ static bool dd_parse_tags(zai_str value, zval *decoded_value, bool persistent) { } static bool dd_parse_http_error_statuses(zai_str value, zval *decoded_value, bool persistent) { + UNUSED(persistent); array_init(decoded_value); if (value.len == 0) { diff --git a/ext/serializer.c b/ext/serializer.c index 0cdc58f8e5f..65d5c67c7da 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1479,8 +1479,6 @@ void transfer_data(zend_array *source, zend_array *destination, const char *key, static bool _dd_should_mark_as_error(ddtrace_span_data *span) { // Explicitly set errors are the priority zend_array *meta = ddtrace_property_array(&span->property_meta); - zend_string *key; - zval *zv; if (meta) { zval *error_zv = zend_hash_str_find(meta, "error", sizeof("error") - 1); if (error_zv && Z_TYPE_P(error_zv) == IS_LONG) { @@ -1533,6 +1531,7 @@ static bool _dd_should_mark_as_error(ddtrace_span_data *span) { // For SET, the keys are the status codes/ranges ZEND_HASH_FOREACH_STR_KEY_VAL(cfg, str_key, entry_zv) { + UNUSED(entry_zv); if (str_key) { const char *s = ZSTR_VAL(str_key); char *dash = strchr(s, '-'); From f8cba28d29e4cf76dc057268e0fa5f9bff3e4a16 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 11:42:50 -0400 Subject: [PATCH 04/16] use a different macro --- ext/serializer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/serializer.c b/ext/serializer.c index 65d5c67c7da..f21dab929a3 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1530,8 +1530,7 @@ static bool _dd_should_mark_as_error(ddtrace_span_data *span) { zval *entry_zv; // For SET, the keys are the status codes/ranges - ZEND_HASH_FOREACH_STR_KEY_VAL(cfg, str_key, entry_zv) { - UNUSED(entry_zv); + ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { if (str_key) { const char *s = ZSTR_VAL(str_key); char *dash = strchr(s, '-'); From 1a50ccc79673bd5ea24949cb45a570e527e7bc09 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 11:49:52 -0400 Subject: [PATCH 05/16] oops missed one --- ext/serializer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/serializer.c b/ext/serializer.c index f21dab929a3..6d9694d9b90 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1527,7 +1527,6 @@ static bool _dd_should_mark_as_error(ddtrace_span_data *span) { if (cfg_sz > 0) { zend_string *str_key; - zval *entry_zv; // For SET, the keys are the status codes/ranges ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { From 505ff642160a378ddc17f52b59c7615d0a55eacc Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 12:14:28 -0400 Subject: [PATCH 06/16] only consider http status if explicitly set --- ext/serializer.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/serializer.c b/ext/serializer.c index 6d9694d9b90..0fb59c9800b 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1478,7 +1478,7 @@ void transfer_data(zend_array *source, zend_array *destination, const char *key, static bool _dd_should_mark_as_error(ddtrace_span_data *span) { // Explicitly set errors are the priority - zend_array *meta = ddtrace_property_array(&span->property_meta); + zend_array *meta = ddtrace_property_array(&span->property_meta); if (meta) { zval *error_zv = zend_hash_str_find(meta, "error", sizeof("error") - 1); if (error_zv && Z_TYPE_P(error_zv) == IS_LONG) { @@ -1523,12 +1523,10 @@ static bool _dd_should_mark_as_error(ddtrace_span_data *span) { zend_array *cfg = is_client_span ? get_DD_TRACE_HTTP_CLIENT_ERROR_STATUSES() : get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); - size_t cfg_sz = cfg ? zend_hash_num_elements(cfg) : 0; - if (cfg_sz > 0) { + // Only check status codes if configuration is explicitly set + if (cfg && zend_hash_num_elements(cfg) > 0) { zend_string *str_key; - - // For SET, the keys are the status codes/ranges ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { if (str_key) { const char *s = ZSTR_VAL(str_key); @@ -1552,8 +1550,11 @@ static bool _dd_should_mark_as_error(ddtrace_span_data *span) { } } ZEND_HASH_FOREACH_END(); + // If we get here with specific configuration but no match, + // this status code is not considered an error return false; } + // If no configuration is set, we fall through to exception checking } } From d79d2bd98a58acef54747509907f8300bf79b045 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 12:26:20 -0400 Subject: [PATCH 07/16] remove custom parser --- ext/configuration.c | 45 --------------------------------------------- ext/configuration.h | 6 ++---- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/ext/configuration.c b/ext/configuration.c index bf67ad362cd..d3aa42dcc85 100644 --- a/ext/configuration.c +++ b/ext/configuration.c @@ -157,51 +157,6 @@ static bool dd_parse_tags(zai_str value, zval *decoded_value, bool persistent) { return true; } -static bool dd_parse_http_error_statuses(zai_str value, zval *decoded_value, bool persistent) { - UNUSED(persistent); - array_init(decoded_value); - - if (value.len == 0) { - return true; - } - - // Add null termination - char *buf = estrndup(value.ptr, value.len); - if (!buf) { - return false; - } - - char *tok, *saveptr = NULL; - - for (tok = strtok_r(buf, ",", &saveptr); tok; tok = strtok_r(NULL, ",", &saveptr)) { - // Trim whitespace - char *start = tok; - char *end = tok + strlen(tok) - 1; - - while (start <= end && isspace((unsigned char)*start)) start++; - while (end > start && isspace((unsigned char)*end)) *end-- = '\0'; - - // Remove quotes if present - if (strlen(start) >= 2 && start[0] == '"' && start[strlen(start)-1] == '"') { - start++; - start[strlen(start)-1] = '\0'; - } - - if (strlen(start) > 0) { - // For SET, the value is the key and the value is boolean true - zval true_val; - ZVAL_TRUE(&true_val); - - // Add to array using string key with boolean true value - zend_hash_str_update(Z_ARR_P(decoded_value), start, strlen(start), &true_val); - } - } - - efree(buf); - - return true; -} - #define INI_CHANGE_DYNAMIC_CONFIG(name, config) \ static bool ddtrace_alter_##name(zval *old_value, zval *new_value, zend_string *new_str) { \ UNUSED(old_value, new_value); \ diff --git a/ext/configuration.h b/ext/configuration.h index e956c57673d..c794f3f12e4 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -248,10 +248,8 @@ enum ddtrace_sampling_rules_format { CONFIG(INT, DD_TRACE_BAGGAGE_MAX_ITEMS, "64") \ CONFIG(INT, DD_TRACE_BAGGAGE_MAX_BYTES, "8192") \ CONFIG(BOOL, DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED, "false") \ - CONFIG(SET, DD_TRACE_HTTP_CLIENT_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change, \ - .parser = dd_parse_http_error_statuses) \ - CONFIG(SET, DD_TRACE_HTTP_SERVER_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change, \ - .parser = dd_parse_http_error_statuses) \ + CONFIG(SET, DD_TRACE_HTTP_CLIENT_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change) \ + CONFIG(SET, DD_TRACE_HTTP_SERVER_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change) \ DD_INTEGRATIONS #ifndef _WIN32 From 4ce1cfad10041289879ffc1bd3f1b3984087cf24 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 15:36:44 -0400 Subject: [PATCH 08/16] refactoring for review --- ext/serializer.c | 203 ++++++------------ .../Integrations/Curl/CurlIntegration.php | 144 +++++++++---- .../Integrations/Guzzle/GuzzleIntegration.php | 82 ++++++- .../Integrations/Psr18/Psr18Integration.php | 56 ++++- .../http_error_status_is_client_error.phpt | 76 +++++++ .../http_server_error_status_configured.phpt | 77 +++++++ .../http_client_error_status_configured.phpt | 20 -- .../http_server_error_status_configured.phpt | 19 -- ...server_status_with_explicit_error_set.phpt | 44 ---- .../http_server_vs_client_status.phpt | 51 ----- ...http_status_mixed_range_configuration.phpt | 41 ---- ...tatus_no_configuration_with_exception.phpt | 47 ---- 12 files changed, 448 insertions(+), 412 deletions(-) create mode 100644 tests/ext/error_status_configuration/http_error_status_is_client_error.phpt create mode 100644 tests/ext/error_status_configuration/http_server_error_status_configured.phpt delete mode 100644 tests/ext/serialization/http_client_error_status_configured.phpt delete mode 100644 tests/ext/serialization/http_server_error_status_configured.phpt delete mode 100644 tests/ext/serialization/http_server_status_with_explicit_error_set.phpt delete mode 100644 tests/ext/serialization/http_server_vs_client_status.phpt delete mode 100644 tests/ext/serialization/http_status_mixed_range_configuration.phpt delete mode 100644 tests/ext/serialization/http_status_no_configuration_with_exception.phpt diff --git a/ext/serializer.c b/ext/serializer.c index 0fb59c9800b..537ab82c3cd 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1152,10 +1152,77 @@ static void dd_set_entrypoint_root_span_props_end(zend_array *meta, int status, ZVAL_STR(&status_zv, status_str); zend_hash_str_update(meta, ZEND_STRL("http.status_code"), &status_zv); - if (status >= 500 && !ignore_error) { - zval zv = {0}, *value; - if ((value = zend_hash_str_add(meta, ZEND_STRL("error.type"), &zv))) { - ZVAL_STR(value, zend_string_init(ZEND_STRL("Internal Server Error"), 0)); + // Only check status codes if not ignoring errors + if (!ignore_error) { + bool is_error = false; + bool is_custom_error = false; + + // Get server error configuration + zend_array *cfg = get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); + + // If configuration exists and has entries, check against it + if (cfg && zend_hash_num_elements(cfg) > 0) { + // Check custom configuration first + zend_string *str_key; + ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { + if (str_key) { + const char *s = ZSTR_VAL(str_key); + char *dash = strchr(s, '-'); + + if (dash) { + // Range like "500-599" + int start, end; + if (sscanf(s, "%d-%d", &start, &end) == 2) { + if (status >= start && status <= end) { + is_error = true; + is_custom_error = true; + break; + } + } + } else { + // Single status code + int code = atoi(s); + if (status == code) { + is_error = true; + is_custom_error = true; + break; + } + } + } + } ZEND_HASH_FOREACH_END(); + + // If no custom match, still check for >=500 for backward compatibility + if (!is_error) { + is_error = (status >= 500); + } + } else { + // Fallback to original behavior if no configuration is set + is_error = (status >= 500); + } + + if (is_error) { + zval zv = {0}, *value; + if ((value = zend_hash_str_add(meta, ZEND_STRL("error.type"), &zv))) { + if (is_custom_error) { + // New behavior for custom-configured errors + ZVAL_STR(value, zend_string_init(ZEND_STRL("http_error"), 0)); + + // Add error message and flag + zval error_msg_zv = {0}, *msg_value; + if ((msg_value = zend_hash_str_add(meta, ZEND_STRL("error.msg"), &error_msg_zv))) { + char error_msg[50]; + snprintf(error_msg, sizeof(error_msg), "HTTP %d Error", status); + ZVAL_STR(msg_value, zend_string_init(error_msg, strlen(error_msg), 0)); + } + + zval error_flag; + ZVAL_LONG(&error_flag, 1); + zend_hash_str_update(meta, ZEND_STRL("error"), &error_flag); + } else { + // Original behavior for >= 500 errors + ZVAL_STR(value, zend_string_init(ZEND_STRL("Internal Server Error"), 0)); + } + } } } } @@ -1476,97 +1543,6 @@ void transfer_data(zend_array *source, zend_array *destination, const char *key, } } -static bool _dd_should_mark_as_error(ddtrace_span_data *span) { - // Explicitly set errors are the priority - zend_array *meta = ddtrace_property_array(&span->property_meta); - if (meta) { - zval *error_zv = zend_hash_str_find(meta, "error", sizeof("error") - 1); - if (error_zv && Z_TYPE_P(error_zv) == IS_LONG) { - if (Z_LVAL_P(error_zv) == 1) { - return true; - } - if (Z_LVAL_P(error_zv) == 0) { - return false; - } - } - } - - // HTTP status configured - zval *status_zv = meta - ? zend_hash_str_find(meta, "http.status_code", sizeof("http.status_code") - 1) - : NULL; - if (status_zv) { - int status_code = 0; - if (Z_TYPE_P(status_zv) == IS_STRING) { - status_code = atoi(Z_STRVAL_P(status_zv)); - } else if (Z_TYPE_P(status_zv) == IS_LONG) { - status_code = Z_LVAL_P(status_zv); - } - - if (status_code > 0) { - // Determine if client or server span. Check kind first and fallback to type - bool is_client_span = false; - zval *kind_zv = zend_hash_str_find(meta, "span.kind", sizeof("span.kind") - 1); - if (kind_zv && Z_TYPE_P(kind_zv) == IS_STRING && - strcmp(Z_STRVAL_P(kind_zv), "client") == 0) { - is_client_span = true; - } else { - zval *type_zv = zend_hash_str_find(meta, "span.type", sizeof("span.type") - 1); - if (type_zv && Z_TYPE_P(type_zv) == IS_STRING && - (strcmp(Z_STRVAL_P(type_zv), "http") == 0 || - strcmp(Z_STRVAL_P(type_zv), "client") == 0)) { - is_client_span = true; - } - } - - // Get the proper configuration - zend_array *cfg = is_client_span - ? get_DD_TRACE_HTTP_CLIENT_ERROR_STATUSES() - : get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); - - // Only check status codes if configuration is explicitly set - if (cfg && zend_hash_num_elements(cfg) > 0) { - zend_string *str_key; - ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { - if (str_key) { - const char *s = ZSTR_VAL(str_key); - char *dash = strchr(s, '-'); - - if (dash) { - // Range like "500-599" - int start, end; - if (sscanf(s, "%d-%d", &start, &end) == 2) { - if (status_code >= start && status_code <= end) { - return true; - } - } - } else { - // Single status code - int code = atoi(s); - if (code == status_code) { - return true; - } - } - } - } ZEND_HASH_FOREACH_END(); - - // If we get here with specific configuration but no match, - // this status code is not considered an error - return false; - } - // If no configuration is set, we fall through to exception checking - } - } - - // Exception with no HTTP status configuration - zval *ex_zv = &span->property_exception; - if (Z_TYPE_P(ex_zv) == IS_OBJECT) { - return true; - } - - return false; -} - zval *ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { bool is_root_span = span->std.ce == ddtrace_ce_root_span_data; bool is_inferred_span = span->std.ce == ddtrace_ce_inferred_span_data; @@ -1924,43 +1900,6 @@ zval *ddtrace_serialize_span_to_array(ddtrace_span_data *span, zval *array) { } } - // Figure out if an error should be tracked - bool is_error = _dd_should_mark_as_error(span); - - if (is_error) { - zval zv_error; - ZVAL_LONG(&zv_error, 1); - add_assoc_zval(el, "error", &zv_error); - - // If there's an exception, the existing serializer code will handle it - // We only need to add error metadata for HTTP status code errors (no exception) - zval *exception = &span->property_exception; - if (Z_TYPE_P(exception) != IS_OBJECT) { - // HTTP status code error case - // Add error meta if not already present - zval *serialized_meta = zend_hash_str_find(Z_ARR_P(el), ZEND_STRL("meta")); - if (serialized_meta && Z_TYPE_P(serialized_meta) == IS_ARRAY) { - zend_array *meta_arr = Z_ARR_P(serialized_meta); - if (!zend_hash_str_exists(meta_arr, ZEND_STRL("error.type"))) { - add_assoc_str(serialized_meta, "error.type", zend_string_init("http_error", sizeof("http_error") - 1, 0)); - } - - if (!zend_hash_str_exists(meta_arr, ZEND_STRL("error.msg"))) { - zval *status_code_zv = zend_hash_str_find(meta, ZEND_STRL("http.status_code")); - if (status_code_zv) { - char error_msg[50]; - if (Z_TYPE_P(status_code_zv) == IS_STRING) { - snprintf(error_msg, sizeof(error_msg), "HTTP %s Error", Z_STRVAL_P(status_code_zv)); - } else if (Z_TYPE_P(status_code_zv) == IS_LONG) { - snprintf(error_msg, sizeof(error_msg), "HTTP %ld Error", Z_LVAL_P(status_code_zv)); - } - add_assoc_string(serialized_meta, "error.msg", error_msg); - } - } - } - } - } - if (inferred_span) { zval *serialized_inferred_span = ddtrace_serialize_span_to_array(inferred_span, array); zend_array *serialized_inferred_span_meta = Z_ARR_P(zend_hash_str_find(Z_ARR_P(serialized_inferred_span), ZEND_STRL("meta"))); diff --git a/src/DDTrace/Integrations/Curl/CurlIntegration.php b/src/DDTrace/Integrations/Curl/CurlIntegration.php index 61304f8d294..9d2e34e2d60 100644 --- a/src/DDTrace/Integrations/Curl/CurlIntegration.php +++ b/src/DDTrace/Integrations/Curl/CurlIntegration.php @@ -25,6 +25,46 @@ function addSpanDataTagFromCurlInfo($span, &$info, $tagName, $curlInfoOpt) } } + /** + * Determines if a given status code should be considered an error + * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. + * + * @param int $statusCode The HTTP status code to check + * @return bool Whether the status code should be considered an error + */ +private static function isClientError($statusCode) { + // Get configured status codes from environment + $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); + + if (!empty($errorStatusCodes)) { + // Custom configuration exists, use it + $codesList = explode(',', $errorStatusCodes); + + foreach ($codesList as $item) { + $item = trim($item); + + if (strpos($item, '-') !== false) { + // Range ("400-499") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior + return ($statusCode >= 400); + } +} + final class CurlIntegration extends Integration { const NAME = 'curl'; @@ -251,51 +291,61 @@ public function setup_curl_span($span) { $span->meta[Tag::SPAN_KIND] = Tag::SPAN_KIND_VALUE_CLIENT; } - public static function set_curl_attributes($span, $info) { - $sanitizedUrl = \DDTrace\Util\Normalizer::urlSanitize($info['url']); - $normalizedPath = \DDTrace\Util\Normalizer::uriNormalizeOutgoingPath($info['url']); - $host = Urls::hostname($sanitizedUrl); - $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; - unset($info['url']); - - if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { - $span->service = Urls::hostnameForTag($sanitizedUrl); - } - - $span->resource = $normalizedPath; - - /* Special case the Datadog Standard Attributes - * See https://docs.datadoghq.com/logs/processing/attributes_naming_convention/ - */ - if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { - $span->meta[Tag::HTTP_URL] = $sanitizedUrl; - } - - $span->peerServiceSources = HttpClientIntegrationHelper::PEER_SERVICE_SOURCES; - - addSpanDataTagFromCurlInfo($span, $info, Tag::HTTP_STATUS_CODE, 'http_code'); - - addSpanDataTagFromCurlInfo($span, $info, 'network.client.ip', 'local_ip'); - addSpanDataTagFromCurlInfo($span, $info, 'network.client.port', 'local_port'); - - addSpanDataTagFromCurlInfo($span, $info, 'network.destination.ip', 'primary_ip'); - addSpanDataTagFromCurlInfo($span, $info, 'network.destination.port', 'primary_port'); - - addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_read', 'size_download'); - addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_written', 'size_upload'); - - // Add the rest to a 'curl.' object - foreach ($info as $key => $val) { - // Datadog doesn't support arrays in tags - if (\is_scalar($val) && $val !== '') { - // Datadog sets durations in nanoseconds - convert from seconds - if (\substr_compare($key, '_time', -5) === 0) { - $val *= 1000000000; - } - $span->meta["curl.{$key}"] = $val; - } - } - - return $info; - } + public static function set_curl_attributes($span, $info) { + $sanitizedUrl = \DDTrace\Util\Normalizer::urlSanitize($info['url']); + $normalizedPath = \DDTrace\Util\Normalizer::uriNormalizeOutgoingPath($info['url']); + $host = Urls::hostname($sanitizedUrl); + $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; + unset($info['url']); + + if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { + $span->service = Urls::hostnameForTag($sanitizedUrl); + } + + $span->resource = $normalizedPath; + + /* Special case the Datadog Standard Attributes + * See https://docs.datadoghq.com/logs/processing/attributes_naming_convention/ + */ + if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { + $span->meta[Tag::HTTP_URL] = $sanitizedUrl; + } + + $span->peerServiceSources = HttpClientIntegrationHelper::PEER_SERVICE_SOURCES; + + addSpanDataTagFromCurlInfo($span, $info, Tag::HTTP_STATUS_CODE, 'http_code'); + + // Check if the status code should be marked as an error based on configuration + if (isset($info['http_code']) && !empty($info['http_code']) && !isset($span->meta[Tag::ERROR])) { + $statusCode = (int)$info['http_code']; + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode Error"; + } + } + + addSpanDataTagFromCurlInfo($span, $info, 'network.client.ip', 'local_ip'); + addSpanDataTagFromCurlInfo($span, $info, 'network.client.port', 'local_port'); + + addSpanDataTagFromCurlInfo($span, $info, 'network.destination.ip', 'primary_ip'); + addSpanDataTagFromCurlInfo($span, $info, 'network.destination.port', 'primary_port'); + + addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_read', 'size_download'); + addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_written', 'size_upload'); + + // Add the rest to a 'curl.' object + foreach ($info as $key => $val) { + // Datadog doesn't support arrays in tags + if (\is_scalar($val) && $val !== '') { + // Datadog sets durations in nanoseconds - convert from seconds + if (\substr_compare($key, '_time', -5) === 0) { + $val *= 1000000000; + } + $span->meta["curl.{$key}"] = $val; + } + } + + return $info; + } } diff --git a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php index 2bf782333a1..5c3aabbad82 100644 --- a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php +++ b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php @@ -50,14 +50,38 @@ function (SpanData $span, $args, $retval) use ($integration) { $response = $retval; if (\is_a($response, 'GuzzleHttp\Message\ResponseInterface')) { /** @var \GuzzleHttp\Message\ResponseInterface $response */ - $span->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode(); + $statusCode = $response->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + + // Mark as error if status code matches configuration + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); + } } elseif (\is_a($response, 'Psr\Http\Message\ResponseInterface')) { /** @var \Psr\Http\Message\ResponseInterface $response */ - $span->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode(); + $statusCode = $response->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + + // Mark as error if status code matches configuration + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); + } } elseif (\is_a($response, 'GuzzleHttp\Promise\PromiseInterface')) { /** @var \GuzzleHttp\Promise\PromiseInterface $response */ $response->then(function (\Psr\Http\Message\ResponseInterface $response) use ($span) { - $span->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode(); + $statusCode = $response->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + + // Mark as error if status code matches configuration + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); + } }); } } @@ -84,7 +108,15 @@ function (SpanData $span, $args, $retval) use ($integration) { if (\is_a($response, 'GuzzleHttp\Promise\PromiseInterface')) { /** @var \GuzzleHttp\Promise\PromiseInterface $response */ $response->then(function (\Psr\Http\Message\ResponseInterface $response) use ($span) { - $span->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode(); + $statusCode = $response->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + + // Mark as error if status code matches configuration + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); + } }); } } @@ -125,4 +157,44 @@ public function addRequestInfo(SpanData $span, $request) } } } -} + + /** + * Determines if a given status code should be considered an error + * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. + * + * @param int $statusCode The HTTP status code to check + * @return bool Whether the status code should be considered an error + */ + private static function isClientError($statusCode) { + // Get configured status codes from environment + $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); + + if (!empty($errorStatusCodes)) { + // Custom configuration exists, use it + $codesList = explode(',', $errorStatusCodes); + + foreach ($codesList as $item) { + $item = trim($item); + + if (strpos($item, '-') !== false) { + // Range ("400-499") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior + return ($statusCode >= 400); + } + } +} \ No newline at end of file diff --git a/src/DDTrace/Integrations/Psr18/Psr18Integration.php b/src/DDTrace/Integrations/Psr18/Psr18Integration.php index 2287036bbbc..c62cfce1434 100644 --- a/src/DDTrace/Integrations/Psr18/Psr18Integration.php +++ b/src/DDTrace/Integrations/Psr18/Psr18Integration.php @@ -1,5 +1,4 @@ meta[Tag::HTTP_STATUS_CODE] = $retval->getStatusCode(); + $statusCode = $retval->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + + // Mark as error if status code matches configuration and no error is already set + if (self::isClientError($statusCode) && !isset($span->meta[Tag::ERROR])) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $retval->getReasonPhrase(); + } } } ); @@ -50,18 +57,55 @@ public function addRequestInfo(SpanData $span, $request) /** @var \Psr\Http\Message\RequestInterface $request */ $url = $request->getUri(); $host = Urls::hostname($url); - if ($host) { $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; } - if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { $span->service = Urls::hostnameForTag($url); } $span->meta[Tag::HTTP_METHOD] = $request->getMethod(); - if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { $span->meta[Tag::HTTP_URL] = \DDTrace\Util\Normalizer::urlSanitize($url); } } -} + + /** + * Determines if a given status code should be considered an error + * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. + * + * @param int $statusCode The HTTP status code to check + * @return bool Whether the status code should be considered an error + */ + private static function isClientError($statusCode) { + // Get configured status codes from environment + $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); + + if (!empty($errorStatusCodes)) { + // Custom configuration exists, use it + $codesList = explode(',', $errorStatusCodes); + + foreach ($codesList as $item) { + $item = trim($item); + + if (strpos($item, '-') !== false) { + // Range ("400-499") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior + return ($statusCode >= 400); + } + } +} \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt b/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt new file mode 100644 index 00000000000..1ae0f865fc8 --- /dev/null +++ b/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt @@ -0,0 +1,76 @@ +--TEST-- +HTTP client status code error configuration parsing +--INI-- +extension=ddtrace.so +--ENV-- +DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=403,408-417 +--FILE-- += (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior + return ($statusCode >= 400); + } +} + +// Test various status codes directly against the function +echo "-- With custom configuration --\n"; +echo "Status 200: " . (isClientError(200) ? "Error" : "Not error") . "\n"; +echo "Status 404: " . (isClientError(404) ? "Error" : "Not error") . "\n"; +echo "Status 403: " . (isClientError(403) ? "Error" : "Not error") . "\n"; +echo "Status 408: " . (isClientError(408) ? "Error" : "Not error") . "\n"; +echo "Status 412: " . (isClientError(412) ? "Error" : "Not error") . "\n"; +echo "Status 417: " . (isClientError(417) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isClientError(500) ? "Error" : "Not error") . "\n"; +echo "Status 503: " . (isClientError(503) ? "Error" : "Not error") . "\n"; + +// Now test default behavior without config +putenv('DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='); + +echo "\n-- Without custom configuration --\n"; +echo "Status 200: " . (isClientError(200) ? "Error" : "Not error") . "\n"; +echo "Status 404: " . (isClientError(404) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isClientError(500) ? "Error" : "Not error") . "\n"; +?> +--EXPECT-- +-- With custom configuration -- +Status 200: Not error +Status 404: Not error +Status 403: Error +Status 408: Error +Status 412: Error +Status 417: Error +Status 500: Not error +Status 503: Not error + +-- Without custom configuration -- +Status 200: Not error +Status 404: Error +Status 500: Error \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_server_error_status_configured.phpt b/tests/ext/error_status_configuration/http_server_error_status_configured.phpt new file mode 100644 index 00000000000..74045c42580 --- /dev/null +++ b/tests/ext/error_status_configuration/http_server_error_status_configured.phpt @@ -0,0 +1,77 @@ +--TEST-- +HTTP status code configuration parsing test +--INI-- +extension=ddtrace.so +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,429-451,503 +--FILE-- += (int)$start && $code <= (int)$end) { + $isError = true; + break; + } + } else { + // It's a single code + if ($code == (int)$item) { + $isError = true; + break; + } + } + } + } else { + // No custom configuration - use default behavior (500+ are errors) + $isError = ($code >= 500); + } + + return $isError; +} + +// Test various status codes +echo "-- With custom configuration --\n"; +echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; +echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; +echo "Status 429: " . (isStatusError(429) ? "Error" : "Not error") . "\n"; +echo "Status 451: " . (isStatusError(451) ? "Error" : "Not error") . "\n"; +echo "Status 490: " . (isStatusError(490) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; +echo "Status 503: " . (isStatusError(503) ? "Error" : "Not error") . "\n"; + +// Now test default behavior without config +putenv('DD_TRACE_HTTP_SERVER_ERROR_STATUSES='); + +echo "\n-- Without custom configuration --\n"; +echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; +echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; +?> +--EXPECT-- +-- With custom configuration -- +Status 200: Not error +Status 418: Error +Status 429: Error +Status 451: Error +Status 490: Not error +Status 500: Not error +Status 503: Error + +-- Without custom configuration -- +Status 200: Not error +Status 418: Not error +Status 500: Error \ No newline at end of file diff --git a/tests/ext/serialization/http_client_error_status_configured.phpt b/tests/ext/serialization/http_client_error_status_configured.phpt deleted file mode 100644 index 078016f1e9e..00000000000 --- a/tests/ext/serialization/http_client_error_status_configured.phpt +++ /dev/null @@ -1,20 +0,0 @@ ---TEST-- -Client span with configured error status code (basic functionality) ---ENV-- -DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=400-499 ---FILE-- -meta['http.status_code'] = 404; -$span->meta['span.kind'] = 'client'; -\DDTrace\close_span(); - -$serialized = dd_trace_serialize_closed_spans(); -var_dump($serialized[0]['error'] ?? null); -var_dump($serialized[0]['meta']['error.type'] ?? null); -var_dump($serialized[0]['meta']['error.msg'] ?? null); -?> ---EXPECT-- -int(1) -string(10) "http_error" -string(14) "HTTP 404 Error" \ No newline at end of file diff --git a/tests/ext/serialization/http_server_error_status_configured.phpt b/tests/ext/serialization/http_server_error_status_configured.phpt deleted file mode 100644 index 5d038e4bfee..00000000000 --- a/tests/ext/serialization/http_server_error_status_configured.phpt +++ /dev/null @@ -1,19 +0,0 @@ ---TEST-- -Server span with configured error status code (basic functionality) ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 ---FILE-- -meta['http.status_code'] = 500; -\DDTrace\close_span(); - -$serialized = dd_trace_serialize_closed_spans(); -var_dump($serialized[0]['error'] ?? null); -var_dump($serialized[0]['meta']['error.type'] ?? null); -var_dump($serialized[0]['meta']['error.msg'] ?? null); -?> ---EXPECT-- -int(1) -string(10) "http_error" -string(14) "HTTP 500 Error" \ No newline at end of file diff --git a/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt b/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt deleted file mode 100644 index 59563ec552e..00000000000 --- a/tests/ext/serialization/http_server_status_with_explicit_error_set.phpt +++ /dev/null @@ -1,44 +0,0 @@ ---TEST-- -Explicit error flag takes precedence over status code configuration ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 ---FILE-- -meta['http.status_code'] = 500; -$span1->meta['error'] = 0; -$span1->meta['debug_id'] = 'EXPLICIT_NO_ERROR'; -\DDTrace\close_span(); - -// Should be error due to explicit flag = 1, even though status code is 200 -$span2 = \DDTrace\start_span(); -$span2->meta['http.status_code'] = 200; -$span2->meta['error'] = 1; -$span2->meta['debug_id'] = 'EXPLICIT_ERROR'; -\DDTrace\close_span(); - -$serialized = dd_trace_serialize_closed_spans(); - -// Find spans by debug ID -$no_error_span = null; -$error_span = null; -foreach ($serialized as $span) { - if (isset($span['meta']['debug_id'])) { - if ($span['meta']['debug_id'] === 'EXPLICIT_NO_ERROR') { - $no_error_span = $span; - } elseif ($span['meta']['debug_id'] === 'EXPLICIT_ERROR') { - $error_span = $span; - } - } -} - -echo "Explicit error=0 with 500 status: "; -var_dump(isset($no_error_span['error']) ? $no_error_span['error'] : null); - -echo "Explicit error=1 with 200 status: "; -var_dump(isset($error_span['error']) ? $error_span['error'] : null); -?> ---EXPECT-- -Explicit error=0 with 500 status: NULL -Explicit error=1 with 200 status: int(1) \ No newline at end of file diff --git a/tests/ext/serialization/http_server_vs_client_status.phpt b/tests/ext/serialization/http_server_vs_client_status.phpt deleted file mode 100644 index b0f2a53c5a7..00000000000 --- a/tests/ext/serialization/http_server_vs_client_status.phpt +++ /dev/null @@ -1,51 +0,0 @@ ---TEST-- -Client and server spans use their respective configurations ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599 -DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=400-499 ---FILE-- -meta['http.status_code'] = 404; -$span_server->meta['debug_id'] = 'SERVER_SPAN'; -\DDTrace\close_span(); - -// 2. Client span with 404 - should be error -echo "=== CLIENT SPAN WITH 404 ===\n"; -$span_client = \DDTrace\start_span(); -$span_client->meta['http.status_code'] = 404; -$span_client->meta['span.kind'] = 'client'; -$span_client->meta['debug_id'] = 'CLIENT_SPAN'; -\DDTrace\close_span(); - -// Get serialized spans -$serialized = dd_trace_serialize_closed_spans(); - -// Check results -echo "=== RESULTS ===\n"; -// Find spans by debug ID -$client_span = null; -$server_span = null; -foreach ($serialized as $span) { - if (isset($span['meta']['debug_id'])) { - if ($span['meta']['debug_id'] === 'CLIENT_SPAN') { - $client_span = $span; - } elseif ($span['meta']['debug_id'] === 'SERVER_SPAN') { - $server_span = $span; - } - } -} - -var_dump("Client 404 error:", isset($client_span['error']) ? $client_span['error'] : null); -var_dump("Server 404 error:", isset($server_span['error']) ? $server_span['error'] : null); -?> ---EXPECTF-- -=== SERVER SPAN WITH 404 === -=== CLIENT SPAN WITH 404 === -=== RESULTS === -string(17) "Client 404 error:" -int(1) -string(17) "Server 404 error:" -NULL \ No newline at end of file diff --git a/tests/ext/serialization/http_status_mixed_range_configuration.phpt b/tests/ext/serialization/http_status_mixed_range_configuration.phpt deleted file mode 100644 index 58ce5bfd763..00000000000 --- a/tests/ext/serialization/http_status_mixed_range_configuration.phpt +++ /dev/null @@ -1,41 +0,0 @@ ---TEST-- -Mixed range configuration with specific status codes ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=400-403,419,500-503 ---FILE-- -meta['http.status_code'] = $code; - $span->meta['debug_id'] = "STATUS_$code"; - \DDTrace\close_span(); -} - -$serialized = dd_trace_serialize_closed_spans(); - -// Check results for each status code -echo "=== RESULTS ===\n"; -foreach ($status_codes as $code) { - $span = null; - foreach ($serialized as $s) { - if (isset($s['meta']['debug_id']) && $s['meta']['debug_id'] === "STATUS_$code") { - $span = $s; - break; - } - } - - echo "Status $code is error: " . (isset($span['error']) ? 'YES' : 'NO') . "\n"; -} -?> ---EXPECT-- -=== RESULTS === -Status 400 is error: YES -Status 403 is error: YES -Status 404 is error: NO -Status 419 is error: YES -Status 500 is error: YES -Status 503 is error: YES -Status 504 is error: NO \ No newline at end of file diff --git a/tests/ext/serialization/http_status_no_configuration_with_exception.phpt b/tests/ext/serialization/http_status_no_configuration_with_exception.phpt deleted file mode 100644 index 788d6435067..00000000000 --- a/tests/ext/serialization/http_status_no_configuration_with_exception.phpt +++ /dev/null @@ -1,47 +0,0 @@ ---TEST-- -Exception fallback when no configuration is set ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,500-599 ---FILE-- -meta['http.status_code'] = 200; -try { - throw new Exception("Test exception"); -} catch (Exception $e) { - $span1->exception = $e; -} -$span1->meta['debug_id'] = 'EXCEPTION_SPAN'; -\DDTrace\close_span(); - -// Should NOT be error - 404 status code but no configuration -$span2 = \DDTrace\start_span(); -$span2->meta['http.status_code'] = 404; -$span2->meta['debug_id'] = 'NO_EXCEPTION_SPAN'; -\DDTrace\close_span(); - -$serialized = dd_trace_serialize_closed_spans(); - -// Find spans by debug ID -$exception_span = null; -$no_exception_span = null; -foreach ($serialized as $span) { - if (isset($span['meta']['debug_id'])) { - if ($span['meta']['debug_id'] === 'EXCEPTION_SPAN') { - $exception_span = $span; - } elseif ($span['meta']['debug_id'] === 'NO_EXCEPTION_SPAN') { - $no_exception_span = $span; - } - } -} - -echo "200 status with exception: "; -var_dump(isset($exception_span['error']) ? $exception_span['error'] : null); - -echo "404 status without exception or config: "; -var_dump(isset($no_exception_span['error']) ? $no_exception_span['error'] : null); -?> ---EXPECT-- -200 status with exception: int(1) -404 status without exception or config: NULL \ No newline at end of file From c09544c111b1d2033d7445a7263f4a1a2aaaeeb6 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Mon, 28 Apr 2025 16:14:14 -0400 Subject: [PATCH 09/16] remove left overs from local testing --- .../http_error_status_is_client_error.phpt | 2 -- .../http_server_error_status_configured.phpt | 2 -- 2 files changed, 4 deletions(-) diff --git a/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt b/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt index 1ae0f865fc8..019b85cb553 100644 --- a/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt +++ b/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt @@ -1,7 +1,5 @@ --TEST-- HTTP client status code error configuration parsing ---INI-- -extension=ddtrace.so --ENV-- DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=403,408-417 --FILE-- diff --git a/tests/ext/error_status_configuration/http_server_error_status_configured.phpt b/tests/ext/error_status_configuration/http_server_error_status_configured.phpt index 74045c42580..c550c2f1c0e 100644 --- a/tests/ext/error_status_configuration/http_server_error_status_configured.phpt +++ b/tests/ext/error_status_configuration/http_server_error_status_configured.phpt @@ -1,7 +1,5 @@ --TEST-- HTTP status code configuration parsing test ---INI-- -extension=ddtrace.so --ENV-- DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,429-451,503 --FILE-- From 7925c57d38def52e2fc15fec5e42a0f6ae9c7074 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 12:08:17 -0400 Subject: [PATCH 10/16] addressing review --- ext/configuration.c | 1 - ext/configuration.h | 4 +- ext/serializer.c | 79 +++----- .../Integrations/Curl/CurlIntegration.php | 146 +++++--------- .../Integrations/Guzzle/GuzzleIntegration.php | 72 +------ .../HttpClientIntegrationHelper.php | 72 ++++++- .../Integrations/Psr18/Psr18Integration.php | 47 +---- ...ttp_error_status_client_configuration.phpt | 178 ++++++++++++++++++ .../http_error_status_is_client_error.phpt | 74 -------- .../http_error_status_parsing.phpt | 110 +++++++++++ .../http_server_error_status_configured.phpt | 75 -------- 11 files changed, 442 insertions(+), 416 deletions(-) create mode 100644 tests/ext/error_status_configuration/http_error_status_client_configuration.phpt delete mode 100644 tests/ext/error_status_configuration/http_error_status_is_client_error.phpt create mode 100644 tests/ext/error_status_configuration/http_error_status_parsing.phpt delete mode 100644 tests/ext/error_status_configuration/http_server_error_status_configured.phpt diff --git a/ext/configuration.c b/ext/configuration.c index d3aa42dcc85..11a0c10202f 100644 --- a/ext/configuration.c +++ b/ext/configuration.c @@ -7,7 +7,6 @@ #include "json/json.h" #include "sidecar.h" #include -#include #include #include "sidecar.h" diff --git a/ext/configuration.h b/ext/configuration.h index c794f3f12e4..dab4d686230 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -248,8 +248,8 @@ enum ddtrace_sampling_rules_format { CONFIG(INT, DD_TRACE_BAGGAGE_MAX_ITEMS, "64") \ CONFIG(INT, DD_TRACE_BAGGAGE_MAX_BYTES, "8192") \ CONFIG(BOOL, DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED, "false") \ - CONFIG(SET, DD_TRACE_HTTP_CLIENT_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change) \ - CONFIG(SET, DD_TRACE_HTTP_SERVER_ERROR_STATUSES, "", .ini_change = zai_config_system_ini_change) \ + CONFIG(SET, DD_TRACE_HTTP_CLIENT_ERROR_STATUSES, "500-599", .ini_change = zai_config_system_ini_change) \ + CONFIG(SET, DD_TRACE_HTTP_SERVER_ERROR_STATUSES, "500-599", .ini_change = zai_config_system_ini_change) \ DD_INTEGRATIONS #ifndef _WIN32 diff --git a/ext/serializer.c b/ext/serializer.c index 537ab82c3cd..e72f40cb534 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1155,72 +1155,43 @@ static void dd_set_entrypoint_root_span_props_end(zend_array *meta, int status, // Only check status codes if not ignoring errors if (!ignore_error) { bool is_error = false; - bool is_custom_error = false; // Get server error configuration zend_array *cfg = get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); - // If configuration exists and has entries, check against it - if (cfg && zend_hash_num_elements(cfg) > 0) { - // Check custom configuration first - zend_string *str_key; - ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { - if (str_key) { - const char *s = ZSTR_VAL(str_key); - char *dash = strchr(s, '-'); - - if (dash) { - // Range like "500-599" - int start, end; - if (sscanf(s, "%d-%d", &start, &end) == 2) { - if (status >= start && status <= end) { - is_error = true; - is_custom_error = true; - break; - } - } - } else { - // Single status code - int code = atoi(s); - if (status == code) { - is_error = true; - is_custom_error = true; - break; - } + // Loop through configuration if any + zend_string *str_key; + ZEND_HASH_FOREACH_STR_KEY(cfg, str_key) { + if (str_key) { + const char *s = ZSTR_VAL(str_key); + + // Range like "500-599" + int start, end; + if (sscanf(s, "%d-%d", &start, &end) == 2) { + if (status >= start && status <= end) { + is_error = true; + break; + } + } else { + // Single status code + int code = atoi(s); + if (status == code) { + is_error = true; + break; } } - } ZEND_HASH_FOREACH_END(); - - // If no custom match, still check for >=500 for backward compatibility - if (!is_error) { - is_error = (status >= 500); } - } else { - // Fallback to original behavior if no configuration is set - is_error = (status >= 500); - } + } ZEND_HASH_FOREACH_END(); if (is_error) { zval zv = {0}, *value; if ((value = zend_hash_str_add(meta, ZEND_STRL("error.type"), &zv))) { - if (is_custom_error) { - // New behavior for custom-configured errors - ZVAL_STR(value, zend_string_init(ZEND_STRL("http_error"), 0)); - - // Add error message and flag - zval error_msg_zv = {0}, *msg_value; - if ((msg_value = zend_hash_str_add(meta, ZEND_STRL("error.msg"), &error_msg_zv))) { - char error_msg[50]; - snprintf(error_msg, sizeof(error_msg), "HTTP %d Error", status); - ZVAL_STR(msg_value, zend_string_init(error_msg, strlen(error_msg), 0)); - } + ZVAL_STR(value, zend_string_init(ZEND_STRL("http_error"), 0)); - zval error_flag; - ZVAL_LONG(&error_flag, 1); - zend_hash_str_update(meta, ZEND_STRL("error"), &error_flag); - } else { - // Original behavior for >= 500 errors - ZVAL_STR(value, zend_string_init(ZEND_STRL("Internal Server Error"), 0)); + // Add error message and flag + zval error_msg_zv = {0}, *msg_value; + if ((msg_value = zend_hash_str_add(meta, ZEND_STRL("error.msg"), &error_msg_zv))) { + ZVAL_STR(msg_value, zend_strpprintf(0, "HTTP %d Error", status)); } } } diff --git a/src/DDTrace/Integrations/Curl/CurlIntegration.php b/src/DDTrace/Integrations/Curl/CurlIntegration.php index 9d2e34e2d60..205d8734ccb 100644 --- a/src/DDTrace/Integrations/Curl/CurlIntegration.php +++ b/src/DDTrace/Integrations/Curl/CurlIntegration.php @@ -25,46 +25,6 @@ function addSpanDataTagFromCurlInfo($span, &$info, $tagName, $curlInfoOpt) } } - /** - * Determines if a given status code should be considered an error - * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. - * - * @param int $statusCode The HTTP status code to check - * @return bool Whether the status code should be considered an error - */ -private static function isClientError($statusCode) { - // Get configured status codes from environment - $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); - - if (!empty($errorStatusCodes)) { - // Custom configuration exists, use it - $codesList = explode(',', $errorStatusCodes); - - foreach ($codesList as $item) { - $item = trim($item); - - if (strpos($item, '-') !== false) { - // Range ("400-499") - list($start, $end) = explode('-', $item); - if ($statusCode >= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } - } - } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior - return ($statusCode >= 400); - } -} - final class CurlIntegration extends Integration { const NAME = 'curl'; @@ -292,60 +252,56 @@ public function setup_curl_span($span) { } public static function set_curl_attributes($span, $info) { - $sanitizedUrl = \DDTrace\Util\Normalizer::urlSanitize($info['url']); - $normalizedPath = \DDTrace\Util\Normalizer::uriNormalizeOutgoingPath($info['url']); - $host = Urls::hostname($sanitizedUrl); - $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; - unset($info['url']); - - if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { - $span->service = Urls::hostnameForTag($sanitizedUrl); - } - - $span->resource = $normalizedPath; - - /* Special case the Datadog Standard Attributes - * See https://docs.datadoghq.com/logs/processing/attributes_naming_convention/ - */ - if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { - $span->meta[Tag::HTTP_URL] = $sanitizedUrl; - } - - $span->peerServiceSources = HttpClientIntegrationHelper::PEER_SERVICE_SOURCES; - - addSpanDataTagFromCurlInfo($span, $info, Tag::HTTP_STATUS_CODE, 'http_code'); - - // Check if the status code should be marked as an error based on configuration - if (isset($info['http_code']) && !empty($info['http_code']) && !isset($span->meta[Tag::ERROR])) { - $statusCode = (int)$info['http_code']; - if (self::isClientError($statusCode)) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode Error"; - } - } - - addSpanDataTagFromCurlInfo($span, $info, 'network.client.ip', 'local_ip'); - addSpanDataTagFromCurlInfo($span, $info, 'network.client.port', 'local_port'); - - addSpanDataTagFromCurlInfo($span, $info, 'network.destination.ip', 'primary_ip'); - addSpanDataTagFromCurlInfo($span, $info, 'network.destination.port', 'primary_port'); - - addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_read', 'size_download'); - addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_written', 'size_upload'); - - // Add the rest to a 'curl.' object - foreach ($info as $key => $val) { - // Datadog doesn't support arrays in tags - if (\is_scalar($val) && $val !== '') { - // Datadog sets durations in nanoseconds - convert from seconds - if (\substr_compare($key, '_time', -5) === 0) { - $val *= 1000000000; - } - $span->meta["curl.{$key}"] = $val; - } - } - - return $info; + $sanitizedUrl = \DDTrace\Util\Normalizer::urlSanitize($info['url']); + $normalizedPath = \DDTrace\Util\Normalizer::uriNormalizeOutgoingPath($info['url']); + $host = Urls::hostname($sanitizedUrl); + $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; + unset($info['url']); + + if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { + $span->service = Urls::hostnameForTag($sanitizedUrl); + } + + $span->resource = $normalizedPath; + + /* Special case the Datadog Standard Attributes + * See https://docs.datadoghq.com/logs/processing/attributes_naming_convention/ + */ + if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { + $span->meta[Tag::HTTP_URL] = $sanitizedUrl; + } + + $span->peerServiceSources = HttpClientIntegrationHelper::PEER_SERVICE_SOURCES; + + addSpanDataTagFromCurlInfo($span, $info, Tag::HTTP_STATUS_CODE, 'http_code'); + + // Mark as an error if needed based on configuration + if (isset($info['http_code']) && !empty($info['http_code'])) { + $statusCode = (int)$info['http_code']; + HttpClientIntegrationHelper::setClientError($span, $statusCode); + } + + addSpanDataTagFromCurlInfo($span, $info, 'network.client.ip', 'local_ip'); + addSpanDataTagFromCurlInfo($span, $info, 'network.client.port', 'local_port'); + + addSpanDataTagFromCurlInfo($span, $info, 'network.destination.ip', 'primary_ip'); + addSpanDataTagFromCurlInfo($span, $info, 'network.destination.port', 'primary_port'); + + addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_read', 'size_download'); + addSpanDataTagFromCurlInfo($span, $info, 'network.bytes_written', 'size_upload'); + + // Add the rest to a 'curl.' object + foreach ($info as $key => $val) { + // Datadog doesn't support arrays in tags + if (\is_scalar($val) && $val !== '') { + // Datadog sets durations in nanoseconds - convert from seconds + if (\substr_compare($key, '_time', -5) === 0) { + $val *= 1000000000; + } + $span->meta["curl.{$key}"] = $val; + } + } + + return $info; } } diff --git a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php index 5c3aabbad82..84f63861cee 100644 --- a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php +++ b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php @@ -52,36 +52,18 @@ function (SpanData $span, $args, $retval) use ($integration) { /** @var \GuzzleHttp\Message\ResponseInterface $response */ $statusCode = $response->getStatusCode(); $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; - - // Mark as error if status code matches configuration - if (self::isClientError($statusCode)) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); - } + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); } elseif (\is_a($response, 'Psr\Http\Message\ResponseInterface')) { /** @var \Psr\Http\Message\ResponseInterface $response */ $statusCode = $response->getStatusCode(); $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; - - // Mark as error if status code matches configuration - if (self::isClientError($statusCode)) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); - } + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); } elseif (\is_a($response, 'GuzzleHttp\Promise\PromiseInterface')) { /** @var \GuzzleHttp\Promise\PromiseInterface $response */ $response->then(function (\Psr\Http\Message\ResponseInterface $response) use ($span) { $statusCode = $response->getStatusCode(); $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; - - // Mark as error if status code matches configuration - if (self::isClientError($statusCode)) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); - } + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); }); } } @@ -110,13 +92,7 @@ function (SpanData $span, $args, $retval) use ($integration) { $response->then(function (\Psr\Http\Message\ResponseInterface $response) use ($span) { $statusCode = $response->getStatusCode(); $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; - - // Mark as error if status code matches configuration - if (self::isClientError($statusCode)) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $response->getReasonPhrase(); - } + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); }); } } @@ -157,44 +133,4 @@ public function addRequestInfo(SpanData $span, $request) } } } - - /** - * Determines if a given status code should be considered an error - * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. - * - * @param int $statusCode The HTTP status code to check - * @return bool Whether the status code should be considered an error - */ - private static function isClientError($statusCode) { - // Get configured status codes from environment - $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); - - if (!empty($errorStatusCodes)) { - // Custom configuration exists, use it - $codesList = explode(',', $errorStatusCodes); - - foreach ($codesList as $item) { - $item = trim($item); - - if (strpos($item, '-') !== false) { - // Range ("400-499") - list($start, $end) = explode('-', $item); - if ($statusCode >= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } - } - } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior - return ($statusCode >= 400); - } - } } \ No newline at end of file diff --git a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php index 857392d67b0..5931d37ca18 100644 --- a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php +++ b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php @@ -10,4 +10,74 @@ class HttpClientIntegrationHelper Tag::NETWORK_DESTINATION_NAME, Tag::TARGET_HOST, ]; -} + + /** + * Determines if a given status code should be considered an error + * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. + * + * @param int $statusCode The HTTP status code to check + * @return bool Whether the status code should be considered an error + */ + public static function isClientError($statusCode) { + // Get configured status codes from environment + $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); + + if (!empty($errorStatusCodes)) { + // Custom configuration exists, use it + $codesList = explode(',', $errorStatusCodes); + + foreach ($codesList as $item) { + $item = trim($item); + + if (strpos($item, '-') !== false) { + // Range ("400-499") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior + return ($statusCode >= 400); + } + } + + /** + * Sets error tags on a span for HTTP client errors, if the status code + * matches the configuration in DD_TRACE_HTTP_CLIENT_ERROR_STATUSES. + * + * @param \DDTrace\SpanData $span The span to mark as an error + * @param int $statusCode The HTTP status code to check + * @param string|null $reasonPhrase Optional reason phrase to include in the error message + * @return bool Whether the span was marked as an error + */ + public static function setClientError($span, $statusCode, $reasonPhrase = null) { + // Only set error if it's not already set + if (isset($span->meta[Tag::ERROR])) { + return false; + } + + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + + if ($reasonPhrase) { + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: $reasonPhrase"; + } else { + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode Error"; + } + + return true; + } + + return false; + } +} \ No newline at end of file diff --git a/src/DDTrace/Integrations/Psr18/Psr18Integration.php b/src/DDTrace/Integrations/Psr18/Psr18Integration.php index c62cfce1434..57b9df5b9b1 100644 --- a/src/DDTrace/Integrations/Psr18/Psr18Integration.php +++ b/src/DDTrace/Integrations/Psr18/Psr18Integration.php @@ -38,12 +38,7 @@ function (SpanData $span, $args, $retval) use ($integration) { /** @var \Psr\Http\Message\ResponseInterface $retval */ $statusCode = $retval->getStatusCode(); $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; - - // Mark as error if status code matches configuration and no error is already set - if (self::isClientError($statusCode) && !isset($span->meta[Tag::ERROR])) { - $span->meta[Tag::ERROR] = 1; - $span->meta[Tag::ERROR_TYPE] = 'http_error'; - $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: " . $retval->getReasonPhrase(); + HttpClientIntegrationHelper::setClientError($span, $statusCode, $retval->getReasonPhrase()); } } } @@ -68,44 +63,4 @@ public function addRequestInfo(SpanData $span, $request) $span->meta[Tag::HTTP_URL] = \DDTrace\Util\Normalizer::urlSanitize($url); } } - - /** - * Determines if a given status code should be considered an error - * based on the DD_TRACE_HTTP_CLIENT_ERROR_STATUSES configuration. - * - * @param int $statusCode The HTTP status code to check - * @return bool Whether the status code should be considered an error - */ - private static function isClientError($statusCode) { - // Get configured status codes from environment - $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); - - if (!empty($errorStatusCodes)) { - // Custom configuration exists, use it - $codesList = explode(',', $errorStatusCodes); - - foreach ($codesList as $item) { - $item = trim($item); - - if (strpos($item, '-') !== false) { - // Range ("400-499") - list($start, $end) = explode('-', $item); - if ($statusCode >= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } - } - } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior - return ($statusCode >= 400); - } - } } \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt new file mode 100644 index 00000000000..4352844a91b --- /dev/null +++ b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt @@ -0,0 +1,178 @@ +--TEST-- +HTTP client status code error configuration and helper functions +--ENV-- +DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=403,408-417 +--FILE-- += (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; + } + } + } + + // The status code isn't in any defined error range + return false; + } else { + // Default behavior (500-599 are errors) + return ($statusCode >= 500 && $statusCode <= 599); + } + } + + public static function setClientError($span, $statusCode, $reasonPhrase = null) { + // Only set error if it's not already set + if (isset($span->meta[Tag::ERROR])) { + return false; + } + + if (self::isClientError($statusCode)) { + $span->meta[Tag::ERROR] = 1; + $span->meta[Tag::ERROR_TYPE] = 'http_error'; + + if ($reasonPhrase) { + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode: $reasonPhrase"; + } else { + $span->meta[Tag::ERROR_MSG] = "HTTP $statusCode Error"; + } + + return true; + } + + return false; + } +} + +// Test isClientError directly +echo "-- Testing isClientError with custom configuration --\n"; +echo "Status 200: " . (HttpClientIntegrationHelper::isClientError(200) ? "Error" : "Not error") . "\n"; +echo "Status 404: " . (HttpClientIntegrationHelper::isClientError(404) ? "Error" : "Not error") . "\n"; +echo "Status 403: " . (HttpClientIntegrationHelper::isClientError(403) ? "Error" : "Not error") . "\n"; +echo "Status 408: " . (HttpClientIntegrationHelper::isClientError(408) ? "Error" : "Not error") . "\n"; +echo "Status 412: " . (HttpClientIntegrationHelper::isClientError(412) ? "Error" : "Not error") . "\n"; +echo "Status 417: " . (HttpClientIntegrationHelper::isClientError(417) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (HttpClientIntegrationHelper::isClientError(500) ? "Error" : "Not error") . "\n"; +echo "Status 503: " . (HttpClientIntegrationHelper::isClientError(503) ? "Error" : "Not error") . "\n"; +echo "Status 600: " . (HttpClientIntegrationHelper::isClientError(600) ? "Error" : "Not error") . "\n"; + +// Test setClientError method +echo "\n-- Testing setClientError with custom configuration --\n"; +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 403); +echo "Status 403: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Error type: " . ($span->meta[Tag::ERROR_TYPE] ?? 'none') . "\n"; +echo "Error message: " . ($span->meta[Tag::ERROR_MSG] ?? 'none') . "\n"; + +// Test with custom reason phrase +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 412, 'Precondition Failed'); +echo "\nStatus 412 with reason: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Error message: " . ($span->meta[Tag::ERROR_MSG] ?? 'none') . "\n"; + +// Test non-error status +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 200); +echo "\nStatus 200: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Has error tag: " . (isset($span->meta[Tag::ERROR]) ? 'Yes' : 'No') . "\n"; + +// Test already marked error +$span = new SpanData(); +$span->meta[Tag::ERROR] = 1; +$result = HttpClientIntegrationHelper::setClientError($span, 403); +echo "\nPre-marked span with status 403: " . ($result ? "Marked again" : "Not marked again") . "\n"; + +// Now test default behavior without config +putenv('DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='); + +echo "\n-- Testing with default configuration (500-599) --\n"; +echo "Status 200: " . (HttpClientIntegrationHelper::isClientError(200) ? "Error" : "Not error") . "\n"; +echo "Status 404: " . (HttpClientIntegrationHelper::isClientError(404) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (HttpClientIntegrationHelper::isClientError(500) ? "Error" : "Not error") . "\n"; +echo "Status 599: " . (HttpClientIntegrationHelper::isClientError(599) ? "Error" : "Not error") . "\n"; +echo "Status 600: " . (HttpClientIntegrationHelper::isClientError(600) ? "Error" : "Not error") . "\n"; + +// Test with default behavior +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 500); +echo "\nStatus 500 with default config: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Error message: " . ($span->meta[Tag::ERROR_MSG] ?? 'none') . "\n"; + +// Test with default behavior - non-error status code +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 404); +echo "\nStatus 404 with default config: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Has error tag: " . (isset($span->meta[Tag::ERROR]) ? 'Yes' : 'No') . "\n"; +?> +--EXPECT-- +-- Testing isClientError with custom configuration -- +Status 200: Not error +Status 404: Not error +Status 403: Error +Status 408: Error +Status 412: Error +Status 417: Error +Status 500: Not error +Status 503: Not error +Status 600: Not error + +-- Testing setClientError with custom configuration -- +Status 403: Marked as error +Error type: http_error +Error message: HTTP 403 Error + +Status 412 with reason: Marked as error +Error message: HTTP 412: Precondition Failed + +Status 200: Not marked +Has error tag: No + +Pre-marked span with status 403: Not marked again + +-- Testing with default configuration (500-599) -- +Status 200: Not error +Status 404: Not error +Status 500: Error +Status 599: Error +Status 600: Not error + +Status 500 with default config: Marked as error +Error message: HTTP 500 Error + +Status 404 with default config: Not marked +Has error tag: No \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt b/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt deleted file mode 100644 index 019b85cb553..00000000000 --- a/tests/ext/error_status_configuration/http_error_status_is_client_error.phpt +++ /dev/null @@ -1,74 +0,0 @@ ---TEST-- -HTTP client status code error configuration parsing ---ENV-- -DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=403,408-417 ---FILE-- -= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } - } - } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior - return ($statusCode >= 400); - } -} - -// Test various status codes directly against the function -echo "-- With custom configuration --\n"; -echo "Status 200: " . (isClientError(200) ? "Error" : "Not error") . "\n"; -echo "Status 404: " . (isClientError(404) ? "Error" : "Not error") . "\n"; -echo "Status 403: " . (isClientError(403) ? "Error" : "Not error") . "\n"; -echo "Status 408: " . (isClientError(408) ? "Error" : "Not error") . "\n"; -echo "Status 412: " . (isClientError(412) ? "Error" : "Not error") . "\n"; -echo "Status 417: " . (isClientError(417) ? "Error" : "Not error") . "\n"; -echo "Status 500: " . (isClientError(500) ? "Error" : "Not error") . "\n"; -echo "Status 503: " . (isClientError(503) ? "Error" : "Not error") . "\n"; - -// Now test default behavior without config -putenv('DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='); - -echo "\n-- Without custom configuration --\n"; -echo "Status 200: " . (isClientError(200) ? "Error" : "Not error") . "\n"; -echo "Status 404: " . (isClientError(404) ? "Error" : "Not error") . "\n"; -echo "Status 500: " . (isClientError(500) ? "Error" : "Not error") . "\n"; -?> ---EXPECT-- --- With custom configuration -- -Status 200: Not error -Status 404: Not error -Status 403: Error -Status 408: Error -Status 412: Error -Status 417: Error -Status 500: Not error -Status 503: Not error - --- Without custom configuration -- -Status 200: Not error -Status 404: Error -Status 500: Error \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_error_status_parsing.phpt b/tests/ext/error_status_configuration/http_error_status_parsing.phpt new file mode 100644 index 00000000000..464ce238bb2 --- /dev/null +++ b/tests/ext/error_status_configuration/http_error_status_parsing.phpt @@ -0,0 +1,110 @@ +--TEST-- +HTTP server error status configuration parsing test +--ENV-- +DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,429-451,503 +--FILE-- += (int)$start && $code <= (int)$end) { + $isError = true; + break; + } + } else { + // Single status code + $code_val = (int)$item; + if ($code == $code_val) { + $isError = true; + break; + } + } + } + + return $isError; +} + +// Function to mimic the C implementation for getting error message +function getErrorMessage($code) { + return "HTTP $code Error"; +} + +// Test various status codes +echo "-- With custom configuration --\n"; +echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; +echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; +if (isStatusError(418)) { + echo "Error message: " . getErrorMessage(418) . "\n"; +} +echo "Status 429: " . (isStatusError(429) ? "Error" : "Not error") . "\n"; +if (isStatusError(429)) { + echo "Error message: " . getErrorMessage(429) . "\n"; +} +echo "Status 451: " . (isStatusError(451) ? "Error" : "Not error") . "\n"; +if (isStatusError(451)) { + echo "Error message: " . getErrorMessage(451) . "\n"; +} +echo "Status 490: " . (isStatusError(490) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; +echo "Status 503: " . (isStatusError(503) ? "Error" : "Not error") . "\n"; +if (isStatusError(503)) { + echo "Error message: " . getErrorMessage(503) . "\n"; +} +echo "Status 600: " . (isStatusError(600) ? "Error" : "Not error") . "\n"; + +// Now test default behavior without custom config +putenv('DD_TRACE_HTTP_SERVER_ERROR_STATUSES='); + +echo "\n-- Without custom configuration (using default 500-599) --\n"; +echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; +echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; +echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; +if (isStatusError(500)) { + echo "Error message: " . getErrorMessage(500) . "\n"; +} +echo "Status 599: " . (isStatusError(599) ? "Error" : "Not error") . "\n"; +if (isStatusError(599)) { + echo "Error message: " . getErrorMessage(599) . "\n"; +} +echo "Status 600: " . (isStatusError(600) ? "Error" : "Not error") . "\n"; +?> +--EXPECT-- +-- With custom configuration -- +Status 200: Not error +Status 418: Error +Error message: HTTP 418 Error +Status 429: Error +Error message: HTTP 429 Error +Status 451: Error +Error message: HTTP 451 Error +Status 490: Not error +Status 500: Not error +Status 503: Error +Error message: HTTP 503 Error +Status 600: Not error + +-- Without custom configuration (using default 500-599) -- +Status 200: Not error +Status 418: Not error +Status 500: Error +Error message: HTTP 500 Error +Status 599: Error +Error message: HTTP 599 Error +Status 600: Not error \ No newline at end of file diff --git a/tests/ext/error_status_configuration/http_server_error_status_configured.phpt b/tests/ext/error_status_configuration/http_server_error_status_configured.phpt deleted file mode 100644 index c550c2f1c0e..00000000000 --- a/tests/ext/error_status_configuration/http_server_error_status_configured.phpt +++ /dev/null @@ -1,75 +0,0 @@ ---TEST-- -HTTP status code configuration parsing test ---ENV-- -DD_TRACE_HTTP_SERVER_ERROR_STATUSES=418,429-451,503 ---FILE-- -= (int)$start && $code <= (int)$end) { - $isError = true; - break; - } - } else { - // It's a single code - if ($code == (int)$item) { - $isError = true; - break; - } - } - } - } else { - // No custom configuration - use default behavior (500+ are errors) - $isError = ($code >= 500); - } - - return $isError; -} - -// Test various status codes -echo "-- With custom configuration --\n"; -echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; -echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; -echo "Status 429: " . (isStatusError(429) ? "Error" : "Not error") . "\n"; -echo "Status 451: " . (isStatusError(451) ? "Error" : "Not error") . "\n"; -echo "Status 490: " . (isStatusError(490) ? "Error" : "Not error") . "\n"; -echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; -echo "Status 503: " . (isStatusError(503) ? "Error" : "Not error") . "\n"; - -// Now test default behavior without config -putenv('DD_TRACE_HTTP_SERVER_ERROR_STATUSES='); - -echo "\n-- Without custom configuration --\n"; -echo "Status 200: " . (isStatusError(200) ? "Error" : "Not error") . "\n"; -echo "Status 418: " . (isStatusError(418) ? "Error" : "Not error") . "\n"; -echo "Status 500: " . (isStatusError(500) ? "Error" : "Not error") . "\n"; -?> ---EXPECT-- --- With custom configuration -- -Status 200: Not error -Status 418: Error -Status 429: Error -Status 451: Error -Status 490: Not error -Status 500: Not error -Status 503: Error - --- Without custom configuration -- -Status 200: Not error -Status 418: Not error -Status 500: Error \ No newline at end of file From 25e9a868c460968b0fc1b648c6068e23921a83c2 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 12:09:50 -0400 Subject: [PATCH 11/16] space --- src/DDTrace/Integrations/Curl/CurlIntegration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DDTrace/Integrations/Curl/CurlIntegration.php b/src/DDTrace/Integrations/Curl/CurlIntegration.php index 205d8734ccb..becf791957d 100644 --- a/src/DDTrace/Integrations/Curl/CurlIntegration.php +++ b/src/DDTrace/Integrations/Curl/CurlIntegration.php @@ -251,7 +251,7 @@ public function setup_curl_span($span) { $span->meta[Tag::SPAN_KIND] = Tag::SPAN_KIND_VALUE_CLIENT; } - public static function set_curl_attributes($span, $info) { + public static function set_curl_attributes($span, $info) { $sanitizedUrl = \DDTrace\Util\Normalizer::urlSanitize($info['url']); $normalizedPath = \DDTrace\Util\Normalizer::uriNormalizeOutgoingPath($info['url']); $host = Urls::hostname($sanitizedUrl); From 531a821114d6039e5c741c0d53764d3dcf67495d Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 12:12:27 -0400 Subject: [PATCH 12/16] spacing --- src/DDTrace/Integrations/Curl/CurlIntegration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DDTrace/Integrations/Curl/CurlIntegration.php b/src/DDTrace/Integrations/Curl/CurlIntegration.php index becf791957d..0ef02e69905 100644 --- a/src/DDTrace/Integrations/Curl/CurlIntegration.php +++ b/src/DDTrace/Integrations/Curl/CurlIntegration.php @@ -303,5 +303,5 @@ public static function set_curl_attributes($span, $info) { } return $info; - } + } } From 6460080e01d4d7862a2caef752d5c8749a359b99 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 12:19:01 -0400 Subject: [PATCH 13/16] formatting fixes --- src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php | 2 +- src/DDTrace/Integrations/HttpClientIntegrationHelper.php | 2 +- src/DDTrace/Integrations/Psr18/Psr18Integration.php | 7 ++++++- .../http_error_status_client_configuration.phpt | 2 +- .../http_error_status_parsing.phpt | 2 +- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php index 84f63861cee..2b3589e9b23 100644 --- a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php +++ b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php @@ -133,4 +133,4 @@ public function addRequestInfo(SpanData $span, $request) } } } -} \ No newline at end of file +} diff --git a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php index 5931d37ca18..1d95c889f78 100644 --- a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php +++ b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php @@ -80,4 +80,4 @@ public static function setClientError($span, $statusCode, $reasonPhrase = null) return false; } -} \ No newline at end of file +} diff --git a/src/DDTrace/Integrations/Psr18/Psr18Integration.php b/src/DDTrace/Integrations/Psr18/Psr18Integration.php index 57b9df5b9b1..d6fa8576056 100644 --- a/src/DDTrace/Integrations/Psr18/Psr18Integration.php +++ b/src/DDTrace/Integrations/Psr18/Psr18Integration.php @@ -1,4 +1,5 @@ getUri(); $host = Urls::hostname($url); + if ($host) { $span->meta[Tag::NETWORK_DESTINATION_NAME] = $host; } + if (\dd_trace_env_config("DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN")) { $span->service = Urls::hostnameForTag($url); } + $span->meta[Tag::HTTP_METHOD] = $request->getMethod(); + if (!array_key_exists(Tag::HTTP_URL, $span->meta)) { $span->meta[Tag::HTTP_URL] = \DDTrace\Util\Normalizer::urlSanitize($url); } } -} \ No newline at end of file +} diff --git a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt index 4352844a91b..a270d5e7b6d 100644 --- a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt +++ b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt @@ -175,4 +175,4 @@ Status 500 with default config: Marked as error Error message: HTTP 500 Error Status 404 with default config: Not marked -Has error tag: No \ No newline at end of file +Has error tag: No diff --git a/tests/ext/error_status_configuration/http_error_status_parsing.phpt b/tests/ext/error_status_configuration/http_error_status_parsing.phpt index 464ce238bb2..9a4d8239859 100644 --- a/tests/ext/error_status_configuration/http_error_status_parsing.phpt +++ b/tests/ext/error_status_configuration/http_error_status_parsing.phpt @@ -107,4 +107,4 @@ Status 500: Error Error message: HTTP 500 Error Status 599: Error Error message: HTTP 599 Error -Status 600: Not error \ No newline at end of file +Status 600: Not error From 2696438980557420c004e30c79df3e5947ed5933 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 13:25:50 -0400 Subject: [PATCH 14/16] remove the client default --- .../HttpClientIntegrationHelper.php | 43 ++++++----- ...ttp_error_status_client_configuration.phpt | 74 +++++++++---------- 2 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php index 1d95c889f78..c2b6ff6daff 100644 --- a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php +++ b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php @@ -22,33 +22,32 @@ public static function isClientError($statusCode) { // Get configured status codes from environment $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); - if (!empty($errorStatusCodes)) { - // Custom configuration exists, use it - $codesList = explode(',', $errorStatusCodes); + if (empty($errorStatusCodes)) { + return false; + } + + // Custom configuration exists, use it + $codesList = explode(',', $errorStatusCodes); - foreach ($codesList as $item) { - $item = trim($item); + foreach ($codesList as $item) { + $item = trim($item); - if (strpos($item, '-') !== false) { - // Range ("400-499") - list($start, $end) = explode('-', $item); - if ($statusCode >= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } + if (strpos($item, '-') !== false) { + // Range ("400-499") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code ("404") + if ($statusCode == (int)$item) { + return true; } } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior - return ($statusCode >= 400); } + + // The status code isn't in any defined error range + return false; } /** diff --git a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt index a270d5e7b6d..2f335e87bc7 100644 --- a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt +++ b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt @@ -27,33 +27,33 @@ class HttpClientIntegrationHelper // Get configured status codes from environment $errorStatusCodes = getenv("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); - if (!empty($errorStatusCodes)) { - // Custom configuration exists, use it - $codesList = explode(',', $errorStatusCodes); - - foreach ($codesList as $item) { - $item = trim($item); - - if (strpos($item, '-') !== false) { - // Range ("400-499") - list($start, $end) = explode('-', $item); - if ($statusCode >= (int)$start && $statusCode <= (int)$end) { - return true; - } - } else { - // Single code ("404") - if ($statusCode == (int)$item) { - return true; - } + // Return early if no configuration exists + if (empty($errorStatusCodes)) { + return false; + } + + // Parse and check the status codes + $codesList = explode(',', $errorStatusCodes); + foreach ($codesList as $item) { + $item = trim($item); + if (empty($item)) continue; + + if (strpos($item, '-') !== false) { + // Range ("500-599") + list($start, $end) = explode('-', $item); + if ($statusCode >= (int)$start && $statusCode <= (int)$end) { + return true; + } + } else { + // Single code + if ($statusCode == (int)$item) { + return true; } } - - // The status code isn't in any defined error range - return false; - } else { - // Default behavior (500-599 are errors) - return ($statusCode >= 500 && $statusCode <= 599); } + + // No matching error status codes found + return false; } public static function setClientError($span, $statusCode, $reasonPhrase = null) { @@ -117,26 +117,24 @@ $span->meta[Tag::ERROR] = 1; $result = HttpClientIntegrationHelper::setClientError($span, 403); echo "\nPre-marked span with status 403: " . ($result ? "Marked again" : "Not marked again") . "\n"; -// Now test default behavior without config +// Test with empty configuration putenv('DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='); -echo "\n-- Testing with default configuration (500-599) --\n"; +echo "\n-- Testing with empty configuration --\n"; echo "Status 200: " . (HttpClientIntegrationHelper::isClientError(200) ? "Error" : "Not error") . "\n"; echo "Status 404: " . (HttpClientIntegrationHelper::isClientError(404) ? "Error" : "Not error") . "\n"; echo "Status 500: " . (HttpClientIntegrationHelper::isClientError(500) ? "Error" : "Not error") . "\n"; echo "Status 599: " . (HttpClientIntegrationHelper::isClientError(599) ? "Error" : "Not error") . "\n"; echo "Status 600: " . (HttpClientIntegrationHelper::isClientError(600) ? "Error" : "Not error") . "\n"; -// Test with default behavior $span = new SpanData(); $result = HttpClientIntegrationHelper::setClientError($span, 500); -echo "\nStatus 500 with default config: " . ($result ? "Marked as error" : "Not marked") . "\n"; -echo "Error message: " . ($span->meta[Tag::ERROR_MSG] ?? 'none') . "\n"; +echo "\nStatus 500 with empty config: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "Has error tag: " . (isset($span->meta[Tag::ERROR]) ? 'Yes' : 'No') . "\n"; -// Test with default behavior - non-error status code $span = new SpanData(); $result = HttpClientIntegrationHelper::setClientError($span, 404); -echo "\nStatus 404 with default config: " . ($result ? "Marked as error" : "Not marked") . "\n"; +echo "\nStatus 404 with empty config: " . ($result ? "Marked as error" : "Not marked") . "\n"; echo "Has error tag: " . (isset($span->meta[Tag::ERROR]) ? 'Yes' : 'No') . "\n"; ?> --EXPECT-- @@ -164,15 +162,15 @@ Has error tag: No Pre-marked span with status 403: Not marked again --- Testing with default configuration (500-599) -- +-- Testing with empty configuration -- Status 200: Not error Status 404: Not error -Status 500: Error -Status 599: Error +Status 500: Not error +Status 599: Not error Status 600: Not error -Status 500 with default config: Marked as error -Error message: HTTP 500 Error - -Status 404 with default config: Not marked +Status 500 with empty config: Not marked Has error tag: No + +Status 404 with empty config: Not marked +Has error tag: No \ No newline at end of file From f72016d60e8c3f8806eb68315d4aab169272bb18 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Tue, 29 Apr 2025 13:27:52 -0400 Subject: [PATCH 15/16] formatting and comment --- src/DDTrace/Integrations/HttpClientIntegrationHelper.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php index c2b6ff6daff..fc0d01cc8df 100644 --- a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php +++ b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php @@ -22,8 +22,9 @@ public static function isClientError($statusCode) { // Get configured status codes from environment $errorStatusCodes = \dd_trace_env_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES"); + // Specifically set configuration to empty if (empty($errorStatusCodes)) { - return false; + return false; } // Custom configuration exists, use it From 69f19dba8bd5f9b0fe1729541e774d63a1b63fa7 Mon Sep 17 00:00:00 2001 From: Scott Shields Date: Wed, 30 Apr 2025 16:02:13 -0400 Subject: [PATCH 16/16] update error message string --- ext/serializer.c | 2 +- .../http_error_status_client_configuration.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/serializer.c b/ext/serializer.c index e72f40cb534..dffc321d761 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1190,7 +1190,7 @@ static void dd_set_entrypoint_root_span_props_end(zend_array *meta, int status, // Add error message and flag zval error_msg_zv = {0}, *msg_value; - if ((msg_value = zend_hash_str_add(meta, ZEND_STRL("error.msg"), &error_msg_zv))) { + if ((msg_value = zend_hash_str_add(meta, ZEND_STRL("error.message"), &error_msg_zv))) { ZVAL_STR(msg_value, zend_strpprintf(0, "HTTP %d Error", status)); } } diff --git a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt index 2f335e87bc7..126c4007400 100644 --- a/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt +++ b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt @@ -12,7 +12,7 @@ class SpanData { class Tag { const ERROR = 'error'; const ERROR_TYPE = 'error.type'; - const ERROR_MSG = 'error.msg'; + const ERROR_MSG = 'error.message'; } // Include the HttpClientIntegrationHelper class