fix(otel): preserve all exemplars per data point (#1662)#1720
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughOTEL metrics now support configurable exemplar storage: nested arrays preserve all exemplars by default, while ChangesOTEL exemplar handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OTELClient
participant flatten_otel_metrics
participant process_resource_metrics
participant flatten_metrics_record
participant insert_exemplars
OTELClient->>flatten_otel_metrics: OTEL metrics payload
flatten_otel_metrics->>process_resource_metrics: configured exemplar mode
process_resource_metrics->>flatten_metrics_record: metric record and mode
flatten_metrics_record->>insert_exemplars: exemplar list and mode
insert_exemplars-->>flatten_metrics_record: nested array or flattened columns
flatten_metrics_record-->>process_resource_metrics: flattened records
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note on the failing |
|
@prabhaks thank you for raising the lint PR, i have verified and merged it, you can rebase this PR now |
42b3bde to
1e9eb32
Compare
Done, thank you for reviewing and merging my other PR! |
insert_exemplars wrote every exemplar of a data point to the same static columns (exemplar_time_unix_nano, exemplar_span_id, exemplar_trace_id, exemplar_value), so each exemplar overwrote the previous one and only the last survived. This affected Sum, Histogram, ExponentialHistogram (and Gauge) metric types. Serialize all exemplars into a single nested `exemplars` array column (array of objects), the OTel-faithful representation recommended in the issue. Each element carries exemplar_time_unix_nano, exemplar_span_id, exemplar_trace_id, exemplar_value, and any filtered attributes. Add the P_OTEL_FLATTEN_EXEMPLARS flag (default false) to opt back into the legacy flat columns for backward compatibility. Also stops exemplar attributes from leaking into the physical-series hash: `exemplars` is added to OTEL_METRICS_KNOWN_FIELD_LIST so exemplar contents no longer fragment series identity.
1e9eb32 to
bf49759
Compare
|
@nikhilsinhaparseable @parmesant please take a look at this PR whenever you get a chance! |
What
Fixes #1662.
insert_exemplarsinsrc/otel/metrics.rswrote every exemplar of a data point to the same static columns (exemplar_time_unix_nano,exemplar_span_id,exemplar_trace_id,exemplar_value). Since the keys were shared, each exemplar overwrote the previous one and only the last survived. This affected Sum, Histogram and ExponentialHistogram (and Gauge, which shares the sameNumberDataPointpath).Approach
Following the recommended option 1 in the issue, all exemplars for a data point are now serialized into a single nested
exemplarsarray column. Each element is an object withexemplar_time_unix_nano,exemplar_span_id,exemplar_trace_id,exemplar_valueand any filtered attributes. This matches the OTel data model (exemplars are a repeated field per data point) and keeps the schema stable regardless of exemplar count.Backward compatibility
Added a
P_OTEL_FLATTEN_EXEMPLARSflag (defaultfalse). When set totruethe previous flatexemplar_*columns are emitted (last exemplar wins, same as before) for anyone already querying those columns. Per the discussion on the issue, the default is the new array behavior.Bonus fix
Previously exemplar attributes were written straight into the data point map, so they leaked into
compute_series_hashand fragmented physical-series identity.exemplarsis now part ofOTEL_METRICS_KNOWN_FIELD_LIST, so exemplar contents no longer affect the series hash.Query validation (PoC)
Before settling on the schema I ran a PoC against the same library versions used here (datafusion 53, arrow/arrow-json 58) to confirm the query layer handles the array-of-objects column. arrow-json infers it as
List<Struct<...>>, and DataFusion handles reading the column,array_length,unnest, struct field access, and filtering on nested fields. Details are in the issue thread.Testing
cargo test: 311 passed, 0 failed.Flag reference
P_OTEL_FLATTEN_EXEMPLARSenv var /--otel-flatten-exemplarsflag, defaultfalse(new nestedexemplarsarray). Set totruefor the legacy flatexemplar_*columns.Summary by CodeRabbit
P_OTEL_FLATTEN_EXEMPLARS(--otel-flatten-exemplars).exemplarsarray.