diff --git a/ext/configuration.h b/ext/configuration.h index 09d6ef0050b..dab4d686230 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -247,7 +247,9 @@ 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, "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 72cbb9ee14b..dffc321d761 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1152,10 +1152,48 @@ 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; + + // Get server error configuration + zend_array *cfg = get_DD_TRACE_HTTP_SERVER_ERROR_STATUSES(); + + // 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 (is_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("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.message"), &error_msg_zv))) { + ZVAL_STR(msg_value, zend_strpprintf(0, "HTTP %d Error", status)); + } + } } } } @@ -1476,7 +1514,6 @@ void transfer_data(zend_array *source, zend_array *destination, const char *key, } } - 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; diff --git a/src/DDTrace/Integrations/Curl/CurlIntegration.php b/src/DDTrace/Integrations/Curl/CurlIntegration.php index 61304f8d294..0ef02e69905 100644 --- a/src/DDTrace/Integrations/Curl/CurlIntegration.php +++ b/src/DDTrace/Integrations/Curl/CurlIntegration.php @@ -275,6 +275,12 @@ public static function set_curl_attributes($span, $info) { 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'); diff --git a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php index 2bf782333a1..2b3589e9b23 100644 --- a/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php +++ b/src/DDTrace/Integrations/Guzzle/GuzzleIntegration.php @@ -50,14 +50,20 @@ 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; + HttpClientIntegrationHelper::setClientError($span, $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; + 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) { - $span->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode(); + $statusCode = $response->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); }); } } @@ -84,7 +90,9 @@ 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; + HttpClientIntegrationHelper::setClientError($span, $statusCode, $response->getReasonPhrase()); }); } } diff --git a/src/DDTrace/Integrations/HttpClientIntegrationHelper.php b/src/DDTrace/Integrations/HttpClientIntegrationHelper.php index 857392d67b0..fc0d01cc8df 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"); + + // Specifically set configuration to empty + if (empty($errorStatusCodes)) { + return false; + } + + // 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; + } + + /** + * 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; + } } diff --git a/src/DDTrace/Integrations/Psr18/Psr18Integration.php b/src/DDTrace/Integrations/Psr18/Psr18Integration.php index 2287036bbbc..d6fa8576056 100644 --- a/src/DDTrace/Integrations/Psr18/Psr18Integration.php +++ b/src/DDTrace/Integrations/Psr18/Psr18Integration.php @@ -37,7 +37,10 @@ function (SpanData $span, $args, $retval) use ($integration) { if (isset($retval)) { /** @var \Psr\Http\Message\ResponseInterface $retval */ - $span->meta[Tag::HTTP_STATUS_CODE] = $retval->getStatusCode(); + $statusCode = $retval->getStatusCode(); + $span->meta[Tag::HTTP_STATUS_CODE] = $statusCode; + HttpClientIntegrationHelper::setClientError($span, $statusCode, $retval->getReasonPhrase()); + } } } ); @@ -58,6 +61,7 @@ public function addRequestInfo(SpanData $span, $request) 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)) { 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..126c4007400 --- /dev/null +++ b/tests/ext/error_status_configuration/http_error_status_client_configuration.phpt @@ -0,0 +1,176 @@ +--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 + if ($statusCode == (int)$item) { + return true; + } + } + } + + // No matching error status codes found + return false; + } + + 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"; + +// Test with empty configuration +putenv('DD_TRACE_HTTP_CLIENT_ERROR_STATUSES='); + +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"; + +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 500); +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"; + +$span = new SpanData(); +$result = HttpClientIntegrationHelper::setClientError($span, 404); +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-- +-- 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 empty configuration -- +Status 200: Not error +Status 404: Not error +Status 500: Not error +Status 599: Not error +Status 600: Not error + +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 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..9a4d8239859 --- /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