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

JSON: Accessing string from array leads to Unknown value type error #9179

Closed
svennergr opened this issue Apr 18, 2023 · 6 comments
Closed

JSON: Accessing string from array leads to Unknown value type error #9179

svennergr opened this issue Apr 18, 2023 · 6 comments
Labels
component/logql type/bug Somehing is not working as expected

Comments

@svennergr
Copy link
Contributor

Describe the bug
Given a JSON logline {"foo":["bar"]} and trying to access the bar string from it leads to a Unknown value type error.

You can see it here: https://grafana.com/docs/loki/latest/logql/analyzer/?query=%7C+json+baz%3D%22foo%5B0%5D%22&line%5B%5D=%7B%22foo%22%3A%5B%22bar%22%5D%7D
image

Using number arrays works as expected: https://grafana.com/docs/loki/latest/logql/analyzer/?query=%7C+json+baz%3D%22foo%5B0%5D%22&line%5B%5D=%7B%22foo%22%3A%5B1%5D%7D
image

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://grafana.com/docs/loki/latest/logql/analyzer/?query=%7C+json+baz%3D%22foo%5B0%5D%22&line%5B%5D=%7B%22foo%22%3A%5B%22bar%22%5D%7D

Expected behavior

String slices should be parsed as number slices.

Environment:

  • Loki: latest
@svennergr svennergr added the type/bug Somehing is not working as expected label Apr 18, 2023
@rgroothuijsen
Copy link
Contributor

This appears to be an oddity in the underlying JSON parser. When the string is either wrapped in a JSON object or in a 2-dimensional array, it suddenly does manage to parse.

@slvlirnoff
Copy link

Any news on this one? Looks very odd indeed.

@slvlirnoff
Copy link

slvlirnoff commented Sep 22, 2023

It works if the string can be parsed to a 'known' type, i.e. "21:12:12", "500"
See here, items 0 and 1 can be extracted but not "abc": https://grafana.com/docs/loki/latest/query/analyzer/?query=%7C+json+baz%3D%22foo%5B0%5D%22%2C+bazz%3D%22foo%5B1%5D%22%2C+bazzz%3D%22foo%5B2%5D%22&line%5B%5D=%7B%22foo%22%3A%5B%2221%3A12%3A00%22%2C+%22500%22%2C+%22abc%22%5D%7D

image

@slvlirnoff
Copy link

After more sleuthing, it seems to have been introduced by #8734 and would be related to buger/jsonparser#232

@jrgvf
Copy link

jrgvf commented Jan 25, 2024

Hi, any news?

paul1r added a commit that referenced this issue Feb 13, 2024
…#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](#9179)
#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](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](0d4416a)
@paul1r
Copy link
Contributor

paul1r commented Feb 13, 2024

Hello, apologies for the delay! I've integrated a fix today that should fix this issue. If it does not, please reopen this.

@paul1r paul1r closed this as completed Feb 13, 2024
rhnasc pushed a commit to inloco/loki that referenced this issue Apr 12, 2024
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/logql type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants