-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Parse JSON String arrays properly so string elements can be retrieved #11921
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Can you add the unit tests from https://github.com/grafana/loki/pull/10690/files?
operator/.bingo/go.mod
Outdated
module _ // Fake go.mod auto-created by 'bingo' for go -moddir compatibility with non-Go projects. Commit this file, together with other .mod files. | ||
|
||
go 1.21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) Perhaps we shouldn't change operator files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can revert, this was picked up through one of the build commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We'll need a CHANGELOG entry though as this is user visible.
…erly so string elements can be retrieved (#11931) Add PR #11921 to release notes for next release --------- Co-authored-by: Paul Rogers <[email protected]>
…grafana#11921) **What this PR does / why we need it**: This PR imports the newly forked grafana/jsonparser over the buger/jsonparser module. The latter has seemingly been abandoned. PR 10690 introduces a fix to the jsonparser module, which has been incorporated into the grafana fork of the module. The PR is designed to fix accessing string array elements from within a JSON structure. For example, with the following JSON: `{"log":{"message":{"content":{"misses":["a","b","c","d"]}}}}` The Loki code, before this PR, when searching for `json misses = "log.message.content.misses[0]" ` will result in an "Unknown value type error". After this PR is merged, the result will assign `a` to the `misses` variable. **Which issue(s) this PR fixes**: Fixes #[9179](grafana#9179) grafana#10690 **Special notes for your reviewer**: **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [x] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](grafana@d10549e) - [ ] If the change is deprecating or removing a configuration option, update the `deprecated-config.yaml` and `deleted-config.yaml` files respectively in the `tools/deprecated-config-checker` directory. [Example PR](grafana@0d4416a)
…ys properly so string elements can be retrieved (grafana#11931) Add PR grafana#11921 to release notes for next release --------- Co-authored-by: Paul Rogers <[email protected]>
What this PR does / why we need it:
This PR imports the newly forked grafana/jsonparser over the buger/jsonparser module. The latter has seemingly been abandoned. PR 10690 introduces a fix to the jsonparser module, which has been incorporated into the grafana fork of the module.
The PR is designed to fix accessing string array elements from within a JSON structure. For example, with the following JSON:
{"log":{"message":{"content":{"misses":["a","b","c","d"]}}}}
The Loki code, before this PR, when searching for
json misses = "log.message.content.misses[0]"
will result in an "Unknown value type error". After this PR is merged, the result will assigna
to themisses
variable.Which issue(s) this PR fixes:
Fixes #9179
#10690
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updatedadd-to-release-notes
labeldocs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR