esnet / gdg

Grafana Dashboard Manager
https://software.es.net/gdg/
Other
333 stars 31 forks source link

Upload should be idempotent #250

Closed fingon closed 6 months ago

fingon commented 6 months ago

Is your feature request related to a problem? Please describe.

I was trying to reimplement my dashboards-from-git using this tool and ran at a wall. The problem:

It seems that the 'id' of dashboards changes across upload+backup cycle even if I do no changes. And that causes some git churn (potentially also problems if I wind up having links which refer to dashboards by id instead of uuid). It also loses all history which is quite frustrating (as I'm doing quite a lot of this as I am planning to use https://github.com/fingon/gg-grafana to rewrite dashboards every now and then).

Describe the solution you'd like

If there's matching dashboards, it shouldn't wind up doing anything (or rewrite them in place better). I'm not sure what is the difference, but in $PREVJOB we had grafana restore+save tooling which did not do anything by default and that is not the case here.

Additional context

Example which illustrates the problem:

mstenber@hana ~/projects/pyinfra/fw/gdg>gdg backup dash upload --dashboard jellyfin
2024-02-22 09:59:28 INF 1 dashboards have been uploaded
┌──────────┬────┬─────────┬──────────────────────────────────────┐
│ TITLE    │ ID │ FOLDER  │ UID                                  │
├──────────┼────┼─────────┼──────────────────────────────────────┤
│ Jellyfin │ 10 │ General │ d7430cfe-9417-4965-aac7-2c2978051a6a │
└──────────┴────┴─────────┴──────────────────────────────────────┘
mstenber@hana ~/projects/pyinfra/fw/gdg>gdg backup dash upload --dashboard jellyfin
WARNING: this will delete all dashboards from the monitored folders: 'General' (or all folders if ignore_dashboard_filters is set to true) and upload your local copy.  Do you wish to continue (y/n) y
2024-02-22 10:03:37 INF 1 dashboards have been uploaded
┌──────────┬────┬─────────┬──────────────────────────────────────┐
│ TITLE    │ ID │ FOLDER  │ UID                                  │
├──────────┼────┼─────────┼──────────────────────────────────────┤
│ Jellyfin │ 11 │ General │ d7430cfe-9417-4965-aac7-2c2978051a6a │
└──────────┴────┴─────────┴──────────────────────────────────────┘
safaci2000 commented 6 months ago

There was a previous conversation about this. I found the code needed to ensure the IDs are preserved to be a bit too convoluted, though that can be somewhat iterated on. The biggest issue I found is that even IF you maintain the ID, then you still end up with a git delta of a version change. I would argue that your changes should be maintained in git, not grafana. You should choose what your source of truth is, grafana or git, and treat that as the primary way of versioning dashboards.

Doing an upload and redownload seem to be generate SOME delta, so making the code more complicated to preserve the sanctity of the git diff I don't think is worth it.

Preserving a dashboard history, I could see being worth while, though I'll have to look at the code again to see what's needed. Either ways though you'll always end up with a delta between upload/download. If there's a pattern that doesn't I'd be curious to know what that is.

fingon commented 6 months ago

Thanks for the earlier info, I missed it when quickly browsing for pre-existing issues.

Even for dashes in git only case, the current functionality doesn't work well as e.g. favourites are tracked by id, and due to that if you re-upload dashboards users lose their favourites (and id-based links).

I don't unfortunately have anymore access to the original code I wrote for $PREVJOB, but I think there was API call which just set overwrite flag on and supplied id+uuid and it mostly worked for us.

To make the no-change detection work with git, you want to normalize the dashboards (e.g. export only always in same way and ignore version changes). That can be done elsewhere, though, given the upload otherwise retains the data but with id changes Grafana behaviour changes and you cannot really work around that.

safaci2000 commented 6 months ago

So, I can take another pass like I said, and per usual PRs are accepted, but as far as having a 0 diff as a goal; it's not the purpose of GDG. GDG is a tool that lets you manage entities (dashboards, libraryelements, connections) and allows you to have a consistent behavior between one instance of grafana and another.

The idea of having no delta in git is a nice to have but it's not really a strong motivation. Now, if there is a way of doing this easily and cleanly that doesn't make the code less maintainable or more complex that's fine. Doing things like ignoring the version number is not something I'd really put into GDG. The only way I can see doing that is disregarding the version number and having a static value. I'm not even sure what would happen if you upload version 1 when there's been 7 versions updated.

As far as Implementation goes, it's a lot easier to wipe everything and push what you have in your file system. After all the idea is that whatever is your backup storage should be what is in grafana. If anything else is in grafana in the watched folder it's supposed to be removed, and replaced with the data in git.

I don't mind adding complexity, we've done that before, but it should get us a net gain of some kind. your grafana history should ideally useless since git log <file.json> gives you all the changes that went with that dashboard, and the git diff being empty will not happen without some hacks.

fingon commented 6 months ago

Delta in git is not even the problem. It is the other user visible aspect ( when dash is deleted. It is removed from favorites ) which the upload step does even if it is really overwriting existing dashboard.

Anyway, as this is not that high priority problem for me either, we shall see if I get around to making PR for this.

safaci2000 commented 6 months ago

Interesting, okay that's a legitimate issue. I never noticed that behavior. Let me think about it a bit and revisit this.

safaci2000 commented 6 months ago

I'm not able to get the import endpoint to support this. Right now the options are to use /import or /dashboard/db. I'm sure /dashboard/db would work, but it drops support for libraryelements. /import currently isn't working for me.

I may revisit this later, and leave it open and as I said, I'll consider a PR.