mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-10-26 21:56:17 +00:00
feat: otlp duration_nano and trace_flag signed integer coercion (#8816)
* feat(servers): add signed→unsigned int coercion to OTLP ingest path Phase 0 of transitioning built-in data models from unsigned to signed integers (#8793): add lossless Int64→UInt64 and Int32→UInt32 coercion arms so existing UInt64/UInt32 columns (e.g. trace `duration_nano`, log `trace_flags`) keep accepting new signed ingest without an ALTER TABLE. The OTLP ingest path already reconciles every incoming column against the existing table schema and treats it as authoritative. With these arms, `choose_trace_reconcile_decision` returns `UseExisting(UInt64/Uint32)` for an existing unsigned column receiving signed data: the table keeps its type byte-for-byte and the request value is coerced. No persisted format is mutated; existing data stays readable as-is. This is the safety net that makes the actual schema flip (Phase 1) safe. Only the signed→unsigned direction is supported — the reverse would be lossy for values above the signed range and is intentionally rejected. Tests cover both new arms plus an end-to-end log test proving an existing UInt64 column coerces an incoming Int64 value while keeping its type. Signed-off-by: Ning Sun <sunning@greptime.com> * feat: add compatibility layer for uint trace/log fields Signed-off-by: Ning Sun <sunning@greptime.com> * fix: jaeger test Signed-off-by: Ning Sun <sunning@greptime.com> * fix: keep trace v0 unsigned, reject negative span durations - Keep the frozen v0 data model on UInt64 duration_nano: the signed ingest compatibility layer only runs on the v1 path, so flipping v0 would break writes into every pre-existing v0 table at mito's schema check. Pin the schema with unit and integration tests. - Reject spans whose end precedes their start (or whose duration does not fit i64) on the v1 path instead of wrapping: new Int64 tables and existing UInt64 tables now fail identically, rather than storing negative durations that break the Jaeger query API. - Extract is_supported_signed_to_unsigned_coercion so the trace and log ingest paths share one supported-pair predicate and cannot drift. Signed-off-by: Ning Sun <sunning@greptime.com> * fix: clamp negative span durations to zero, revert semantic_graph comment - Record duration 0 for spans whose end precedes their start instead of erroring: a malformed span no longer fails the request, and the value written is always a non-negative, in-range i64 so new Int64 tables and existing UInt64 tables (via the checked coercion) behave identically. Durations above i64::MAX saturate rather than wrap. - Revert the doc-comment tweak on the semantic_graph test fixture; the file is untouched by this PR again. Signed-off-by: Ning Sun <sunning@greptime.com> --------- Signed-off-by: Ning Sun <sunning@greptime.com>
This commit is contained in:
@@ -31,7 +31,7 @@ use opentelemetry_proto::tonic::collector::trace::v1::ExportTraceServiceRequest;
|
||||
use pipeline::{GreptimePipelineParams, PipelineWay};
|
||||
use servers::error::{self, Result as ServerResult};
|
||||
use servers::otlp;
|
||||
use servers::otlp::coerce::{coerce_value_data, trace_value_datatype};
|
||||
use servers::otlp::coerce::{coerce_value_data, is_supported_trace_coercion, trace_value_datatype};
|
||||
use servers::otlp::trace::span::{TraceSpan, TraceSpanGroup};
|
||||
use servers::otlp::trace::v1::{TraceBatchSchema, TraceBinaryType, TraceRetryColumns};
|
||||
use servers::otlp::trace::{SERVICE_NAME_COLUMN, TraceAuxData};
|
||||
@@ -2025,10 +2025,23 @@ fn trace_logical_types_incompatible(
|
||||
right_datatype: ColumnDataType,
|
||||
right_concrete_type: &ConcreteDataType,
|
||||
) -> bool {
|
||||
left_concrete_type != right_concrete_type
|
||||
&& (left_datatype == right_datatype
|
||||
|| !is_trace_reconcile_candidate_type(left_datatype)
|
||||
|| !is_trace_reconcile_candidate_type(right_datatype))
|
||||
if left_concrete_type == right_concrete_type {
|
||||
return false;
|
||||
}
|
||||
// A supported coercion in either direction means the two types can be
|
||||
// reconciled, so they are not logically incompatible. This lets a signed
|
||||
// request reach the coercion path instead of being excluded up front when
|
||||
// the existing column is unsigned (e.g. trace `duration_nano` written as
|
||||
// Int64 into an existing UInt64 column during the unsigned -> signed
|
||||
// transition).
|
||||
if is_supported_trace_coercion(left_datatype, right_datatype)
|
||||
|| is_supported_trace_coercion(right_datatype, left_datatype)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
left_datatype == right_datatype
|
||||
|| !is_trace_reconcile_candidate_type(left_datatype)
|
||||
|| !is_trace_reconcile_candidate_type(right_datatype)
|
||||
}
|
||||
|
||||
fn chunk_owned<T>(items: Vec<T>, chunk_size: usize) -> Vec<Vec<T>> {
|
||||
|
||||
@@ -659,6 +659,34 @@ fn test_trace_request_schema_isolates_non_candidate_batch() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_trace_request_schema_keeps_signed_request_into_existing_unsigned_column() {
|
||||
// Regression for the unsigned -> signed transition: an existing
|
||||
// `duration_nano` UInt64 column must NOT exclude a new signed (Int64)
|
||||
// batch as "logically incompatible". The batch must reach the
|
||||
// reconciliation path, which coerces Int64 -> UInt64 in place (no ALTER).
|
||||
// Excluding it up front made ingestion error after the flip.
|
||||
let column_name = "duration_nano";
|
||||
let int64_schema = field_schema(column_name, ColumnDataType::Int64);
|
||||
let existing_uint64_schema =
|
||||
DatatypesSchemaBuilder::try_from_columns(vec![DatatypesColumnSchema::new(
|
||||
column_name,
|
||||
ConcreteDataType::uint64_datatype(),
|
||||
true,
|
||||
)])
|
||||
.unwrap()
|
||||
.build()
|
||||
.unwrap();
|
||||
|
||||
let mut request_schema = TraceRequestSchema::default();
|
||||
request_schema.observe_trace_column(0, &int64_schema, Some(ColumnDataType::Int64));
|
||||
|
||||
assert_eq!(
|
||||
request_schema.incompatible_schema_observations(Some(&existing_uint64_schema)),
|
||||
HashMap::new()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_trace_request_schema_isolates_non_candidate_new_column_batch() {
|
||||
let column_name = "span_attributes.payload";
|
||||
|
||||
@@ -351,6 +351,24 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_choose_trace_reconcile_decision_existing_uint64_keeps_uint64() {
|
||||
// Backward-compat for the unsigned -> signed transition: an existing
|
||||
// table whose `duration_nano` is still UInt64 must keep that type (no
|
||||
// ALTER) when new signed (Int64) ingest arrives, coercing the value in
|
||||
// place. This is the no-ALTER guarantee for the trace path; it relies on
|
||||
// the Int64 -> UInt64 coercion arm added in Phase 0.
|
||||
assert_eq!(
|
||||
choose_trace_reconcile_decision(
|
||||
"duration_nano",
|
||||
&[ColumnDataType::Int64],
|
||||
Some(ColumnDataType::Uint64)
|
||||
)
|
||||
.unwrap(),
|
||||
Some(TraceReconcileDecision::UseExisting(ColumnDataType::Uint64))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_choose_trace_reconcile_decision_existing_int64_widens_to_float64() {
|
||||
assert_eq!(
|
||||
@@ -612,6 +630,42 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_trace_column_rewrites_coerces_int64_into_existing_uint64() {
|
||||
// Existing-table backward-compat for the trace path: new signed ingest
|
||||
// arrives as Int64, but the existing `duration_nano` column is UInt64, so
|
||||
// the rewrite coerces the value into the existing type in place (no
|
||||
// ALTER). Mirrors what happens for a table created before the
|
||||
// unsigned -> signed flip.
|
||||
let mut rows = Rows {
|
||||
schema: vec![ColumnSchema {
|
||||
datatype: ColumnDataType::Int64 as i32,
|
||||
..Default::default()
|
||||
}],
|
||||
rows: vec![Row {
|
||||
values: vec![Value {
|
||||
value_data: Some(ValueData::I64Value(42)),
|
||||
}],
|
||||
}],
|
||||
};
|
||||
let pending_rewrites = vec![PendingTraceColumnRewrite {
|
||||
col_idx: 0,
|
||||
target_type: ColumnDataType::Uint64,
|
||||
column_name: "duration_nano".to_string(),
|
||||
}];
|
||||
|
||||
let prepared =
|
||||
prepare_trace_column_rewrites(&rows.rows, pending_rewrites, "trace_type_atomicity")
|
||||
.unwrap();
|
||||
|
||||
prepared.apply(&mut rows);
|
||||
assert_eq!(rows.schema[0].datatype, ColumnDataType::Uint64 as i32);
|
||||
assert_eq!(
|
||||
rows.rows[0].values[0].value_data,
|
||||
Some(ValueData::U64Value(42))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_trace_column_rewrites_boolean_rejects_invalid_string_parse() {
|
||||
let rows = vec![Row {
|
||||
|
||||
Reference in New Issue
Block a user