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

chore(parser.json_v2): Error out if no config is provided #15844

Merged
merged 8 commits into from
Sep 9, 2024

Conversation

mskonovalov
Copy link
Contributor

@mskonovalov mskonovalov commented Sep 4, 2024

Summary

Making json_v2 parser fail to initialize, if there are no configured fields or tags as there is no chance this parser will ever produce a metric in this state.

Checklist

  • No AI generated code was used in this PR

Related issues

superseeds #15840

@telegraf-tiger telegraf-tiger bot added area/json json and json_v2 parser/serialiser related chore plugin/parser 1. Request for new parser plugins 2. Issues/PRs that are related to parser plugins labels Sep 4, 2024
@srebhan srebhan self-assigned this Sep 4, 2024
@srebhan srebhan changed the title chore(parser.json_v2): fail json_v2 parser if no config chore(parser.json_v2): Error out if no config is provided Sep 4, 2024
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mskonovalov! Please remove the parser type from the error message as this will be automatically added by the logger. Furthermore, it would be nice to add a hint to this behavioral change in the CHANGELOG.md file (see this example).

plugins/parsers/json_v2/parser.go Outdated Show resolved Hide resolved
Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
@mskonovalov
Copy link
Contributor Author

mskonovalov commented Sep 5, 2024

there are a few tests that are failing which I may need some help fixing :)

@srebhan
Copy link
Member

srebhan commented Sep 5, 2024

I guess you just need to provide a rudimentary config there. Also please add a test for your new error!

@mskonovalov mskonovalov requested a review from srebhan September 7, 2024 23:18
plugins/parsers/json_v2/parser_test.go Outdated Show resolved Hide resolved
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@me.com>
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thank you very much @mskonovalov!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Sep 9, 2024
@srebhan srebhan assigned DStrand1 and unassigned srebhan Sep 9, 2024
@DStrand1 DStrand1 merged commit 53fb5ad into influxdata:master Sep 9, 2024
27 checks passed
@github-actions github-actions bot added this to the v1.32.0 milestone Sep 9, 2024
@mskonovalov mskonovalov deleted the jsonv2-empty-cfg branch September 11, 2024 13:34
asaharn pushed a commit to asaharn/telegraf that referenced this pull request Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/json json and json_v2 parser/serialiser related chore plugin/parser 1. Request for new parser plugins 2. Issues/PRs that are related to parser plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants