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(parsers.json_v2): allow optional paths and handle wrong paths correctly #10468

Merged
merged 5 commits into from
Feb 3, 2022

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Jan 19, 2022

resolve: #10072

  • Updated associated README.md.
  • Wrote appropriate unit tests.

The issue showed that when sending multiple different JSON payloads to be parsed by the json_v2 parser it will generate errors whenever the path doesn't match. The situation was created with using the mqtt_consumer plugin with wildcards, check the issue for an example. To overcome this scenario, this pull request introduces a new configuration optional that can be set for a configured object. This can be dangerous because the user won't know if the path is configured incorrectly, I did add a warning in the README and it will log something out with --debug.

I also cleaned up the handling when the gjson path is wrong and returns a more helpful error message, this was a regression for the objects but I noticed not all paths were being checked either.

Copy link
Contributor

@MyaLongmire MyaLongmire left a comment

Choose a reason for hiding this comment

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

Sorry, I didn't know mqtt wild cards would break the json parser 😅

@sspaink sspaink 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 Jan 24, 2022
@reimda
Copy link
Contributor

reimda commented Feb 1, 2022

In the issue you had the new setting called "optional" but it's "optional_path" here in the PR. Optional seems clearer to me. Did you prefer optional_path for some reason?

@sspaink
Copy link
Contributor Author

sspaink commented Feb 1, 2022

Both work for me, last minute I thought maybe optional_path was clearer seeing there are a lot of configurable settings under [[inputs.x.json_v2.object]] to avoid any confusion. I can change it to just "optional".

@influxdata influxdata deleted a comment from telegraf-tiger bot Feb 2, 2022
@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Feb 2, 2022

@reimda reimda merged commit 75946f5 into master Feb 3, 2022
@reimda reimda deleted the optionalpath branch February 3, 2022 22:08
pteich added a commit to pteich/telegraf that referenced this pull request Feb 13, 2022
* master: (117 commits)
  fix: bump github.com/nats-io/nats-server/v2 from 2.6.5 to 2.7.2 (influxdata#10638)
  chore: add -race flag to go tests (influxdata#10629)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10631)
  fix: license doc outdated causing CI failure (influxdata#10630)
  fix: bump k8s.io/client-go from 0.22.2 to 0.23.3 (influxdata#10589)
  feat: Implemented support for reading raw values, added tests and doc (influxdata#6501)
  fix: Improve parser tests by using go-cmp/cmp (influxdata#10497)
  feat(mongodb): add FsTotalSize and FsUsedSize informations (influxdata#10625)
  docs: update quay docs for auth (influxdata#10612)
  chore: allow downgrade of go version in windows script (influxdata#10614)
  chore: update CI go to 1.17.6 (influxdata#10611)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10600)
  fix(inputs.opcua): add more data to error log (influxdata#10465)
  fix: bump github.com/aws/aws-sdk-go-v2/service/kinesis from 1.6.0 to 1.13.0 (influxdata#10601)
  refactor: use early return pattern (influxdata#10591)
  fix: bump github.com/benbjohnson/clock from 1.1.0 to 1.3.0 (influxdata#10588)
  feat: add dynamic tagging to gnmi plugin (influxdata#7484)
  fix: bump github.com/Azure/azure-kusto-go from 0.5.0 to 0.5.2 (influxdata#10598)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10584)
  fix(parsers.json_v2): allow optional paths and handle wrong paths correctly (influxdata#10468)
  ...

# Conflicts:
#	plugins/outputs/elasticsearch/elasticsearch.go
#	plugins/outputs/elasticsearch/elasticsearch_test.go
powersj pushed a commit that referenced this pull request Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug 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.

json_v2 parser with wildcard mqtt topics produces lot of "GJSON Path returned null" errors
3 participants