Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Add support for polystat panel #210

Closed
wants to merge 1 commit into from
Closed

Add support for polystat panel #210

wants to merge 1 commit into from

Conversation

jbfavre
Copy link

@jbfavre jbfavre commented May 7, 2020

This PR intends to add support for polystat panel.
Please note that this is one of my first grafonnet contributions, therefore I'm likely to have missed some things.

I'll be happy to get your feedback so that I can improve this PR.

@arodrime
Copy link
Contributor

@jbfavre as writing in #212, it seems you did a quality work here.
The thing I see is that you did not use the 'plugin' approach that grafonnet folks recommend for new plugins (see the docs).

If you want to merge with my current work (or even fork mine and take ownership of maintaining this bit) I'm happy to merge your work with mine (or to ask for the removal of my plugin in favour of yours, if that is better for you :)).

@jbfavre
Copy link
Author

jbfavre commented May 18, 2020

@arodrime thanks for the comment.
I didn't use the 'plugin' approach because polystat panel is developed and maintained by Grafana.
But maybe I misunderstood the definition of core plugins. 🤔

@arodrime
Copy link
Contributor

arodrime commented May 18, 2020

Ah maybe you're right. I know it's Grafana Labs building this plugin, yet, it needs to be installed like a plugin in Grafana, it's not there out of the box. That's where I draw the line between grafonnet core or extention/plugin as well. I'm not sure what was expected.

@roidelapluie @trotttrotttrott maybe could you share with us your vision on this and pick either this PR or #212 ?
Thanks! 😊

@trotttrotttrott
Copy link
Member

I didn't use the 'plugin' approach because polystat panel is developed and maintained by Grafana.
But maybe I misunderstood the definition of core plugins.

@jbfavre sorry for the confusion, but @arodrime is correct. Polystat is not within the scope we've set for Grafonnet and should be managed as an extension. Though it's maintained by the Grafana Labs org, we don't consider it to be part of its "core features and plugins". Which means only what you'd find in a vanilla install. This has been debated a bit in the last few weeks so I apologize if our contributing docs aren't quite as clear as they could be. Definitely working on that.

I've gone ahead and merged #212. @arodrime's version is solid. Let me know though if you guys have feedback about scope, extension management, etc. Either here or the Grafana public slack.

@jbfavre
Copy link
Author

jbfavre commented May 21, 2020

@trotttrotttrott thanks for the explanation.
I now understand your position, but I'm afraid I quite disagree with it and will try to explain why:
I'm perfectly fine with not including third party plugins into the main repository.
But, since Grafana maintains the polystat panel, and for sure many other panels as well, it sounds logical that Grafana should either maintain the grafonnet version of it, or allow community contributions in the main repository.

I'm new to grafonnet, so I don't have the full debate history, nor have I deep knowledge about how grafonnet works.
But still, IMHO, current setup is one of the worst in term of adoption.

To use grafonnet, one needs to:

  • git clone this repository
  • figure out which plugins are up-to-date and which are not (see Update table_panel for Grafana 6.7.3 #209)
  • clone as many separate community-maintained plugins repository as they need. The list is in the main repository, but still, you have to git clone each and every repositories you need
  • once again, figure how many plugins are up-to-date or not (see flant-status-panel which is awesome, but doesn't work for Grafana >6.7)
  • finally, use them to write grafonnet dashboards

Compared to stock Grafana:

  • install grafana
  • enable needed plugins
  • create dashboard, add panels in the web UI, save dashboard. Done !

I agree this comparison isn't quite fair, because grafonnet provides abstraction features and consistency between files, which have to be manually resolve when using web UI.
But, I still think the learning curve for using grafonnet is quite high. Which advocates to simplify things as much as possible. For example: provide clear and easy entry points.

I know and understand Grafana isn't responsible to make each and every plugins compatible with latest Grafana versions. And so it goes for grafonnet.
But, as far as I know, Grafana will take care of updating plugins they maintain so that they're not broken with a Grafana update. I definitely think this should be the same for grafonnet.
I also understand you might not have needed bandwith to do that. That's fine to me, I'll be glad to contribute whenever I can or need to.

As for the grafonnet writing now. I'm not aware of any tool to ease plugin creation.
I'm new to it, so I basically had to manually create a dashboard in Grafana, manually add a polystat panel, manually look at its Json output, manually extract variables and manually add them into the libsonnet file.
Maybe I was wrong doing things this way. Maybe there's a tools to automate these steps, and I'll be more than happy to hear about it.
Such tool would allow to automatically create and/or update and/or document grafonnet plugin , using variables and documentation in the plugin source code.
This would, in turn, improve the possible grafonnet coverage since plugin creation and update would be much easier.

In summary:

  • Grafana should maintain the grafonnet version of the plugins they maintain, or allow community contributions in the main repository
  • Grafana should offer a central community-managed repository with third party plugins so that users have only one place to look at to be able to use them straight
  • Grafana should provide or advocate for a tool to ease grafonnet plugins maintenance

I'm sorry I missed the discussions of the last few week. I would have given my point of view on this already. I do not intend to re-open the debate.
I also do not intend to be rude in any way, so I apologize in advance if you feel so.
Cheers

@malcolmholmes
Copy link
Contributor

@jbfavre you make reasonable points. I would suggest moving them to a separate issue so they can be discussed independently of this case.

The limitation on which plugins Grafonnet would support was made relatively arbitrarily, based upon the capacity of the teams that manage it. The alternative was a large number of either unreviewed or unmerged PRs.

The other possibility is we look at auto generating Grafonnet plugins, based upon field definitions provided by Grafana 7.0. However, this idea is currently totally untested, and thus still just dreaming.

@jbfavre
Copy link
Author

jbfavre commented May 21, 2020

@malcolmholmes I'll open 2 separate issues then.
One about the scope of main grafonnet repository and what we could do with third-party ones.
One about the tools I dream about (yes, I'm aware this hasn't been tested and is only a dream ;-) )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants