Add OTLP logs endpoint implementation#147446
Merged
Merged
Conversation
The endpoint is still behind a feature flag. Some leftovers - Handling of geo ip fields - Preserving nanosecond precision of timestamps - Store trace flags - bodymap mapping mode
Restore shared trace and span ID helpers needed by the rebased logs implementation and keep the module formatting checks green.
Match the upstream OTLP log serializer for timestamps, mixed-array body wrapping, and control-attribute omission so the initial logs implementation stays compatible with the collector's otel mapping.
elasticsearchmachine
added
needs:triage
Collaborator
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
elasticsearchmachine
removed
the
needs:triage
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
WalkthroughIntroduces OpenTelemetry log indexing capabilities with REST integration tests validating batch indexing, field mapping, data stream routing, and scope handling. Enhances LogDocumentBuilder to derive timestamps, severity, event names, span/trace IDs, and handle complex body structures. Extends OTelDocumentBuilder for additional AnyValue variants and ID serialization helpers. Adds comprehensive unit test coverage. Changes
Sequence DiagramsequenceDiagram
participant Test as Test Runner
participant Logger as OTLP Logger Provider
participant Builder as LogDocumentBuilder
participant ES as Elasticsearch
Test->>Logger: emit log record (body, attributes, resource, scope)
Logger->>Logger: accumulate in provider
Test->>Test: call indexLogs()
Test->>Builder: convert LogRecord to doc
Builder->>Builder: extract & format timestamps
Builder->>Builder: map severity, event_name, span_id, trace_id
Builder->>Builder: handle body (text/kvlist/array)
Builder->>Builder: flatten attributes & scope
Test->>ES: index document
ES->>ES: store fields in data stream
Test->>ES: search via ObjectPath
ES-->>Test: return indexed document
Test->>Test: assert field values
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.java (2)
68-73: Minor: redundant emptiness guards.
addSpanId/addTraceIdalready checklength > 0, so theisEmpty()guards here are redundant. Dropping them also avoids thetoByteArray()copy when the id is empty (thoughByteString.toByteArray()is cheap for empty, the double-guard just adds noise).♻️ Proposed simplification
- if (logRecord.getSpanId().isEmpty() == false) { - addSpanId(builder, logRecord.getSpanId().toByteArray()); - } - if (logRecord.getTraceId().isEmpty() == false) { - addTraceId(builder, logRecord.getTraceId().toByteArray()); - } + addSpanId(builder, logRecord.getSpanId().toByteArray()); + addTraceId(builder, logRecord.getTraceId().toByteArray());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.java` around lines 68 - 73, In LogDocumentBuilder, remove the redundant isEmpty() checks around logRecord.getSpanId() and logRecord.getTraceId() and call addSpanId(builder, logRecord.getSpanId().toByteArray()) and addTraceId(builder, logRecord.getTraceId().toByteArray()) unguarded; addSpanId/addTraceId already ignore zero-length ids (length > 0), so this eliminates the unnecessary double-guard and avoids an extra empty ByteString handling path while keeping behavior unchanged.
88-124: Recommended: consolidate with the base class to avoid near-duplicate resource/scope/attributes helpers.
buildLogResourceandbuildLogScopeare structural duplicates ofOTelDocumentBuilder#buildResourceand#buildScope; they only differ by callingbuildLogAttributesinstead ofbuildAttributes. Similarly,buildLogAttributesisbuildAttributeswith (a) an extra ignore predicate and (b) lazy opening of the"attributes"object.Consider lifting this into the base class — e.g., by letting
buildAttributesaccept anIntPredicate/Predicate<String>of additional ignored keys and by making the lazy-open behavior the default. That removes the log-specific override path entirely and keeps metrics/traces aligned if they later need similar filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.java` around lines 88 - 124, The log-specific helpers buildLogResource, buildLogScope, and buildLogAttributes duplicate OTelDocumentBuilder#buildResource/#buildScope/#buildAttributes; refactor by changing OTelDocumentBuilder#buildAttributes to accept an additional ignore predicate (e.g., Predicate<String> or IntPredicate for attribute keys) and to do lazy-opening of the "attributes" object, then have buildResource/buildScope call this generalized buildAttributes (passing a no-op predicate for non-log callers) and replace buildLogResource/buildLogScope to delegate to the base implementations (or remove them); update any callers to pass the log-specific ignore predicate (isIgnoredLogAttribute) so the log-only filtering behavior is preserved without duplicating structure.x-pack/plugin/otel-data/src/test/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilderTests.java (1)
40-333: Recommended: add unit coverage forspan_id/trace_idemission.The new
addSpanId/addTraceIdpath is only exercised via callers inLogDocumentBuilder, but there's no unit test here that verifies (a) the hex encoding and (b) that the fields are omitted when the respective ByteString is empty. A small test with a non-emptysetSpanId(ByteString.copyFrom(...))/setTraceId(...)plus a negative assertion when absent would close this gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/plugin/otel-data/src/test/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilderTests.java` around lines 40 - 333, Add unit tests in LogDocumentBuilderTests that construct LogRecord instances with non-empty span_id and trace_id bytes (use ByteString.copyFrom with known bytes), call buildDocument(...) and assert the emitted fields (e.g., "trace.id" and "span.id") contain the expected hex-encoded strings; also add cases where setSpanId(ByteString.EMPTY)/setTraceId(ByteString.EMPTY) are used and assert those fields are omitted (null) from the ObjectPath. Locate test helpers buildDocument(Resource, InstrumentationScope, LogRecord) and the class under test LogDocumentBuilder to plug the new assertions; ensure assertions check hex encoding exactly and negative assertions for absence.x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/OTelDocumentBuilder.java (1)
129-139: Minor: cacheHexFormat.of()as a constant and consider consolidating the two helpers.
HexFormat.of()is invoked on every call; while it returns a cached instance internally, binding it to aprivate static final HexFormat HEX = HexFormat.of();is idiomatic and avoids repeated method dispatch on hot paths.addSpanIdandaddTraceIdalso differ only in the field name, so they could be reduced to a single helper.♻️ Proposed refactor
+ private static final HexFormat HEX = HexFormat.of(); + protected void addSpanId(XContentBuilder builder, byte[] spanId) throws IOException { - if (spanId.length > 0) { - builder.field("span_id", HexFormat.of().formatHex(spanId)); - } + addHexIdIfNotEmpty(builder, "span_id", spanId); } protected void addTraceId(XContentBuilder builder, byte[] traceId) throws IOException { - if (traceId.length > 0) { - builder.field("trace_id", HexFormat.of().formatHex(traceId)); - } + addHexIdIfNotEmpty(builder, "trace_id", traceId); + } + + private static void addHexIdIfNotEmpty(XContentBuilder builder, String field, byte[] id) throws IOException { + if (id.length > 0) { + builder.field(field, HEX.formatHex(id)); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/OTelDocumentBuilder.java` around lines 129 - 139, Cache HexFormat.of() as a class constant and consolidate the two near-identical helpers: add a private static final HexFormat HEX = HexFormat.of() in OTelDocumentBuilder, replace HexFormat.of().formatHex(...) calls with HEX.formatHex(...), and collapse addSpanId/addTraceId into a single helper (e.g., addHexId(XContentBuilder builder, byte[] id, String fieldName)) that both original methods call (or replace them directly) to write the field name ("span_id" / "trace_id") only when id.length > 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.java`:
- Around line 82-86: The addLogTimestampField method writes a millisecond epoch
plus a sub-millisecond nanoseconds remainder without zero-padding, corrupting
timestamps; fix addLogTimestampField by formatting nanosRemainder as a
zero-padded 6-digit decimal fraction before calling builder.field(fieldName,
...), e.g. convert nanosRemainder to a 6-digit string (use
org.elasticsearch.common.Strings.padLeft or equivalent padding) and concatenate
millis + "." + paddedRemainder so values like 1 or 123 become "000001" /
"000123"; update imports to include org.elasticsearch.common.Strings and add a
unit test case in testTimestampPreservesSubMillisecondPrecision that asserts
behavior for remainders 1 and 123.
- Around line 134-171: The buildBody method in LogDocumentBuilder treats an
empty ARRAY_VALUE as structured (allMaps stays true) and writes an empty array;
change it to short-circuit empty arrays the same way VALUE_NOT_SET does so the
body object is omitted. Specifically, in buildBody (where you inspect AnyValue
body and valueCase ARRAY_VALUE), detect if body.getArrayValue().getValuesCount()
== 0 and return immediately before starting the "body" object; keep existing
logic for non-empty arrays (calling buildStructuredBody or wrapping primitives
with buildAnyValue) and unchanged paths for KVLIST_VALUE and default
(buildTextBody).
In
`@x-pack/plugin/otel-data/src/test/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilderTests.java`:
- Around line 250-260: Add a second assertion in
testTimestampPreservesSubMillisecondPrecision that uses a LogRecord whose
timeUnixNano has a sub-millisecond remainder with fewer than 6 digits (e.g., use
1_000_000_001L or 1_000_000_123L), build the document via buildDocument and
assert `@timestamp` formats with zero-padded micro/nan digits (e.g., "1000.000001"
or "1000.000123"); this exercises addLogTimestampField’s padding logic to ensure
sub-millisecond remainders are left-padded to six digits and observed_timestamp
remains "0.0".
---
Nitpick comments:
In
`@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.java`:
- Around line 68-73: In LogDocumentBuilder, remove the redundant isEmpty()
checks around logRecord.getSpanId() and logRecord.getTraceId() and call
addSpanId(builder, logRecord.getSpanId().toByteArray()) and addTraceId(builder,
logRecord.getTraceId().toByteArray()) unguarded; addSpanId/addTraceId already
ignore zero-length ids (length > 0), so this eliminates the unnecessary
double-guard and avoids an extra empty ByteString handling path while keeping
behavior unchanged.
- Around line 88-124: The log-specific helpers buildLogResource, buildLogScope,
and buildLogAttributes duplicate
OTelDocumentBuilder#buildResource/#buildScope/#buildAttributes; refactor by
changing OTelDocumentBuilder#buildAttributes to accept an additional ignore
predicate (e.g., Predicate<String> or IntPredicate for attribute keys) and to do
lazy-opening of the "attributes" object, then have buildResource/buildScope call
this generalized buildAttributes (passing a no-op predicate for non-log callers)
and replace buildLogResource/buildLogScope to delegate to the base
implementations (or remove them); update any callers to pass the log-specific
ignore predicate (isIgnoredLogAttribute) so the log-only filtering behavior is
preserved without duplicating structure.
In
`@x-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/OTelDocumentBuilder.java`:
- Around line 129-139: Cache HexFormat.of() as a class constant and consolidate
the two near-identical helpers: add a private static final HexFormat HEX =
HexFormat.of() in OTelDocumentBuilder, replace HexFormat.of().formatHex(...)
calls with HEX.formatHex(...), and collapse addSpanId/addTraceId into a single
helper (e.g., addHexId(XContentBuilder builder, byte[] id, String fieldName))
that both original methods call (or replace them directly) to write the field
name ("span_id" / "trace_id") only when id.length > 0.
In
`@x-pack/plugin/otel-data/src/test/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilderTests.java`:
- Around line 40-333: Add unit tests in LogDocumentBuilderTests that construct
LogRecord instances with non-empty span_id and trace_id bytes (use
ByteString.copyFrom with known bytes), call buildDocument(...) and assert the
emitted fields (e.g., "trace.id" and "span.id") contain the expected hex-encoded
strings; also add cases where
setSpanId(ByteString.EMPTY)/setTraceId(ByteString.EMPTY) are used and assert
those fields are omitted (null) from the ObjectPath. Locate test helpers
buildDocument(Resource, InstrumentationScope, LogRecord) and the class under
test LogDocumentBuilder to plug the new assertions; ensure assertions check hex
encoding exactly and negative assertions for absence.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Enterprise
Run ID: 68bb2107-b8a9-458e-91a7-1d85ae4e6190
📒 Files selected for processing (4)
x-pack/plugin/otel-data/src/javaRestTest/java/org/elasticsearch/xpack/oteldata/otlp/OTLPLogsIndexingRestIT.javax-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilder.javax-pack/plugin/otel-data/src/main/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/OTelDocumentBuilder.javax-pack/plugin/otel-data/src/test/java/org/elasticsearch/xpack/oteldata/otlp/docbuilder/LogDocumentBuilderTests.java
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
3 tasks
Preserve sub-millisecond log timestamp precision and omit empty array bodies so OTLP log documents stay aligned with upstream serialization behavior.
kkrik-es
reviewed
Apr 27, 2026
kkrik-es
reviewed
Apr 27, 2026
Member
kkrik-es
left a comment
kkrik-es
left a comment
There was a problem hiding this comment.
Maybe someone familiar with OTel Logs can review as well?
Use the common OTLP document builder to filter shared control attributes so logs and future signals serialize them consistently.
Share timestamp formatting between log and span builders, use numeric epoch millis when possible, and serialize unspecified span kind explicitly.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Summary
otelmapping mode for timestamps, mixed-array bodies, and control attribute filteringFollow-ups
geo.locationarrays for log documents Merge OTLP geo attributes for traces and logs #147802otelmode for self-telemetry,encoding.format, and connector or receiver-based routing Align OTLP routing with ES exporter #147461elasticsearch.ingest_pipelineas request pipeline metadata for logs, and stop serializing it once that handling exists Honor OTLP log document metadata #147799