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

Switch to grafana-golang-api-client #260

Closed
wants to merge 5 commits into from

Conversation

mbarrien
Copy link
Contributor

Switches datasource, dashboard, and folder resources to using the grafana-golang-api-client instead of accessing the REST API's directly.

There are some small bits of functionality lost in the change:

In addition to the missing fields, the api client's JSON rendering also has a lot of omitempty annotations, which results in many fields no longer being present in the next pull after you change to this code (especially for datasources).

If you have previously run grizzly pull, then run grizzly pull again in the new code, you will get diffs like this on all previously pulled resources (anonymized from actual examples):

Datasources:

--- a/grafana/grizzly/datasources/datasource-Grafana Cloud k6.yaml
+++ b/grafana/grizzly/datasources/datasource-Grafana Cloud k6.yaml
@@ -5,8 +5,6 @@ metadata:
 spec:
     access: proxy
     basicAuth: false
-    basicAuthUser: ""
-    database: ""
     id: 33
     isDefault: false
     jsonData:
@@ -18,11 +16,6 @@ spec:
     name: Grafana Cloud k6
     orgId: 1
     readOnly: true
-    secureJsonFields: {}
     type: k6-datasource
-    typeLogoUrl: /public/plugins/k6-app/img/logo.svg
     uid: Grafana Cloud k6
     url: proxy
-    user: ""
-    version: 1
-    withCredentials: false

Dashboards:

--- a/grizzly/dashboards/example-folder/dashboard-bazqux.yaml
+++ b/grizzly/dashboards/example-folder/dashboard-bazqux.yaml
@@ -23,6 +23,7 @@ spec:
     editable: true
     fiscalYearStartMonth: 0
     graphTooltip: 0
+    id: 180
     links: []
     liveNow: false
     panels:
@@ -133,4 +134,5 @@ spec:
     timezone: ""
     title: Sample Dashboard
     uid: bazqux
+    version: 2
     weekStart: ""

Folders:

--- a/grizzly/folders/folder-foobar.yaml
+++ b/grizzly/folders/folder-foobar.yaml
@@ -3,17 +3,7 @@ kind: DashboardFolder
 metadata:
     name: foobar
 spec:
-    canAdmin: false
-    canDelete: true
-    canEdit: true
-    canSave: true
-    created: "2022-12-08T12:51:41Z"
-    createdBy: Anonymous
-    hasAcl: false
     id: 124
     title: unionai
     uid: kVaCBuFVz
-    updated: "2022-12-08T12:51:41Z"
-    updatedBy: Anonymous
     url: /dashboards/f/foobar/example
-    version: 1

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2023

CLA assistant check
All committers have signed the CLA.

@joanlopez
Copy link
Contributor

Hey @mbarrien,

Thanks for opening this pull request, as discussed/suggested in #257. So, just to set proper expectations, let me express you that I don't think we'll be able to merge this as-is straight away, cause there's multiple things we need to verify, like:

  • Regressions (the expected behavior remains)
  • Deal with some (or all) the missing features, like making the underlying client to reach a minimum feature parity.

However, this is definitely super useful, and moves us multiple steps forward in the process of replacing the existing client by the "official" one, so thanks for doing so, I'm sure your contribution will be appreciated! :)

Hope to bring back news sooner than later!

@mbarrien
Copy link
Contributor Author

Wasn't expecting it to be merged as is; I know there are regressions. I see you (@joanlopez) have been getting work in the grafana-golang-api-client repo to fix some of them already, but didn't want to rely on an unreleased version in this PR.

P.S. If you really want to move things forward... the latest commit in https://github.com/mbarrien/grizzly/tree/openapi attempts a switch to using OpenAPI instead of the "official" repo, using the https://github.com/esnet/grafana-swagger-api-golang library (which is just Bingo run on the openapi3.json from the official Grafana repo, just like https://github.com/grafana/grafana-openapi-client-go would). It works well enough to do grr pull's, at least until I ran into grafana/grafana#76578 for the new Alert Rules functionality I was trying to introduce.

@nikimanoledaki
Copy link

nikimanoledaki commented Oct 16, 2023

Hi folks, grafana-golang-api-client will be deprecated since it is maintained manually and has many gaps in its coverage of Grafana's backend.

https://github.com/esnet/grafana-swagger-api-golang was created by a community member who used @papagian's POC to generate the client with swagger. See this comment: grafana/grafana#47827 (comment) - it is not maintained either.

The Grafana Backend Platform team is actively working on moving the swagger POC work to grafana/grafana-openapi-client-go, integrate it with all Grafana client dependencies, and deprecate the manual client. Please use that instead. Cheers :)

@mbarrien
Copy link
Contributor Author

mbarrien commented Oct 16, 2023

@nikimanoledaki With all due respect... it is way too premature to be asking any project to change over to another API right now. All 3 (grafana-golang-api-client, grafana-swagger-api-golang, and grafana-openapi-client-go) have gaps relative to each other.

  • As mentioned in previous comment, OpenAPI: Paths to rules provisioning are incorrect in api-merged.json grafana#76578 was exposed by my exploration of grafana-swagger-api-golang and reveals a gap in it, although the underlying bug blocking that is in Grafana mainline. And it's technically an outside library.
  • What you're asking to switch to within Grafana's own repo, grafana-openapi-client-go, only supports folders and no other feature.
  • The fewest gaps are in grafana-golang-api-client, but is apparently deprecated

Are you expecting all development on outside projects to halt until #47827 is resolved? Or even inside projects, since this particular repo grizzly is in Grafana's own Github? Or do you want a Grafana hosted project to switch to an outside implementation to access its own APIs? Or should Grizzly continue to maintain yet a 4th way to access Grafana's APIs, adding to technical debt. (This PR was done in response to #257, where I already saw work to switch to grafana-golang-api-client already underway.)

@nikimanoledaki
Copy link

nikimanoledaki commented Oct 16, 2023

@mbarrien I had not seen the issue but it's a good spot. Looking into it now.

What you're asking to switch to within Grafana's own repo, grafana-openapi-client-go, only supports folders and no other feature.

That was true before but we generated the client for all Grafana APIs: https://github.com/grafana/grafana-openapi-client-go/tree/main/client

You can swap to the grafana-openapi-client-go instead of using esnet/grafana-swagger-api-golang. 👍 The person who is maintaining that repo is also waiting to move to the new generated client - grafana/grafana#47827 (comment).

Lastly, no, you do not need to wait for #47827 to be complete to use grafana-openapi-client-go. That epic simply tracks some improvements we would like to add to improve the release & automation process and add non-blocking enhancements such as retries.

@malcolmholmes
Copy link
Contributor

Closing in lieu of #262.

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.

5 participants