Repository navigation
feat(codecs): convert native Vector logs to OTLP in otlp serializer - #26605
thomasqueirozb wants to merge 14 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 751cb62de9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48f4e0c219
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…s in native log round trips
…y resource and scope
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d74d78f69a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| match hex::decode(&hex) { | ||
| Ok(id) if id.is_empty() || id.len() == LEN => Ok(id), | ||
| _ => Err(Value::Bytes(hex)), |
There was a problem hiding this comment.
Reject all-zero trace and span identifiers
When a native event supplies a correctly sized all-zero trace_id or span_id, this length-only check accepts it and removes the original field from the attributes. The vendored OTLP definition explicitly considers all-zero IDs invalid, and receivers are instructed to treat the record as unassociated, so the identifier is effectively lost instead of being retained as an attribute like other invalid IDs. Require at least one nonzero byte in addition to the expected length.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining, same as #26605 (comment). We send back the same bytes we received. It is not the encoder's job to check that a trace_id is valid.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7411498c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Legacy logs whose configured timestamp is nested under the message lose their OTLP event time. The new tests also lack an isolated-crate dependency feature and conflict when run in one process.
🤖 Bits Code Review · Commit 3c57c6a
| if let Some(path) = schema.message_key_target_path() { | ||
| record.body = log.remove_prune(path, true).and_then(into_body); | ||
| } | ||
| // The timestamp key can also point into metadata. Many sources and decoders write it | ||
| // there with its full target path, but the `opentelemetry` source writes it to the | ||
| // event root, so read the metadata first and then the event root. | ||
| let metadata_time = schema | ||
| .timestamp_key_target_path() | ||
| .filter(|path| path.prefix == PathPrefix::Metadata) | ||
| .and_then(|path| log.remove(path)) | ||
| .and_then(|value| into_timestamp_nanos(value).ok()); | ||
| let (mut fields, _) = log.into_parts(); | ||
| if let Some(path) = source_type_key { | ||
| fields.remove(&path.path, true); | ||
| } | ||
| record.time_unix_nano = metadata_time | ||
| .or_else(|| { | ||
| schema | ||
| .timestamp_key() | ||
| .and_then(|path| take(&mut fields, path, into_timestamp_nanos)) | ||
| }) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
Read timestamps before removing the message subtree
With message_key = .payload and timestamp_key = .payload.timestamp, removing the message subtree erases the timestamp before conversion reads it. A valid event timestamp therefore becomes time_unix_nano = 0, including for decoded OpenTelemetry logs. Cache the timestamp before removing the body so collectors retain the event's original time.
| if let Some(path) = schema.message_key_target_path() { | |
| record.body = log.remove_prune(path, true).and_then(into_body); | |
| } | |
| // The timestamp key can also point into metadata. Many sources and decoders write it | |
| // there with its full target path, but the `opentelemetry` source writes it to the | |
| // event root, so read the metadata first and then the event root. | |
| let metadata_time = schema | |
| .timestamp_key_target_path() | |
| .filter(|path| path.prefix == PathPrefix::Metadata) | |
| .and_then(|path| log.remove(path)) | |
| .and_then(|value| into_timestamp_nanos(value).ok()); | |
| let (mut fields, _) = log.into_parts(); | |
| if let Some(path) = source_type_key { | |
| fields.remove(&path.path, true); | |
| } | |
| record.time_unix_nano = metadata_time | |
| .or_else(|| { | |
| schema | |
| .timestamp_key() | |
| .and_then(|path| take(&mut fields, path, into_timestamp_nanos)) | |
| }) | |
| .unwrap_or_default(); | |
| // The configured timestamp can be inside the message subtree. | |
| let timestamp_before_body = schema | |
| .timestamp_key_target_path() | |
| .and_then(|path| log.get(path)) | |
| .and_then(Value::as_timestamp) | |
| .and_then(timestamp_nanos) | |
| .or_else(|| { | |
| schema | |
| .timestamp_key() | |
| .and_then(|path| log.get((PathPrefix::Event, path))) | |
| .and_then(Value::as_timestamp) | |
| .and_then(timestamp_nanos) | |
| }); | |
| if let Some(path) = schema.message_key_target_path() { | |
| record.body = log.remove_prune(path, true).and_then(into_body); | |
| } | |
| // The timestamp key can also point into metadata. Many sources and decoders write it | |
| // there with its full target path, but the `opentelemetry` source writes it to the | |
| // event root, so read the metadata first and then the event root. | |
| let metadata_time = schema | |
| .timestamp_key_target_path() | |
| .filter(|path| path.prefix == PathPrefix::Metadata) | |
| .and_then(|path| log.remove(path)) | |
| .and_then(|value| into_timestamp_nanos(value).ok()); | |
| let (mut fields, _) = log.into_parts(); | |
| if let Some(path) = source_type_key { | |
| fields.remove(&path.path, true); | |
| } | |
| let extracted_time = metadata_time.or_else(|| { | |
| schema | |
| .timestamp_key() | |
| .and_then(|path| take(&mut fields, path, into_timestamp_nanos)) | |
| }); | |
| record.time_unix_nano = timestamp_before_body.or(extracted_time).unwrap_or_default(); |
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · Open Bits AI session
| #[test] | ||
| fn native_legacy_log_reads_metadata_timestamp_key() { | ||
| init_metadata_timestamp_key(); | ||
| let mut log = LogEvent::from("disk full"); |
There was a problem hiding this comment.
Enable the constructor feature for isolated crate tests
The new LogEvent::from(&str) calls cannot compile when opentelemetry-proto is tested independently: vector-core gates that implementation behind its test feature, which this crate does not enable. Workspace feature unification can hide the failure. Enable vector-core/test through a development dependency or replace all affected calls with the production constructor.
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · Open Bits AI session
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b9ab03d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let Some(path) = schema.message_key_target_path() { | ||
| record.body = log.remove_prune(path, true).and_then(into_body); |
There was a problem hiding this comment.
Remove nested source markers before extracting the body
When log_schema.source_type_key is nested beneath log_schema.message_key (for example, .payload.source_type beneath .payload), this removes the entire message into record.body before the later source-type removal runs against the remaining fields. Logs produced by sources that insert the marker at the configured path therefore send the internal source_type marker inside the OTLP body, despite the documented promise to omit it; strip the applicable marker from the extracted body as well.
Useful? React with 👍 / 👎.
Summary
The
otlpcodec now converts native Vector log events (without aresourceLogsfield) to OTLP log records instead of rejecting them. The mapping is the inverse of theopentelemetrysource decoding, for both log namespaces. Fields that have no OTLP slot, or that have the wrong type for one, are sent as log record attributes instead of being dropped.Traces are handled separately on top of #26590.
References
Vector configuration
How did you test this PR?
opentelemetrysource. Both received the expected body, timestamps, severity, trace ID, and resource and record attributes.Does this PR include user facing changes?
no-changeloglabel to this PR.