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

fix(logs)!: move JSON parsing after user-defined processors #3281

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

swiatekm
Copy link

It's easier for users to write processors if they can assume the log body is a string, as opposed to having to work with arbitrary structured data.

Checklist

  • Changelog updated or skip changelog label added
  • Documentation updated
  • Template tests added for new features
  • Integration tests added or modified for major features

@swiatekm swiatekm requested a review from a team as a code owner September 19, 2023 10:50
@sumo-drosiek
Copy link
Contributor

It's easier for users to write processors if they can assume the log body is a string

Hmm, it is problematic if they would like to operate on the log structure 🤔

@swiatekm
Copy link
Author

It's easier for users to write processors if they can assume the log body is a string

Hmm, it is problematic if they would like to operate on the log structure 🤔

They can explicitly parse it from json themselves in that case. And going the other way is apparently impossible in OTTL, or at least I haven't found a way. All the body manipulation functions assume the body is a string and do nothing otherwise. If you want to do things like masking PII, working with the serialized JSON is much easier.

@swiatekm swiatekm force-pushed the fix/logs/json-parse-order branch from 4fc1878 to c254aaf Compare September 19, 2023 10:59
@andrzej-stencel
Copy link
Contributor

Isn't this a breaking change?

It's easier for users to write processors if they can assume the log
body is a string, as opposed to having to work with arbitrary structured
data.
@swiatekm swiatekm force-pushed the fix/logs/json-parse-order branch from c254aaf to 3956bfa Compare September 20, 2023 10:17
@swiatekm
Copy link
Author

swiatekm commented Sep 20, 2023

Isn't this a breaking change?

It's a fix, as the current behaviour is not intended, and the new behaviour will be better for the majority of users. However, some users may depend on the current behaviour. I think the change is beneficial enough in the aggregate that it should go into v3. I've added an extended note to the changelog and marked the change as breaking to make things clearer.

@swiatekm swiatekm changed the title fix(logs): move JSON parsing after user-defined processors fix(logs)!: move JSON parsing after user-defined processors Sep 20, 2023
@swiatekm swiatekm merged commit f419cb9 into main Sep 20, 2023
33 checks passed
@swiatekm swiatekm deleted the fix/logs/json-parse-order branch September 20, 2023 10:59
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