mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-10-13 15:32:36 +00:00
fix(otlp): coerce empty and "-" trace attribute strings to null (#9461)
* fix(otlp): coerce empty and "-" trace attribute strings to null Instrumentations fill numeric attributes from absent headers with an empty string (PSR-7 getHeaderLine returns ""). Coercing that into an existing numeric column failed and rejected the whole span. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * test: trim redundant row from empty-string rewrite test Signed-off-by: Dennis Zhuang <killme2008@gmail.com> --------- Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user