Skip to content

[WIP] feat: add http status error configuration #3223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions ext/configuration.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "json/json.h"
#include "sidecar.h"
#include <components/log/log.h>
#include <ctype.h>
#include <zai_string/string.h>
#include "sidecar.h"

Expand Down Expand Up @@ -156,6 +157,51 @@ 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); \
Expand Down
6 changes: 5 additions & 1 deletion ext/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
126 changes: 126 additions & 0 deletions ext/serializer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,95 @@ 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();
size_t cfg_sz = cfg ? zend_hash_num_elements(cfg) : 0;

if (cfg_sz > 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);
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;
Expand Down Expand Up @@ -1834,6 +1923,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")));
Expand Down
20 changes: 20 additions & 0 deletions tests/ext/serialization/http_client_error_status_configured.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
Client span with configured error status code (basic functionality)
--ENV--
DD_TRACE_HTTP_CLIENT_ERROR_STATUSES=400-499
--FILE--
<?php
$span = \DDTrace\start_span();
$span->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"
19 changes: 19 additions & 0 deletions tests/ext/serialization/http_server_error_status_configured.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
Server span with configured error status code (basic functionality)
--ENV--
DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599
--FILE--
<?php
$span = \DDTrace\start_span();
$span->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"
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
Explicit error flag takes precedence over status code configuration
--ENV--
DD_TRACE_HTTP_SERVER_ERROR_STATUSES=500-599
--FILE--
<?php
// Should NOT be error due to explicit flag = 0, even though status code is 500
$span1 = \DDTrace\start_span();
$span1->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)
51 changes: 51 additions & 0 deletions tests/ext/serialization/http_server_vs_client_status.phpt
Original file line number Diff line number Diff line change
@@ -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--
<?php
// 1. Server span with 404 - should NOT be error
echo "=== SERVER SPAN WITH 404 ===\n";
$span_server = \DDTrace\start_span();
$span_server->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
41 changes: 41 additions & 0 deletions tests/ext/serialization/http_status_mixed_range_configuration.phpt
Original file line number Diff line number Diff line change
@@ -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--
<?php
// Create spans with different status codes
$status_codes = [400, 403, 404, 419, 500, 503, 504];

foreach ($status_codes as $code) {
$span = \DDTrace\start_span();
$span->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
Loading