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

(prometheus) get label name/value from series API #12251

Merged
merged 2 commits into from
Sep 13, 2018

Conversation

mtanda
Copy link
Collaborator

@mtanda mtanda commented Jun 13, 2018

instant query doesn't return all label name/value of dashboard time range.
So, change to use series API.

https://prometheus.io/docs/prometheus/latest/querying/api/#finding-series-by-label-matchers

@mtanda mtanda force-pushed the prometheus_use_matchers_for_completion branch 5 times, most recently from 2d9622c to 4bc735c Compare June 13, 2018 07:00
@davkal davkal self-requested a review July 26, 2018 09:53
Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Very useful addition!

Small implementation issue: We're trying to be less reliant on angular services, and hence want to prevent their leakage into plugin submodules. Could you add a function to the datasource instead that returns {start, end}?

@davkal davkal self-assigned this Jul 26, 2018
@mtanda mtanda force-pushed the prometheus_use_matchers_for_completion branch from 37741e1 to 1f3c39f Compare July 26, 2018 16:49
@mtanda
Copy link
Collaborator Author

mtanda commented Jul 26, 2018

Okay, I change the code :-)

Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Almost ready:

  • Please use const { start, end } = this.getTimeRange(); within datasource.ts as well now when start and end are requested. Then this can be merged.
  • Please add types to all functions you added/modified.

return this.datasource.performInstantQuery({ expr: query }, new Date().getTime() / 1000).then(response => {
this.labelQueryCache[expr] = response.data.data.result;
return response.data.data.result;
let range = this.datasource.getTimeRange();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be const { start, end } = this.datasource.getTimeRange();
Also, url below should be const.

@mtanda
Copy link
Collaborator Author

mtanda commented Jul 27, 2018

@davkal

Please add types to all functions you added/modified.

Should I add types like session or pos which provided by Ace editor?
I think set any type is meaningless, do you want to set more strict type?

@davkal
Copy link
Contributor

davkal commented Aug 27, 2018

The types are good as is now. Thank you. Could please rebase? Then we can merge.

@mtanda mtanda force-pushed the prometheus_use_matchers_for_completion branch from 5202c16 to bf88402 Compare August 27, 2018 16:13
@mtanda
Copy link
Collaborator Author

mtanda commented Aug 27, 2018

rebased.

Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@davkal davkal merged commit e449608 into grafana:master Sep 13, 2018
@davkal davkal added this to the 5.4 milestone Sep 13, 2018
@marefr marefr modified the milestones: 5.4, 5.3 Oct 1, 2018
@ying-jeanne ying-jeanne added the pr/external This PR is from external contributor label Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/external This PR is from external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants