Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

felixbarny
Copy link
Contributor

@felixbarny felixbarny commented Jan 3, 2025

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.

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.
Copy link
Contributor

@carsonip carsonip left a 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")
Copy link
Contributor Author

@felixbarny felixbarny Jan 6, 2025

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?

Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants