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

feat(logs): parse JSON logs #2773

Merged
merged 1 commit into from
Jan 13, 2023
Merged

feat(logs): parse JSON logs #2773

merged 1 commit into from
Jan 13, 2023

Conversation

swiatekm
Copy link

@swiatekm swiatekm commented Jan 10, 2023

Parse JSON logs bodies into OTLP data structures. As a result, Sumo renders the JSON correctly. Without this change, JSON log bodies are just escaped strings.

This change is somewhat dangerous, and I'm wondering if we shouldn't make it possible to disable. The problem is that currently, if the JSON parsing returns an error, it'll result in a 500 returned to the collector, and potentially block the whole batch from being processed indefinitely. Discussion is ongoing in otel upstream about how the errors should be handled, but right now there doesn't seem to be a way to just ignore parse errors.

I've switched to logstransform processor, and this actually lets the malformed data through while logging an error. It seems to be buggy in 0.68.0 though, and fine in 0.69.0, so this PR shouldn't be merged until we upgrade to the latter.

@github-actions github-actions bot added the documentation documentation label Jan 10, 2023
@sumo-drosiek
Copy link
Contributor

Discussion is ongoing in open-telemetry/opentelemetry-collector-contrib#16519 about how the errors should be handled, but right now there doesn't seem to be a way to just ignore parse errors.

If so, we should make it configurable or move to separate processor eventually

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request contains invalid labels. Please remove all of the following labels: ['do-not-merge/hold']

@swiatekm swiatekm marked this pull request as ready for review January 11, 2023 15:34
@swiatekm swiatekm requested a review from a team as a code owner January 11, 2023 15:34
@swiatekm
Copy link
Author

Depends on #2791

@swiatekm swiatekm enabled auto-merge (rebase) January 13, 2023 12:50
@swiatekm swiatekm merged commit 29fc359 into main Jan 13, 2023
@swiatekm swiatekm deleted the feat/parse-json branch January 13, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants