-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[exporter/elasticsearch] OTel mode serialization #33290
[exporter/elasticsearch] OTel mode serialization #33290
Conversation
Functionally, the PR looks good to me. The only missing piece is the data stream routing. Maybe a test case with complex (map, list) attributes could be useful. I'll defer the actual code review to folks more familiar with go and the code base. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Updated PR based on the some code review feedback. Added the data_stream routing support, that was initially implemented in Josh's POC per reviewer request. |
Don't have permissions to remove the |
#33794 is doing some changes wrt. routing. After that is merged, please merge in the changes and make sure the same routing mechanism is used. Let us know if you think that would work. |
Yeah, that why I mentioned in DM before that I didn't want to add the routing since the work was happening in that area. It's easy to rollback. |
@@ -245,19 +245,19 @@ func (doc *Document) Dedup() { | |||
// Serialize writes the document to the given writer. The serializer will create nested objects if dedot is true. | |||
// | |||
// NOTE: The documented MUST be sorted if dedot is true. | |||
func (doc *Document) Serialize(w io.Writer, dedot bool) error { | |||
func (doc *Document) Serialize(w io.Writer, dedot bool, otel bool) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to change objmodel for otel mode? Is there a reason why it isn't done like https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/elasticsearchexporter/model.go#L124 where we just add the attributes under a key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I remember, at the time of implementing this feature, I stumbled on the issue where the document could only be serialized as completely flat from the root or "dedotted" (by default https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/elasticsearchexporter/factory.go#L80), all the comma separated properties were unwrapped into the nested object, which was not stored correctly "flattened" with the passthrough attributes in ES.
With OTel-native serialization the serialized document structure is a mix, needed to keep only the ".attributes*" flattened, while the rest of the document from the root is not flattened.
Let me know if anything changed in that area recently, this PR was open for some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context. I'm just learning about how passthrough field type works, and what you describe here makes sense. I wonder if it could be structured better in the code, but I don't have any good ideas at the moment and it is not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it makes sense to revisit this once we remove the dedot option: #33772.
Co-authored-by: Felix Barnsteiner <[email protected]>
Accepted the README suggestion, merged the latest main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! changelog nit, otherwise lgtm
Co-authored-by: Carson Ip <[email protected]>
Co-authored-by: Carson Ip <[email protected]>
Accepted the suggestions, resolved the latest set of conflicts with main. |
Please resolve CI failures and merge conflict |
Resolved conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=== Failed
=== FAIL: internal/objmodel TestDocument_Serialize_Otel/otel (0.00s)
objmodel_test.go:387:
Error Trace: /home/carson/projects/opentelemetry-collector-contrib/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go:387
Error: Not equal:
expected: "{\"@timestamp\":\"2024-03-18T21:09:53.645578000Z\",\"attributes\":{\"auditd.log.op\":\"PAM:session_open\",\"auditd.log.record_type\":\"USER_START\",\"auditd.log.sequence\":6082,\"auditd.log.subj\":\"unconfined\",\"auditd.log.uid\":\"1000\"},\"resource\":{\"attributes\":{\"blah.num\":234,\"blah.str\":\"something\"}},\"scope\":{\"attributes\":{\"bar.one\":\"boo\",\"foo.two\":\"bar\"}}}"
actual : "{\"@timestamp\":\"2024-03-18T21:09:53.645578000Z\",\"attributes\":{\"auditd.log.op\":\"PAM:session_open\",\"auditd.log.record_type\":\"USER_START\",\"auditd.log.sequence\":6082,\"auditd.log.subj\":\"unconfined\",\"auditd.log.uid\":\"1000\"},\"resource\":{\"attributes\":{\"blah\":{\"num\":234,\"str\":\"something\"}}},\"scope\":{\"attributes\":{\"bar\":{\"one\":\"boo\"},\"foo\":{\"two\":\"bar\"}}}}"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-{"@timestamp":"2024-03-18T21:09:53.645578000Z","attributes":{"auditd.log.op":"PAM:session_open","auditd.log.record_type":"USER_START","auditd.log.sequence":6082,"auditd.log.subj":"unconfined","auditd.log.uid":"1000"},"resource":{"attributes":{"blah.num":234,"blah.str":"something"}},"scope":{"attributes":{"bar.one":"boo","foo.two":"bar"}}}
+{"@timestamp":"2024-03-18T21:09:53.645578000Z","attributes":{"auditd.log.op":"PAM:session_open","auditd.log.record_type":"USER_START","auditd.log.sequence":6082,"auditd.log.subj":"unconfined","auditd.log.uid":"1000"},"resource":{"attributes":{"blah":{"num":234,"str":"something"}}},"scope":{"attributes":{"bar":{"one":"boo"},"foo":{"two":"bar"}}}}
Test: TestDocument_Serialize_Otel/otel
=== FAIL: internal/objmodel TestDocument_Serialize_Otel (0.00s)
=== FAIL: internal/objmodel TestDocument_Serialize_Otel/otel (re-run 1) (0.00s)
objmodel_test.go:387:
Error Trace: /home/carson/projects/opentelemetry-collector-contrib/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go:387
Error: Not equal:
expected: "{\"@timestamp\":\"2024-03-18T21:09:53.645578000Z\",\"attributes\":{\"auditd.log.op\":\"PAM:session_open\",\"auditd.log.record_type\":\"USER_START\",\"auditd.log.sequence\":6082,\"auditd.log.subj\":\"unconfined\",\"auditd.log.uid\":\"1000\"},\"resource\":{\"attributes\":{\"blah.num\":234,\"blah.str\":\"something\"}},\"scope\":{\"attributes\":{\"bar.one\":\"boo\",\"foo.two\":\"bar\"}}}"
actual : "{\"@timestamp\":\"2024-03-18T21:09:53.645578000Z\",\"attributes\":{\"auditd.log.op\":\"PAM:session_open\",\"auditd.log.record_type\":\"USER_START\",\"auditd.log.sequence\":6082,\"auditd.log.subj\":\"unconfined\",\"auditd.log.uid\":\"1000\"},\"resource\":{\"attributes\":{\"blah\":{\"num\":234,\"str\":\"something\"}}},\"scope\":{\"attributes\":{\"bar\":{\"one\":\"boo\"},\"foo\":{\"two\":\"bar\"}}}}"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-{"@timestamp":"2024-03-18T21:09:53.645578000Z","attributes":{"auditd.log.op":"PAM:session_open","auditd.log.record_type":"USER_START","auditd.log.sequence":6082,"auditd.log.subj":"unconfined","auditd.log.uid":"1000"},"resource":{"attributes":{"blah.num":234,"blah.str":"something"}},"scope":{"attributes":{"bar.one":"boo","foo.two":"bar"}}}
+{"@timestamp":"2024-03-18T21:09:53.645578000Z","attributes":{"auditd.log.op":"PAM:session_open","auditd.log.record_type":"USER_START","auditd.log.sequence":6082,"auditd.log.subj":"unconfined","auditd.log.uid":"1000"},"resource":{"attributes":{"blah":{"num":234,"str":"something"}}},"scope":{"attributes":{"bar":{"one":"boo"},"foo":{"two":"bar"}}}}
Test: TestDocument_Serialize_Otel/otel
Description:
Implements OTel (OpenTelemetry-native) mode serialization for elasticsearch exporter.
This is an initial cut in order to get the discussion going.
This is approach was tested as internal POC.
It leverages Elasticsearch
"passthrough"
fields mapping initially introduced in Elasticsearch 8.13 allowing users to query the document/scope/resources attributes as top level fields, making the ECS queries compatible with OTel sematic convention schema. Another benefit is the simplicity of conversion of stored document from Elasticsearch back to Otel data model format.The document/scope/resources attributes are dynamically mapped and stored as flattened keys.
Here is an example of index template mappings with
"passthrough"
fields:Here is an example of the auditd document in Elasticsearch abbreviated:
Here is an example of ECS compatible query that works on this Otel native schema:
Link to tracking Issue:
No tracking issue yet.
Testing:
Added unit test for OTel transformation.
Tested with journald OTel receiver.
Documentation:
No documentation is added yet.