Add dashboard panels to all available dashboards

[lennarkivimae]

trafficstars

Like the guy in this thread. I also was in need of a way to automatically add analytics panel to all of the dashboards. The flow is both automatic and also trigger-able with /patch-dashboards endpoint. Manual trigger can be useful for testing. Automatic flow is behind timer. This checks dashboards after your specified timeout, and adds analytics to new dashboards if present. In team environment, this helps to reduce need for each person to manually add the dashboard, hence making it more surefire way to get statistics on the dashboards in use.

[MacroPower]

Thank you for the PR! This will be really helpful to have. I have been a bit busy but should have time to test & review properly in the next few weeks.

Just taking a quick initial look, my main concern would be adding additional functionality, especially endpoints, without requiring opt-in from users. Ideally I want to maintain compatibility and not have people run into issues if they upgrade to a newer release without changing any of their arguments / env vars. Also, due to the nature of this application, I know there are some folks that are running it on the Internet to collect data from users. Technically I know you'd have to actually add credentials for the new endpoint to have any impact, but I would rather not drop this on people in-place all the same.

A few options there would be:

1. Adding a new parameter & environment variable like --dashboard-updates=true / ENABLE_DASHBOARD_UPDATES=true.
2. Adding a subcommand e.g. macropower_analytics_panel_server dashboard update which calls AddAnalyticsToDashboards and then by default just exits 0.

I think option 1 would be a bit easier, especially given that you / end users wouldn't have to create any new container/job/etc. But option 2, using a subcommand, might be better because it could allow people not using this server (and instead using other data collection methods) to easily make use of this.

The only other thing that stuck out to me was naming the package worker, which felt non-descriptive of what this code does. I am not super particular on the exact naming but something like dashboard would be better I think. e.g. function would end up looking like dashboard.AddAnalytics rather than worker.AddAnalyticsToDashboards.

I can also do this when I get around to it if you're too busy. Again, thanks a ton for the contribution, a lot of people have asked for this feature so it will be really nice to finally have something available 🙂

[lennarkivimae]

Thank you for the PR! This will be really helpful to have. I have been a bit busy but should have time to test & review properly in the next few weeks.

Just taking a quick initial look, my main concern would be adding additional functionality, especially endpoints, without requiring opt-in from users. Ideally I want to maintain compatibility and not have people run into issues if they upgrade to a newer release without changing any of their arguments / env vars. Also, due to the nature of this application, I know there are some folks that are running it on the Internet to collect data from users. Technically I know you'd have to actually add credentials for the new endpoint to have any impact, but I would rather not drop this on people in-place all the same.

A few options there would be:

1. Adding a new parameter & environment variable like `--dashboard-updates=true` / `ENABLE_DASHBOARD_UPDATES=true`.

2. Adding a subcommand e.g. `macropower_analytics_panel_server dashboard update` which calls AddAnalyticsToDashboards and then by default just exits 0.

I think option 1 would be a bit easier, especially given that you / end users wouldn't have to create any new container/job/etc. But option 2, using a subcommand, might be better because it could allow people not using this server (and instead using other data collection methods) to easily make use of this.

The only other thing that stuck out to me was naming the package worker, which felt non-descriptive of what this code does. I am not super particular on the exact naming but something like dashboard would be better I think. e.g. function would end up looking like dashboard.AddAnalytics rather than worker.AddAnalyticsToDashboards.

I can also do this when I get around to it if you're too busy. Again, thanks a ton for the contribution, a lot of people have asked for this feature so it will be really nice to finally have something available 🙂

Thank you for the reply and feedback! Sorry for such a late response, as have I been bit busy aswell. In regards to suggestion, I think they're great. I'll try to implement the changes in near future!

[lennarkivimae]

I managed to break my promise by a lot unfortunately. On a plus side, I managed to get around to it now. Better late than never. Sorry for this, it was not my intention.

I think I managed to address your concerns. The feature is only toggled on when grafana-url and dashboard-update-token are provided, as they both are needed for the feature. I currently did not see the need to add special enable-dashboard-updates flag. Currently no one should have those flags passed, hence the feature is disabled by default. This should satisfy the option 1. Unless of course you'd still like that special flag to be in place. The option 2 I did not get around to do due to time constraints. If this is desired by people in the future. I'm happy to take a look at it then.

As requested, I broke up worker package. I also broke api.go into dashboard.go and http.go. There are now dashboard package and httpcli package. Should be more clear of their intents. httpcli just provides a simple HTTP request helpers. All dashboard update logic is done in dashboard package.

Also added checks if update is even required, and fixed a bug where updating dashboard moves the dashboard out of its folder.

Whenever you have a change, have a look. All feedback and guidance is most appreciated!

Thank you for your time