update trace field names for events and links #1455
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add prefix event_ for event attributes add prefix link_ for links attributes update timestamp to have nanoseconds
WalkthroughUpdated timestamp formatter to emit nanosecond precision; added two public helpers to flatten and insert event/link attributes with prefixed keys; switched span/event duration fields to nanoseconds and added epoch timestamp fields; updated callers, known-field list, and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as flatten_events / flatten_links / flatten_span_record
participant Helper as insert_events_attributes / insert_links_attributes
participant Flattener as flatten_attributes
Note over Caller,Helper: New attribute flatten+prefix insertion flow
Caller->>Helper: (map, attributes)
Helper->>Flattener: flatten(attributes)
Flattener-->>Helper: flattened pairs
Helper-->>Caller: insert prefixed keys (event_* / link_*)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-09-18T09:59:20.177ZApplied to files:
📚 Learning: 2025-08-25T01:32:25.980ZApplied to files:
📚 Learning: 2025-08-25T01:31:41.786ZApplied to files:
📚 Learning: 2025-08-21T12:04:38.398ZApplied to files:
📚 Learning: 2025-08-18T12:37:47.732ZApplied to files:
📚 Learning: 2025-09-25T07:13:58.909ZApplied to files:
🧬 Code graph analysis (1)src/otel/traces.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (6)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/otel/otel_utils.rs(1 hunks)src/otel/traces.rs(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1410
File: src/storage/object_storage.rs:0-0
Timestamp: 2025-08-21T12:04:38.398Z
Learning: In Parseable's timestamp extraction (extract_timestamp_for_date in src/storage/object_storage.rs), minute-level precision is sufficient for both first and latest event timestamps. Seconds are always set to 0, meaning both first and latest events within the same minute will have identical timestamps, which is the intended behavior.
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1410
File: src/storage/object_storage.rs:0-0
Timestamp: 2025-08-21T11:47:01.279Z
Learning: In Parseable's object storage implementation (src/storage/object_storage.rs), the hour and minute directory prefixes (hour=XX, minute=YY) are generated from arrow file timestamps following proper datetime conventions, so they are guaranteed to be within valid ranges (0-23 for hours, 0-59 for minutes) and don't require additional range validation.
📚 Learning: 2025-09-18T09:59:20.177Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
Applied to files:
src/otel/otel_utils.rs
📚 Learning: 2025-08-21T12:04:38.398Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1410
File: src/storage/object_storage.rs:0-0
Timestamp: 2025-08-21T12:04:38.398Z
Learning: In Parseable's timestamp extraction (extract_timestamp_for_date in src/storage/object_storage.rs), minute-level precision is sufficient for both first and latest event timestamps. Seconds are always set to 0, meaning both first and latest events within the same minute will have identical timestamps, which is the intended behavior.
Applied to files:
src/otel/otel_utils.rs
🧬 Code graph analysis (1)
src/otel/traces.rs (1)
src/otel/otel_utils.rs (1)
flatten_attributes(152-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/otel/traces.rs (2)
588-591: Fix test to expect prefixed event attributes.The assertion checks for
"service.name"but with the newinsert_events_attributeshelper, event attributes are now prefixed withevent_. The test should check for"event_service.name"instead.Apply this diff:
assert!( - first_event.contains_key("service.name"), + first_event.contains_key("event_service.name"), "Should contain flattened attributes" );
639-642: Fix test to expect prefixed link attributes.The assertion checks for
"service.name"but with the newinsert_links_attributeshelper, link attributes are now prefixed withlink_. The test should check for"link_service.name"instead.Apply this diff:
assert!( - link.contains_key("service.name"), + link.contains_key("link_service.name"), "Should contain flattened attributes" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/otel/traces.rs(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
📚 Learning: 2025-09-18T09:59:20.177Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
Applied to files:
src/otel/traces.rs
🧬 Code graph analysis (1)
src/otel/traces.rs (1)
src/otel/otel_utils.rs (1)
flatten_attributes(152-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (7)
src/otel/traces.rs (7)
19-19: LGTM!The new imports are necessary for the helper functions that prefix event and link attributes.
Also applies to: 28-29
49-49: LGTM!The field name updates correctly reflect the change from millisecond to nanosecond precision.
Also applies to: 52-52
202-202: LGTM!The change correctly applies the
event_prefix to event attributes using the new helper function.
229-229: LGTM!The change correctly applies the
link_prefix to link attributes using the new helper function.
354-361: LGTM!The span duration calculation correctly uses nanosecond precision with integer representation.
398-410: LGTM!The helper functions correctly flatten attributes and apply the appropriate prefixes. The implementation is clean and reuses the existing
flatten_attributesutility.
979-979: LGTM!Test expectations correctly updated to verify the new nanosecond-precision field names.
Also applies to: 986-986
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/otel/traces.rs (1)
33-66: Add missing epoch fields to the known field list.The known field list is missing three new epoch fields that are added to the flattened JSON output:
event_time_unix_nano_epoch(added at line 190)span_start_time_unix_nano_epoch(added at line 351)span_end_time_unix_nano_epoch(added at line 362)These fields should be included in
OTEL_TRACES_KNOWN_FIELD_LISTto ensure they are properly recognized as known fields.Apply this diff to add the missing fields:
pub const OTEL_TRACES_KNOWN_FIELD_LIST: [&str; 32] = [ "scope_name", "scope_version", "scope_schema_url", "scope_dropped_attributes_count", "resource_schema_url", "resource_dropped_attributes_count", "span_trace_id", "span_span_id", "span_name", "span_parent_span_id", "name", "span_kind", "span_kind_description", "span_start_time_unix_nano", + "span_start_time_unix_nano_epoch", "span_end_time_unix_nano", + "span_end_time_unix_nano_epoch", "span_duration_ns", "event_name", "event_time_unix_nano", + "event_time_unix_nano_epoch", "event_duration_ns", "event_dropped_attributes_count", "link_span_id", "link_trace_id", "link_dropped_attributes_count", "span_dropped_events_count", "span_dropped_links_count", "span_dropped_attributes_count", "span_trace_state", "span_flags", "span_flags_description", "span_status_code", "span_status_description", "span_status_message", ];Note: Update the array size from
32to35.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/otel/traces.rs(10 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
📚 Learning: 2025-09-18T09:59:20.177Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:700-756
Timestamp: 2025-09-18T09:59:20.177Z
Learning: In src/event/mod.rs, the parsed_timestamp used in increment_events_ingested_by_date() is correctly UTC-normalized: for dynamic streams it remains Utc::now(), and for streams with time partition enabled it uses the time partition value. Both cases result in proper UTC date strings for metrics labeling, preventing double-counting issues.
Applied to files:
src/otel/traces.rs
📚 Learning: 2025-08-25T01:32:25.980Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metrics/mod.rs:163-173
Timestamp: 2025-08-25T01:32:25.980Z
Learning: The TOTAL_EVENTS_INGESTED_DATE, TOTAL_EVENTS_INGESTED_SIZE_DATE, and TOTAL_EVENTS_STORAGE_SIZE_DATE metrics in src/metrics/mod.rs are intentionally designed to track global totals across all streams for a given date, using labels ["format", "date"] rather than per-stream labels. This is the correct design for global aggregation purposes.
Applied to files:
src/otel/traces.rs
📚 Learning: 2025-08-25T01:31:41.786Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metadata.rs:63-68
Timestamp: 2025-08-25T01:31:41.786Z
Learning: The TOTAL_EVENTS_INGESTED_DATE, TOTAL_EVENTS_INGESTED_SIZE_DATE, and TOTAL_EVENTS_STORAGE_SIZE_DATE metrics in src/metadata.rs and src/storage/object_storage.rs are designed to track total events across all streams, not per-stream. They use labels [origin, parsed_date] to aggregate by format and date, while per-stream metrics use [stream_name, origin, parsed_date] labels.
Applied to files:
src/otel/traces.rs
📚 Learning: 2025-08-21T12:04:38.398Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1410
File: src/storage/object_storage.rs:0-0
Timestamp: 2025-08-21T12:04:38.398Z
Learning: In Parseable's timestamp extraction (extract_timestamp_for_date in src/storage/object_storage.rs), minute-level precision is sufficient for both first and latest event timestamps. Seconds are always set to 0, meaning both first and latest events within the same minute will have identical timestamps, which is the intended behavior.
Applied to files:
src/otel/traces.rs
📚 Learning: 2025-08-18T12:37:47.732Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1405
File: src/parseable/mod.rs:528-533
Timestamp: 2025-08-18T12:37:47.732Z
Learning: In Parseable, the validate_time_partition function in src/utils/json/flatten.rs already provides a default time partition limit of 30 days using `map_or(30, |days| days.get() as i64)` when time_partition_limit is None, so no additional defaulting is needed in the stream creation logic in src/parseable/mod.rs.
Applied to files:
src/otel/traces.rs
🧬 Code graph analysis (1)
src/otel/traces.rs (1)
src/otel/otel_utils.rs (2)
flatten_attributes(152-163)convert_epoch_nano_to_timestamp(192-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: coverage
🔇 Additional comments (6)
src/otel/traces.rs (6)
19-19: LGTM! Necessary imports for the new helpers.The
KeyValueandflatten_attributesimports are correctly added to support the newinsert_events_attributesandinsert_links_attributeshelper functions.Also applies to: 28-28
188-192: LGTM! Event flattening correctly updated.The changes correctly:
- Add epoch field for event time using integer representation
- Change duration from milliseconds to nanoseconds
- Use the new
insert_events_attributeshelper to add prefixed attributesAlso applies to: 200-201, 204-204
231-231: LGTM! Link flattening correctly updated.The change correctly uses the new
insert_links_attributeshelper to add thelink_prefix to link attributes.
349-353: LGTM! Span timing fields correctly updated.The changes correctly:
- Add epoch fields for both span start and end times
- Change duration calculation from milliseconds to nanoseconds
- Use integer representation consistently with
serde_json::Number::fromAlso applies to: 360-364, 366-373
410-422: LGTM! Well-designed helper functions.The new
insert_events_attributesandinsert_links_attributeshelpers provide a clean, reusable way to flatten and prefix event/link attributes. The implementation correctly:
- Reuses the
flatten_attributesutility fromotel_utils- Applies the appropriate prefix (
event_orlink_) to each attribute key- Maintains public visibility for potential reuse
601-601: LGTM! Tests correctly updated for new field names.The test assertions correctly verify:
- Prefixed attribute keys (
event_service.name,link_service.name)- Updated duration field names (
event_duration_ns,span_duration_ns)Note: Once the missing epoch fields are added to
OTEL_TRACES_KNOWN_FIELD_LIST, the test at line 970 (test_known_field_list_completeness) will need to be updated to include those fields in theexpected_fieldsarray.Also applies to: 652-652, 991-991, 998-998
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add prefix event_ for event attributes
add prefix link_ for links attributes
update timestamp to have nanoseconds
Summary by CodeRabbit
Improvements
Refactor