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

Unexpected zero value comparison operation error: binary literal has no digits #14993

Closed
gtk-grafana opened this issue Nov 18, 2024 · 4 comments · Fixed by #15217
Closed

Unexpected zero value comparison operation error: binary literal has no digits #14993

gtk-grafana opened this issue Nov 18, 2024 · 4 comments · Fixed by #15217
Labels
type/bug Somehing is not working as expected

Comments

@gtk-grafana
Copy link
Contributor

Describe the bug
Every comparison operation on a "duration" or "bytes" field requires a unit, except when the field is "bytes" and the value is 0.

To Reproduce
Steps to reproduce the behavior:

  1. Run this query: {service_name=`loki/loki`} | json | logfmt | drop __error__, __error_details__ | throughput>=0B
  2. Cry in the tub
  3. (workaround) delete the unit

Expected behavior
Users might be using variables within grafana, or frameworks like scenes to build dynamic dashboards, as such, one would expect the same query to run with different numeric inputs without needing to adjust the units for specific cases.
So I would expect {service_name=`loki/loki`} | json | logfmt | drop __error__, __error_details__ | throughput>=0B, and {service_name=`loki/loki`} | json | logfmt | drop __error__, __error_details__ | throughput>=1B, to both be valid logQL, however the former throws the binary literal has no digits error.

I understand that the throughput>=0B is not a good filter, and the workaround is to drop the unit or change the filter to throughput!="", but imagine an application like Explore logs tries to create some UI for helping users write comparison operation filters, for every (duration/bytes) comparison a unit is required, and users might be fiddling with values in the UI, and instead of removing a filter, they change the value to 0 temporarily. Currently I'd need to either code in a unit-less filter for the 0 case, or remove the filter entirely, both of which could confuse the end-user. Also since I'm using a variable value to concatenate the users selection for unit with the value in the logQL output, removing the unit from the value will reset their unit selection, upsetting the users expectation that whatever unit they entered in the filter will persist in the future when they make edits

TL;DR: This query would run without error:
{service_name=`loki/loki`} | json | logfmt | drop __error__, __error_details__ | throughput>=0B | throughput <10GB

Not a high priority, but hopefully a quick fix.

Environment:

  • Infrastructure: [e.g., Kubernetes, bare-metal, laptop]
  • Deployment tool: [e.g., helm, jsonnet]

Screenshots, Promtail config, or terminal output
If applicable, add any output to help explain your problem.

@gtk-grafana gtk-grafana added the type/bug Somehing is not working as expected label Nov 18, 2024
@rgroothuijsen
Copy link
Contributor

Looking in the source code, it turns out that '0b' is not actually interpreted as "0 bytes", but rather as the start of a binary number. Similarly, '0o' and '0x' can be used for octal and hexadecimal.

@trevorwhitney
Copy link
Collaborator

yeah, that's an interesting find @rgroothuijsen. So, I think our options here would be a) treat B differently than b (which feels really yucky to me, or b) we could enforce all queries with a unit like GB or MB must include a space before the unit, and then we can make sure we format it the same way in the response.

Any strong opinions?

@trevorwhitney
Copy link
Collaborator

trevorwhitney commented Nov 23, 2024

Image
So yeah, 1B appears to work, even though Grafana complains about it.

@trevorwhitney
Copy link
Collaborator

Some ideas brought up:

  • allow 0 to work for any type conversion
  • special case for 0b
  • promote parsing higher up in the syntax to have a different type at scan time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants