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

[DRAFT] #2529 Fix empty log sended if no raw body #2546

Closed
wants to merge 4 commits into from

Conversation

gillg
Copy link
Contributor

@gillg gillg commented Mar 3, 2021

Description:
Final plan to discuss (still in WIP for this reason).
For now this PR is working as expected, all logs sended from OTEL collector to Loki will be encapsulated in a JSON containing all OTEL attributes at its root level.
If you have a OTEL log body it wil be appened to the entry _otel_raw_log, and same thing for OTEL Name and OTEL Severity (in _otel_shortname and _otel_severity).

JSON Format is ok ?
Idea to have all attributes as top level is ok ?

Link to tracking Issue: #2529

Testing: Tested localy with a fluentbit windows exporter > OTEL fluentforward > OTEL Loki exporter > Loki

Documentation: Potential log format breaking change on Loki side, if you had previously raw unstructured logs exported they will be in a global structure in key _otel_raw_log. But all your OTEL attributes will be at the root level of this main JSON structure

@gillg gillg requested a review from a team March 3, 2021 20:13
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 3, 2021

CLA Signed

The committers are authorized under a signed CLA.

@gramidt
Copy link
Member

gramidt commented Mar 3, 2021

@gillg - Could you make this PR a DRAFT, since it's a WIP?

@gramidt
Copy link
Member

gramidt commented Mar 3, 2021

Also, let's continue the discussion on the issue you created prior to finishing this PR, so we can ensure that we are all aligned on the solution. Thank you again for all of your help!

#2529

@gillg gillg changed the title [WIP] #2529 Fix empty log sended if no raw body [DRAFT] #2529 Fix empty log sended if no raw body Mar 3, 2021
@tigrannajaryan tigrannajaryan marked this pull request as draft March 5, 2021 00:16
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 12, 2021
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Mar 19, 2021
@gillg
Copy link
Contributor Author

gillg commented Apr 8, 2021

What are we doing about that ?

I disscussed about mandatory fields or not on the stable opentelemetry-collector project, and body, attributes are optional in the spec. So in my opinion we really should adapt sended fields to match with loki standards.
In loki we have "labels" and "log message" that's all. Both are separated, and independant, it means you can have a label "severity" without been present in the log itself.

By security we can't imagine automate to put attributes on labels, the curent approach of mapping is interesting and could be improved in the future. We could also do the same thing for "resources" in the future.

So in my opinion, push all attributes as a serialized JSON to loki, adding other top-level attributes _otel_XXXX in the map could be a not too dirty solution.

ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Update go.opentelemetry.io/proto/otlp to v0.12.0

* Changelog

* Update CHANGELOG.md

Fix's md linting

Co-authored-by: Tyler Yahn <[email protected]>

Co-authored-by: Aaron Clawson <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants