Review adjustments to `smartquery` and `query_factory` #38

peekjef72 · 2022-09-19T17:23:58Z

About

I've tried to use query methods from grafana-client info grafana-snapshot-tool. For that I need to perform some adjustement to api interface.

extend query_factory to be able to set more parameters for each "known" datasource according to their parameters. Goal it to use parameters set in the panel mixed with datasource requirements and format to generate a request object (method, uri fragments, data to Post)
review examples datasource-smartquery.py and datasource-health-check.py to ensure they still work.

Discussion

it seems to me that :

datasource accesses change with grafana version: Prometheus in v 7 use format specified in function query() and query_range(), but now use a standard format for the queries (type or range) and results.
the format of the results changes from one datasource to anothers, maybe for grafana vesion too.
some of your tests (Loki ) seems not working for me : format my have change... not sure.

I'm not sure that the grafana_client must implement a generic function smartquery() that permit to users to query and version to Grafana server and any data source in a unified way.
What is your opinion ?

…o panodata-main

now returns dict containing all info to request Grafana API.

fix smart_query for check_health

amotl · 2022-10-17T21:43:38Z

Dear Jean-Francois,

thanks a stack for your excellent contribution and for bringing up the corresponding discussion topics.

I've tried to use query methods from grafana-client in grafana-snapshot-tool. For that I need to perform some adjustments to the API interface.

grafana-client should definitively be able to support grafana-snapshot-tool in the way you are proposing. It is very sweet to see you have been able to build upon my recent additions at grafana_client.elements.datasource, where you certainly improved some important details in order to make it more generic. This is the place where I had to stop on the last iteration, so I am exceptionally happy about it.

I will try to dedicate some time for a detailed review of your patch soon. I think your recent updates, especially removing smartquery_old, makes the overall appearance much more clear now.

With kind regards,
Andreas.

amotl · 2023-05-03T17:57:00Z

grafana_client/elements/datasource.py

+        elif datasource_type in ("prometheus", "loki") and LooseVersion(self.api.version) <= VERSION_7:
+            if "queries" in request["data"] and len(request["data"]["queries"]) > 0 \
+                and "instant" in request["data"]["queries"][0] and request["data"]["queries"][0]["instant"]:
+                return self.query(
+                    datasource.get("id"),
+                    request["expr"],
+                    request["data"]["to"],
+                )
+            else:
+                return self.query_range(
+                    datasource.get("id"),
+                    request["expr"],
+                    request["data"]["from"],
+                    request["data"]["to"],
+                    request["data"]["step"],
+                )


datasource accesses change with grafana version: Prometheus in v 7 use format specified in function query() and query_range(), but now use a standard format for the queries (type or range) and results.

Hi. It looks like @PankajKhanwani is looking for this improvement around properly supporting Prometheus at GH-85, right?

Hi,
I think so.
even if PR was built for grafana 9.2 I think that the Prometheus datasource has not changed.
Do you believe you have time to review ?
JFPIK-

amotl · 2023-09-15T14:31:05Z

Dear Jean-Francois,

having to apologize again for the long delay, I finally found some time to review your patch in detail. I really like it, and would like to wrap it up, to satisfy the linter and the software tests, and also squash the commits, in order to finally integrate it.

~~I would start pushing directly to your feature branch~~, After rebasing your branch, I just pushed it to GH-112 already. Do you have any objections about this? If you want, and have a few spare minutes, feel free to add your comments there if you think I made a mistake anywhere.

datasource accesses change with grafana version: Prometheus in v 7 use format specified in function query() and query_range(), but now use a standard format for the queries (type or range) and results.

It is sad, right. I was well aware about those details when starting this experiment, that things will be in flux for times to come. Thank you for addressing any anomalies with your patch.

the format of the results changes from one datasource to anothers, maybe for grafana vesion too.

C'est la vie, I would say. Thank you so much for making the best out of it, by introducing corresponding version switching.

some of your tests (Loki ) seems not working for me : format my have change... not sure.

Thanks, I will look into it when starting to work on this.

I'm not sure that the grafana_client must implement a generic function smartquery() that permit to users to query and version to Grafana server and any data source in a unified way. What is your opinion?

I am absolutely not sure about it, but I definitively wanted to give it a try, because I thought it would make a nice feature. I know there will always be anomalies wrt. to a completely unified interface, that is natural. I am open to any discussions about this, and to hear back about what you think, how this topic could be approached better.

Still, I think the outcome is quite nice: After learning a bit of query parameters, how to use smartquery() for your purposes, I think it becomes very much usable for scripting purposes and for automating and monitoring Grafana, and the data sources behind. At least, this is what I was originally humbly aiming at, in order to expand what monitoring-check-grafana can do ¹.

With kind regards,
Andreas.

Currently, it is only a Shell program, and just supports InfluxDB 1.x, so it was clear to use Python for the next iteration. ↩

amotl · 2023-09-15T17:45:06Z

Closing this in favor of GH-112, where I added some adjustments to make the tests work again. Your improvements have been included into the 3.8.0 release, and I will be happy to receive corresponding feedback or bugfix patches, in case I made any mistakes. Thank you again, for both your patch and your patience.

peekjef72 and others added 15 commits February 5, 2022 19:30

add health and datasource proxy

e7596f9

update health part

8322aba

fixe migration error !

b47d2d6

test datasource part

3620ebd

test datasource part

c4f7a62

Merge branch 'main' of https://github.com/panodata/grafana-client int…

c21df87

…o panodata-main

merge

3fe5030

t push origin mainMerge branch 'panodata-main'

25470ae

Merge branch 'panodata:main' into main

3d40bca

new args; new examples; new db

34bb0e9

add some db models for query_factory

30ed93f

now returns dict containing all info to request Grafana API.

new smartquery function in conjonction with query_factory

31ce787

fix typo

526aef0

remove smartquery_old

5db363d

fix smart_query for check_health

syncho !?

4d6386f

peekjef72 requested a review from amotl as a code owner September 19, 2022 17:23

This comment was marked as duplicate.

Sign in to view

amotl mentioned this pull request Sep 20, 2022

unhashable type: 'dict' grafana-toolbox/grafana-snapshots-tool#2

Open

peekjef72 added 9 commits September 24, 2022 17:05

change smartquery args

98cf1a2

fix test_*errors

62ed416

loki results depend on grafana version

38549f8

add test for grafite and loki

1292ade

change typo

5a0e9a2

set smaller interval

6ad7b4d

remove smartquery_old again ?!

58218f5

fixe historical queries methods

e317930

fixe instant query mode

e352f29

peekjef72 added 2 commits October 26, 2022 16:46

fix influxdb flux smartquery model

70a3b55

set working example for infludb flux + typo

1a1c653

fix required and default values

b37a67b

amotl self-assigned this Nov 1, 2022

amotl mentioned this pull request May 3, 2023

Grafana Smart Query Prometheus missing key #85

Open

amotl reviewed May 3, 2023

View reviewed changes

amotl changed the title ~~review query_factory and smartquery~~ Improve smartquery and query_factory beyond data source health checks Sep 15, 2023

amotl mentioned this pull request Sep 15, 2023

Improve smartquery and query_factory beyond data source health checks #112

Merged

amotl changed the title ~~Improve smartquery and query_factory beyond data source health checks~~ Review improvements to smartquery and query_factory Sep 15, 2023

amotl changed the title ~~Review improvements to smartquery and query_factory~~ Review adjustments to smartquery and query_factory Sep 15, 2023

amotl closed this Sep 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review adjustments to `smartquery` and `query_factory` #38

Review adjustments to `smartquery` and `query_factory` #38

peekjef72 commented Sep 19, 2022 •

edited by amotl

Loading

This comment was marked as duplicate.

amotl commented Oct 17, 2022

amotl May 3, 2023

peekjef72 May 5, 2023

amotl commented Sep 15, 2023 •

edited

Loading

amotl commented Sep 15, 2023 •

edited

Loading

Review adjustments to smartquery and query_factory #38

Review adjustments to smartquery and query_factory #38

Conversation

peekjef72 commented Sep 19, 2022 • edited by amotl Loading

About

Discussion

This comment was marked as duplicate.

amotl commented Oct 17, 2022

amotl May 3, 2023

Choose a reason for hiding this comment

peekjef72 May 5, 2023

Choose a reason for hiding this comment

amotl commented Sep 15, 2023 • edited Loading

Footnotes

amotl commented Sep 15, 2023 • edited Loading

Review adjustments to `smartquery` and `query_factory` #38

Review adjustments to `smartquery` and `query_factory` #38

peekjef72 commented Sep 19, 2022 •

edited by amotl

Loading

amotl commented Sep 15, 2023 •

edited

Loading

amotl commented Sep 15, 2023 •

edited

Loading