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.
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
Changes from 34 commits
3f2f30f
d00a835
51e0c62
f308e88
5d80a84
bd189d2
87d9929
eedd947
3fba3f7
3efdbec
ca8a2ec
460138c
26a374b
7362669
174f6d8
8d8ce19
018cce9
76824b2
59e272d
39b9cb4
e914547
5b0b4ba
dd78d3f
2537852
182eb37
64d0bd5
54a52f0
5bcdf7e
e24cd4c
74c1d77
f411f0d
10370e9
8f027f5
b7a5def
abd9159
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.