diff --git a/src/frontend/src/instance/otlp/trace_types.rs b/src/frontend/src/instance/otlp/trace_types.rs index 0295d12e685..6f37f14e34a 100644 --- a/src/frontend/src/instance/otlp/trace_types.rs +++ b/src/frontend/src/instance/otlp/trace_types.rs @@ -519,31 +519,34 @@ mod tests { ); } - /// The PHP instrumentation case: an empty string must be distinguishable - /// from any other unparsable value in the reported diagnostics. + /// The PHP instrumentation case: `getHeaderLine('Content-Length')` sends "" + /// when the header is absent. The span must be kept with a null attribute. #[test] - fn test_prepare_trace_column_rewrites_reports_empty_string_value() { - let rows = vec![Row { - values: vec![Value { - value_data: Some(ValueData::StringValue(String::new())), + fn test_prepare_trace_column_rewrites_nulls_empty_string_value() { + let mut rows = Rows { + schema: vec![ColumnSchema { + column_name: "span_attributes.http.response.body.size".to_string(), + datatype: ColumnDataType::String as i32, + ..Default::default() }], - }]; + rows: vec![Row { + values: vec![Value { + value_data: Some(ValueData::StringValue(String::new())), + }], + }], + }; let pending_rewrites = vec![PendingTraceColumnRewrite { col_idx: 0, target_type: ColumnDataType::Int64, column_name: "span_attributes.http.response.body.size".to_string(), }]; - let err = prepare_trace_column_rewrites(&rows, pending_rewrites, "opentelemetry_traces") - .unwrap_err(); - assert!( - err.error.to_string().contains( - "'span_attributes.http.response.body.size' in table 'opentelemetry_traces' \ - from String(\"\") to Int64" - ), - "unexpected error message: {}", - err.error - ); + prepare_trace_column_rewrites(&rows.rows, pending_rewrites, "opentelemetry_traces") + .unwrap() + .apply(&mut rows); + + assert_eq!(rows.schema[0].datatype, ColumnDataType::Int64 as i32); + assert_eq!(rows.rows[0].values[0].value_data, None); } #[test] diff --git a/src/servers/src/otlp/coerce.rs b/src/servers/src/otlp/coerce.rs index 2e7dd36ed7a..2f8d25247cf 100644 --- a/src/servers/src/otlp/coerce.rs +++ b/src/servers/src/otlp/coerce.rs @@ -30,6 +30,9 @@ pub enum TraceCoerceError { // - String to Int64 // - String to Float64 // - String to Boolean +// The empty string and "-" coerce to null instead of failing: instrumentations +// fill numeric attributes from absent headers this way (PSR-7 `getHeaderLine` +// returns ""), and rejecting the value would reject the whole span. // // Lossless signed-to-unsigned integer casts. These let the built-in data // models move from unsigned to signed integers while existing unsigned tables @@ -86,6 +89,13 @@ pub fn coerce_value_data( return Ok(None); }; + if let ValueData::StringValue(s) = v + && matches!(s.as_str(), "" | "-") + && is_supported_trace_coercion(request_type, target) + { + return Ok(None); + } + let Some(value) = coerce_non_null_value(target, request_type, v) else { return Err(TraceCoerceError::Unsupported); }; @@ -273,6 +283,24 @@ mod tests { assert_eq!(result, Err(TraceCoerceError::Unsupported)); } + #[test] + fn test_coerce_absent_value_strings_to_null() { + for s in ["", "-"] { + for target in [ + ColumnDataType::Int64, + ColumnDataType::Float64, + ColumnDataType::Boolean, + ] { + let result = coerce_value_data( + &Some(ValueData::StringValue(s.to_string())), + target, + ColumnDataType::String, + ); + assert_eq!(result, Ok(None), "{s:?} to {target:?}"); + } + } + } + #[test] fn test_coerce_float64_to_int64_not_supported() { let result = coerce_value_data(