Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
37 changes: 34 additions & 3 deletions crates/ourios-ingester/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,45 @@ impl IngestMetrics {

/// Record one durably-acknowledged batch: `record_count` log records
/// made durable in `elapsed` (the append + fsync / WAL-before-ack
/// latency). Call only on a successful, acked commit.
pub fn record_batch(&self, record_count: usize, elapsed: Duration) {
/// latency), of which `severity_out_of_range` carried an out-of-`0..=24`
/// `SeverityNumber`. Call only on a successful, acked commit.
///
/// Per the OpenTelemetry "recording errors on metrics" convention, the
/// `ourios.ingest.records` counter carries the standard `error.type`
/// attribute: in-range records record with it **absent** (success),
/// out-of-range ones with `error.type = severity_out_of_range`
/// (RFC 0018 §3.5) — one counter, reason on a low-cardinality attribute,
/// not a bespoke metric.
pub fn record_batch(
&self,
record_count: usize,
severity_out_of_range: usize,
elapsed: Duration,
) {
self.batches.add(1, &[]);
self.records.add(to_u64(record_count), &[]);
let in_range = record_count.saturating_sub(severity_out_of_range);
if in_range > 0 {
self.records.add(to_u64(in_range), &[]);
}
if severity_out_of_range > 0 {
self.records.add(
to_u64(severity_out_of_range),
&[KeyValue::new(ERROR_TYPE, SEVERITY_OUT_OF_RANGE)],
);
}
self.append_duration.record(elapsed.as_secs_f64(), &[]);
}
}

/// The OpenTelemetry-standard `error.type` attribute key (semconv, stable).
/// Deliberately **not** in the Ourios weaver registry — it is an upstream
/// OpenTelemetry attribute used here per the "recording errors on metrics"
/// convention, not an Ourios-coined name.
const ERROR_TYPE: &str = "error.type";
/// The domain-specific `error.type` value for an out-of-`0..=24`
/// `SeverityNumber` (RFC 0018 §3.5). `error.type`'s value space is open.
const SEVERITY_OUT_OF_RANGE: &str = "severity_out_of_range";

impl Default for IngestMetrics {
fn default() -> Self {
Self::new()
Expand Down
26 changes: 19 additions & 7 deletions crates/ourios-ingester/src/receiver/materialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,26 @@ pub fn materialize_resource_logs(
records
}

/// Narrow proto's `i32` `severity_number` to the schema's `u8`. Valid
/// OTLP severity is `0..=24` (`0` = UNSPECIFIED); any value outside that
/// range — invalid-but-`u8`-representable (`25..=255`), negative, or
/// `> 255` — maps to `0`/UNSPECIFIED, so the `OtlpLogRecord` contract the
/// miner's template key and the Parquet schema rely on holds at this
/// boundary.
/// Narrow proto's `i32` `severity_number` to the schema's `u8`, **preserving**
/// the wire value (RFC 0018 §3.5 — faithful witness, §3.0). Valid OTLP
/// severity is `0..=24` (`0` = UNSPECIFIED); out-of-named-range values
/// (`25..=255`) are kept verbatim (monotone-meaningful and surfaced via the
/// ingest counter's `error.type = severity_out_of_range`, not silently
/// clamped). Only the extremes a `u8` cannot hold — negative or `> 255` —
/// narrow to `0`, where the storage invariant wins.
fn severity_to_u8(n: i32) -> u8 {
u8::try_from(n).ok().filter(|v| *v <= 24).unwrap_or(0)
u8::try_from(n).unwrap_or(0)
}

/// Whether a stored `severity_number` is outside the OTLP `0..=24` range
/// (RFC 0018 §3.5). Drives the `error.type = severity_out_of_range` attribute
/// on the ingest counter. Operates on the *preserved* `u8`: `25..=255` is
/// detected here; the negative / `> 255` extremes narrowed to `0` are out of
/// range too but indistinguishable post-narrowing, so attribution is
/// best-effort over the `u8`-storable band (see §3.5).
#[must_use]
pub(crate) fn severity_is_out_of_range(severity_number: u8) -> bool {
severity_number > 24
}

/// Proto scalar `0` → `None`, else `Some` — the RFC0003.9 narrowing of a
Expand Down
11 changes: 10 additions & 1 deletion crates/ourios-ingester/src/receiver/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,16 @@ impl IngestPipeline {
// replay re-covers those frames (no §3.5.3 divergence).
*self.lock_last_durable() = Some(now);
// Throughput + WAL-before-ack latency for this acked batch.
self.metrics.record_batch(records.len(), append_elapsed);
// RFC 0018 §3.5: tag out-of-range-severity records on the
// ingest counter via `error.type` (post-materialise on the
// preserved `u8`; the rare non-`u8` extremes narrowed to 0
// aren't separately attributed — accepted limitation).
let severity_out_of_range = records
.iter()
.filter(|r| super::materialize::severity_is_out_of_range(r.severity_number))
.count();
self.metrics
.record_batch(records.len(), severity_out_of_range, append_elapsed);
Ok(records.len())
}
// Sync failed: the frame is not durable and not acked; it
Expand Down
2 changes: 1 addition & 1 deletion crates/ourios-ingester/tests/perf_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async fn ingest_and_sink_metrics_export_under_their_registry_names() {
let sink = SinkMetrics::new();

// Act — record a representative slice of the perf signals.
ingest.record_batch(10, Duration::from_millis(2));
ingest.record_batch(10, 0, Duration::from_millis(2));
sink.record_flush("size", 5, Duration::from_millis(7));
sink.record_flush("rotation", 3, Duration::from_millis(9));
sink.record_flush_error();
Expand Down
25 changes: 17 additions & 8 deletions crates/ourios-ingester/tests/rfc0003_9_edge_otlp_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,31 @@ fn rfc0003_9_edge_fields_pass_through_without_coalescing() {
);
}

/// Scenario RFC0003.9 — out-of-range `severity_number` narrows to UNSPECIFIED.
/// See `docs/rfcs/0003-otlp-receiver.md` §5.
/// Scenario RFC0003.9 — out-of-range `severity_number` is **preserved**, not
/// narrowed (RFC 0018 §3.5 supersedes the prior clamp-to-UNSPECIFIED: the
/// receiver is a faithful witness — §3.0 — so out-of-named-range values pass
/// through, more consistent with RFC0003.9's own "edge fields pass through
/// unchanged" theme). Only the extremes a `u8` cannot hold (negative, `>255`)
/// narrow to `0`, where the storage invariant wins.
/// See `docs/rfcs/0003-otlp-receiver.md` §5; `docs/rfcs/0018-otlp-log-spec-compliance.md` §3.5.
#[test]
fn rfc0003_9_out_of_range_severity_narrows_to_unspecified() {
// Valid OTLP severity is 0..=24; values outside that range (incl.
// u8-representable 25..=255, negative, and > 255) normalise to
// 0/UNSPECIFIED so the OtlpLogRecord 0..=24 contract holds.
for (wire, expected) in [(0i32, 0u8), (24, 24), (25, 0), (1000, 0), (-5, 0)] {
fn rfc0003_9_out_of_range_severity_is_preserved() {
for (wire, expected) in [
(0i32, 0u8),
(24, 24),
(25, 25), // out of the named range, but u8-storable → preserved
(200, 200), // preserved
(1000, 0), // not u8-storable → storage-invariant narrow to 0
(-5, 0), // not u8-storable → 0
] {
let record = LogRecord {
severity_number: wire,
..Default::default()
};
let materialized = materialize_record(record, &[], "", None, "", TenantId::new("tenant-a"));
assert_eq!(
materialized.severity_number, expected,
"severity_number {wire} narrows to {expected}",
"severity_number {wire} → {expected} (preserve verbatim; non-u8 → 0)",
);
}
}
94 changes: 82 additions & 12 deletions crates/ourios-ingester/tests/rfc0018_otlp_compliance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@
//!
//! See `docs/rfcs/0018-otlp-log-spec-compliance.md` §5/§6.

use std::time::Duration;

use opentelemetry_proto::tonic::common::v1::any_value::Value;
use opentelemetry_proto::tonic::common::v1::{AnyValue, InstrumentationScope, KeyValue};
use opentelemetry_proto::tonic::logs::v1::{LogRecord, ResourceLogs, ScopeLogs};
use opentelemetry_proto::tonic::resource::v1::Resource;
use opentelemetry_sdk::metrics::data::{
AggregatedMetrics, MetricData, ResourceMetrics, SumDataPoint,
};
use ourios_core::tenant::TenantId;
use ourios_ingester::receiver::materialize_resource_logs;
use ourios_ingester::metrics::IngestMetrics;
use ourios_ingester::receiver::{materialize_record, materialize_resource_logs};
use ourios_semconv as semconv;

fn kv(key: &str, value: &str) -> KeyValue {
KeyValue {
Expand Down Expand Up @@ -86,17 +93,80 @@ fn rfc0018_3_transient_failure_is_retryable() {
todo!("RFC0018.3: transient -> retryable code; permanent -> INVALID_ARGUMENT/400")
}

/// Sum of `ourios.ingest.records` datapoints, filtered by `error.type`:
/// `None` → success points (the attribute absent); `Some(v)` → points whose
/// `error.type` equals `v`.
fn ingest_records_sum(rms: &[ResourceMetrics], error_type: Option<&str>) -> u64 {
let data = rms
.iter()
.flat_map(ResourceMetrics::scope_metrics)
.flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics)
.find(|m| m.name() == semconv::OURIOS_INGEST_RECORDS)
.map(opentelemetry_sdk::metrics::data::Metric::data)
.expect("ourios.ingest.records exported");
let AggregatedMetrics::U64(MetricData::Sum(sum)) = data else {
panic!("ourios.ingest.records should be a u64 sum");
};
sum.data_points()
.filter(|dp| {
let et = dp
.attributes()
.find(|kv| kv.key.as_str() == "error.type")
.map(|kv| kv.value.as_str().into_owned());
match error_type {
None => et.is_none(),
Some(v) => et.as_deref() == Some(v),
}
})
.map(SumDataPoint::value)
.sum()
}

/// Scenario RFC0018.6 — out-of-range `SeverityNumber` is preserved, not clamped:
/// `severity_number` 25 / 200 are stored verbatim (never silently clamped to 0),
/// the `ingest.severity_out_of_range` metric increments, a `severity >= ERROR`
/// query still matches the preserved 25 / 200 (monotonicity), and a value a
/// `u8` cannot hold (negative, > 255) maps to 0 + the same anomaly count.
/// the receiver preserves `25` / `200` verbatim (non-`u8` → `0`), and the
/// `ourios.ingest.records` counter records out-of-range records with
/// `error.type = severity_out_of_range` (in-range ones carry no `error.type`).
/// Monotonicity (`severity >= ERROR` still matches the preserved `25`) is the
/// querier's `SeverityNumber` comparison — covered in
/// `ourios-querier/tests/rfc0018_severity.rs`.
/// See `docs/rfcs/0018-otlp-log-spec-compliance.md` §5.
#[test]
#[ignore = "RFC0018.6 — red until severity preserve+flag replaces the clamp-to-0 (green)"]
fn rfc0018_6_out_of_range_severity_preserved() {
todo!(
"RFC0018.6: 25/200 preserved + anomaly metric; severity >= ERROR still \
matches them (monotonicity); non-u8 -> 0 (storage invariant)"
)
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn rfc0018_6_out_of_range_severity_preserved() {
// Receiver: preserve the wire value (non-u8 extremes narrow to 0).
for (wire, expected) in [(25i32, 25u8), (200, 200), (1000, 0), (-5, 0)] {
let m = materialize_record(
LogRecord {
severity_number: wire,
..Default::default()
},
&[],
"",
None,
"",
TenantId::new("tenant-a"),
);
assert_eq!(
m.severity_number, expected,
"severity {wire} preserved as {expected} (non-u8 → 0)",
);
}

// Metric: a 4-record batch, 2 out-of-range, splits onto the records
// counter via error.type (OTel "recording errors on metrics" convention).
let (guard, exporter) = ourios_telemetry::init_in_memory("ourios-test-rfc0018-6");
let ingest = IngestMetrics::new();
ingest.record_batch(4, 2, Duration::from_millis(1));
guard.force_flush().expect("force_flush");
let rms = exporter.get_finished_metrics().expect("metrics exported");

assert_eq!(
ingest_records_sum(&rms, None),
2,
"in-range records carry no error.type",
);
assert_eq!(
ingest_records_sum(&rms, Some("severity_out_of_range")),
2,
"out-of-range records tagged error.type = severity_out_of_range",
);
}
49 changes: 49 additions & 0 deletions crates/ourios-querier/tests/rfc0018_severity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//! RFC 0018 §5 — RFC0018.6 monotonicity arm.
//!
//! Out-of-range `SeverityNumber` is preserved (not clamped) at ingest, so a
//! `severity >= ERROR` query must still match a preserved `25` — the
//! `SeverityNumber` is monotone, and `25 >= 17 (ERROR)`. Proves the querier's
//! severity comparison treats the preserved value correctly end-to-end.
//!
//! See `docs/rfcs/0018-otlp-log-spec-compliance.md` §5; the receiver-preserve
//! + `error.type` arms live in `ourios-ingester/tests/rfc0018_otlp_compliance.rs`.

mod common;

use common::{DEFAULT_WINDOW_NS, NOW, TS0, no_aliases, simple, write_all};
use ourios_core::record::MinedRecord;
use ourios_core::tenant::TenantId;
use ourios_querier::Querier;

/// Scenario RFC0018.6 — a `severity >= ERROR` query matches a preserved
/// out-of-range `SeverityNumber` (monotonicity).
/// See `docs/rfcs/0018-otlp-log-spec-compliance.md` §5.
#[tokio::test]
async fn rfc0018_6_severity_compare_matches_preserved_out_of_range() {
let bucket = tempfile::TempDir::new().expect("temp");
let sev = |n: u8, i: u64| MinedRecord {
severity_number: n,
..simple("t", 1, TS0 + i * 1_000)
};
write_all(
bucket.path(),
&[
sev(25, 0), // out-of-named-range, preserved — must match `>= error` (25 >= 17)
sev(200, 1), // also preserved, also >= error
sev(9, 2), // INFO — must NOT match `>= error`
],
);

let q = Querier::new(bucket.path());
let tenant = TenantId::new("t");
let query = ourios_querier::dsl::parse("severity >= error").expect("parse");
let result = q
.run_query(&query, &tenant, NOW, DEFAULT_WINDOW_NS, Some(&no_aliases()))
.await
.expect("run_query");

assert_eq!(
result.rows, 2,
"the preserved 25 and 200 match `severity >= error` (monotonic); INFO (9) does not",
);
}
43 changes: 27 additions & 16 deletions docs/rfcs/0018-otlp-log-spec-compliance.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,20 @@ Change to **preserve verbatim**:

- `0..=24` (defined) and `25..=255` (out of the named ranges but storable
and monotone-meaningful) → stored as the wire value;
- a record with `severity_number` outside `0..=24` increments an anomaly
metric (`ingest.severity_out_of_range`), and `severity_text` is retained,
so the violation is observable, not masked;
- only the values a `u8` physically cannot hold (negative, `> 255`) become
`0` + the same anomaly count — here the storage invariant wins (§3.0
point 1's limit).
- a record with `severity_number` outside `0..=24` is recorded on the
existing `ourios.ingest.records` counter with the standard **`error.type`**
attribute set to `severity_out_of_range` — the OTel "recording errors on
metrics" convention (one counter for success + anomaly, reason on a
low-cardinality `error.type`; success records carry no `error.type`), not
a bespoke counter. `severity_text` is retained, so the violation is
observable, not masked;
- the values a `u8` physically cannot hold (negative, `> 255`) become `0` —
here the storage invariant wins (§3.0 point 1's limit). Because they
narrow to `0`, they are indistinguishable post-narrowing from a genuine
UNSPECIFIED and so are **not** separately attributed on the counter (an
accepted limitation: such values are degenerate corruption, not a
meaningful severity); the `25..=255` case — the one an operator actually
sees — is fully attributed.

Severity comparisons (RFC 0002, which correctly compares on
`SeverityNumber`) stay monotone and correct: `severity >= ERROR` still
Expand Down Expand Up @@ -291,12 +299,12 @@ back-reference.
> (out of the named ranges but `u8`-storable)
> - **When** the receiver materialises them
> - **Then** the stored `severity_number` is `25` / `200` verbatim
> (never silently clamped to `0`), the `ingest.severity_out_of_range`
> anomaly metric is incremented, and a `severity >= ERROR` query still
> matches them (monotonicity preserved)
> (never silently clamped to `0`), the `ourios.ingest.records` counter
> records them with `error.type = severity_out_of_range`, and a
> `severity >= ERROR` query still matches them (monotonicity preserved)
> - **And** a value a `u8` cannot hold (negative, `> 255`) maps to `0`
> with the same anomaly count — the storage invariant, not a
> correction
> (the storage invariant, not a correction); narrowed to `0`, it is not
> separately attributed (the §3.5 accepted limitation)

## 6. Testing strategy

Expand All @@ -315,11 +323,14 @@ back-reference.
doubles, replacing the current "encodes to null" assertion with a
round-trip one.
- **RFC0018.6** — a receiver test feeding `severity_number` 25 and 200 and
asserting they are **preserved** (not clamped) + the
`ingest.severity_out_of_range` counter increments; plus a negative / `>255`
case asserting `0` + counter (the storage-invariant limit). Replaces the
prior clamp-to-0 assertion in `severity_to_u8`'s tests (a contract change —
the old test asserted the behaviour this RFC overturns; CLAUDE.md §6.2).
asserting they are **preserved** (not clamped), that the
`ourios.ingest.records` counter records them with
`error.type = severity_out_of_range` (in-memory `MeterProvider`, mirroring
the compaction-metric test), and that a `severity >= ERROR` query still
matches them (monotonicity); plus a negative / `>255` case asserting `0`
(the storage-invariant limit). Replaces the prior clamp-to-0 assertion in
`severity_to_u8`'s tests (a contract change — the old test asserted the
behaviour this RFC overturns; CLAUDE.md §6.2).

Each scenario id (`RFC0018.N`) is referenced from its test so the mapping
is greppable (`docs/verification.md` §2).
Expand Down
Loading