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

Add support for Query Range String: Change step to string and support standard time.Duration parsing #1211

Closed
wants to merge 1 commit into from

Conversation

nehayward
Copy link

What this PR does / why we need it:
This changes Query Range to a string to support step changes.

Which issue(s) this PR fixes:
Fixes #1209

Special notes for your reviewer:
This still needs some refinement and I wanted to get some eyes on it from the Loki community before moving forward with the changes.

Checklist

  • Documentation added
  • Tests updated -- (in progress)

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2019

CLA assistant check
All committers have signed the CLA.

@cyriltovena
Copy link
Contributor

Thanks a lot for this, don’t forget we need to support both format.

See https://prometheus.io/docs/prometheus/latest/querying/api/#range-queries

step=<duration | float>: Query resolution step width in duration format or float number of seconds

@nehayward
Copy link
Author

Any suggestion on how you'd like to do that. Should add another optional arg

@cyriltovena
Copy link
Contributor

No the same argument just try converting to integer if it fails try parsing a duration if it fails it fails ;)

@cyriltovena
Copy link
Contributor

@nehayward any news, I might need this too.

@nehayward
Copy link
Author

Let me update my PR.

@nehayward nehayward closed this Nov 19, 2019
@ghost
Copy link

ghost commented Nov 22, 2019

It seems loki still doesn't support float numbers. I got error when I run query via Grafana.

rpc error: code = Code(400) desc = strconv.Atoi: parsing "54.982": invalid syntax

I'm using the latest docker image for both grafana and Loki.
For reference:
`
version: "3"

networks:
loki:

services:
loki:
image: grafana/loki:v1.0.0
ports:
- "3100:3100"
volumes:
- /data/loki/conf:/data/loki/conf
command: -config.file=/data/loki/conf/loki.yaml
networks:
- loki

grafana:
image: grafana/grafana:master
ports:
- "3000:3000"
networks:
- loki
`

@cyriltovena
Copy link
Contributor

yes @nehayward do you want me to take over ?

@cyriltovena
Copy link
Contributor

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query range step does not support duration string.
3 participants