aerogear / proposals

AeroGear Proposals
Apache License 2.0
0 stars 17 forks source link

Add a subsection on Grafana dashboard discovery #11

Closed darahayes closed 6 years ago

darahayes commented 6 years ago

The main goal of this PR is to give more visibility to the discussion in this ticket: https://issues.jboss.org/browse/AEROGEAR-1821

and in this thread:

https://github.com/aerogear/proposals/pull/5#discussion_r157767612

wtrocki commented 6 years ago

That's a one way to do it, but sidecar is a workaround when we will not be able to do it in grafana. IMHO and we can try do it smarter, without additional overhead..

We can mount dashboards to specific location and let grafana discover that as it's was happening for POC. I was thinking to research that option - to see if we can configure, trigger grafana to reindex folder that contains dashboards. This will happen without any additional pod running. I have seen numerous issues on grafana that will tackle this problem. This will allow us to contribute to grafana instead of providing ineffective workaround on our side. I think it's best to identify if grafana 4.7.0 will have any functionalities for that (there are couple issues in their backlog currently) If we feel that we need something better we can contribute directly, which is aligned with our vision: "Contribute upstream when possible :) "

Idea I wanted to POC: 1) New service is being provisioned with dashboard as config map. 2) Dashboard is discovered and mounted by grafana to index location 2.1) [Alternative] APB edits config-map with dashboard files and appends new dashboard. 3) Grafana reloads configuration

david-martin commented 6 years ago

@darahayes Lets hold off on merging this until the investigation is complete https://issues.jboss.org/browse/AEROGEAR-1821

I'd prefer to see the proposal having the definitive solution once we know what thats going to be i.e. replace the visualisation section of the proposal completely with what we're definitely going to do.

darahayes commented 6 years ago

@david-martin I agree completely. The main goal of the PR was just to add extra visibility. I've also had some in depth discussion with @wtrocki around the approach he mentioned above. I think it would be worthwhile to draw up a clearer outline of his suggestion for the consideration of yourself and the wider teams.

david-martin commented 6 years ago

@wtrocki some replies to your comment below

That's a one way to do it, but sidecar is a workaround when we will not be able to do it in grafana. IMHO and we can try do it smarter, without additional overhead..

What is the additional overhead that you see going away with below approach?

We can mount dashboards to specific location and let grafana discover that as it's was happening for POC. I was thinking to research that option - to see if we can configure, trigger grafana to reindex folder that contains dashboards. This will happen without any additional pod running.

IIRC, in Grafana 4.7 (from master a few weeks ago) automatically updated dashboards from files without any reload required.

I have seen numerous issues on grafana that will tackle this problem.

Getting something definitive about what the solution will be in Grafana would be great.

This will allow us to contribute to grafana instead of providing ineffective workaround on our side. I think it's best to identify if grafana 4.7.0 will have any functionalities for that (there are couple issues in their backlog currently) If we feel that we need something better we can contribute directly, which is aligned with our vision: "Contribute upstream when possible :) "

Agreed. If something that solves our use case is happening upstream, we should hold off on that or see if we can contribute in some way.

Idea I wanted to POC:

New service is being provisioned with dashboard as config map. Dashboard is discovered and mounted by grafana to index location 2.1) [Alternative] APB edits config-map with dashboard files and appends new dashboard. Grafana reloads configuration

wtrocki commented 6 years ago

What is the additional overhead that you see going away with below approach?

Is that you need to build additional container, build system that will do api calls. It feels that this requires more effort than file scan that is implemented in grafana already, but I might be missing some facts here.

darahayes commented 6 years ago

It seems that the Grafana master branch has the functionality described by @wtrocki. It searches a dashboards directory (defaults to /var/lib/grafana/dashboards but can be configured) and will automatically load any dashboard json files found in that directory. It also scans every 5 seconds and updates the dashboards if there are any changes.

You can see the PR (merged) for this feature here: https://github.com/grafana/grafana/pull/10052

The very same behaviour has also been implemented for datasources.

This would be a really nice approach but it could be months away from the official Grafana 5.0 release which will include these changes.

wtrocki commented 6 years ago

@darahayes Master will be included as part of 4.7 release this month. Mentioned comment is there: https://github.com/grafana/grafana/tree/v4.7.x

I think we can build 4.7.x on our own and publish to aerogear temporarily unti it will be released. @pb82 @david-martin WDYT?

david-martin commented 6 years ago

@darahayes is the content in this PR superceded by https://github.com/aerogear/proposals/pull/12?

darahayes commented 6 years ago

@david-martin That's correct. Now that #12 has been merged, I'm happy to close this. Thanks