-
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
[elasticsearchexporter] Support for complex attributes for log records in OTel mode #37021
base: main
Are you sure you want to change the base?
[elasticsearchexporter] Support for complex attributes for log records in OTel mode #37021
Conversation
According to the spec: The log attribute model MUST support any type, a superset of standard Attribute, to preserve the semantics of structured attributes emitted by the applications. This field is optional.
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.
Nice catch, thanks!
|
||
logs := plog.NewLogs() | ||
resourceLog := logs.ResourceLogs().AppendEmpty() | ||
resourceLog.Resource().Attributes().PutEmptyMap("some.resource.attribute").PutEmptyMap("foo").PutStr("bar", "baz") |
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.
While this kinda works, there are some limitations due to the way obj model gets serialized and de-dotted in OTel mode. The test would fail if, for example changing to
resourceLog.Resource().Attributes().PutEmptyMap("some.resource.attribute").PutEmptyMap("foo.bar").PutStr("bar", "baz")
I think we should merge the change despite this limitation but think about longer-term alternatives.
I'm currently doing a POC of serializing pdata events directly to JSON for the OTel mode, without first converting them do an objmodel.Document
. This seems to work out quite well so far and I'd also suspect it to yield performance improvements due to one less layer of abstraction and avoiding the allocation of temporary objects. WDYT of that approach?
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.
Here's the draft PR for that POC: #37032
According to the spec: