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

[Feat] support splitting single graph into multiple graphs via imagefilters #41

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fabianbees
Copy link

@fabianbees fabianbees commented Dec 19, 2024

Summary

This change allows the Grafana graph to be split into multiple separate graphs.

This is useful, if multiple values are plotted into the same graph and their value ranges are quite different. So small value changes are much more visible.

Here is an example with and without this change:

Without change

Bildschirmfoto vom 2024-12-19 11-54-28

With change

Bildschirmfoto vom 2024-12-19 11-53-51 (Kopie)
Bildschirmfoto vom 2024-12-19 11-53-51 (Kopie 2)

Configuration

The values, which should be separated into an additional graph are specified via the Custom variable grafanaimagefiltersarray.

Each entry inside the grafanaimagefiltersarray defines a new graph and its value specifies which value should be omitted via a var-metricfilter. See the following example:
Bildschirmfoto vom 2024-12-19 11-54-49

@martialblog
Copy link
Member

Hi, thanks for the PR. I'll have a look at it.

Could you please check the failed phpcs jobs and fix the code style accordingly.

Also this change would require some new documentation in the docs/

@fabianbees
Copy link
Author

The code style checks should pass now.

Could you please check the failed phpcs jobs and fix the code style accordingly.

You can also use the following Grafana Dashboard Template see this change in action:
icinga_flux.json

@martialblog martialblog self-assigned this Dec 23, 2024
Copy link
Member

@martialblog martialblog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I had a closer look at and discussed it internally.

  • This feature is somewhat already covered with the custom graphs "Metrics per panel" setting.
    The implementation in this PR would introduce another way and thus more complexity.
    We're wondering if it's possible to implement this via the existing custom graphs?

  • The implementation needs some improvement. I tested the branch and without the new custom var
    I don't see any graphs for other services?
    Having the URLs in the custom var is also a bit error prone, maybe it's possible just to have the
    metric per graph value and build the URL in PHP.

  • Maybe we can improve the name grafanaimagefiltersarray and documentation for it (if we keep this implementation)

Regards,
Markus

@RincewindsHat
Copy link
Member

Hi,
I am the other part of "discussed it internally" :-)
Therefore my two cents:

What this feature effectively adds, is the possibility to configure graphs based on hosts, which was not easily possible before, since the Graph matching logic only uses Service name and CheckCommand.
This is indeed nice to have.
The other parts like selecting specific metrics or displaying mutliple panels can already be done with creating custom panels in Grafana and some fitting configuration in the Graphs section of the module.

The specifics of the implementation are not my strong suit (I leave that to @martialblog ), I will just share some thoughts about the general design.

This PR adds a third layer of configuration to the module, besides the module configuration itself and what is done in Grafana. Now there is some configuration in Icinga 2, too.
I think to makes this far too complicated (on users and in the code). If some added filtering is needed I would prefer to make this happen via a Graph configuration option inside the configuration of this module.

PS: I don't know how representative your example (hostalive/ping/icmp Graph) is, for the problem you want to solve, but this is already "solved" by putting packet loss and rta and different axes in a panel (which is advisable anyhow since they use different units), this should be the case in the example dashboards.

PPS: Since you seem to be in/from Erlangen, we also can meetup in ZAM sometime if you would like to talk about the topic in person. :-)

@fabianbees
Copy link
Author

fabianbees commented Jan 30, 2025

  • This feature is somewhat already covered with the custom graphs "Metrics per panel" setting.
    The implementation in this PR would introduce another way and thus more complexity.
    We're wondering if it's possible to implement this via the existing custom graphs?

Unfortunately this is not the solution I was looking for, because with the "Metric per panel" setting
we can configure individual custom graphs, but they would not be visible in a given service itself.

  • The implementation needs some improvement. I tested the branch and without the new custom var
    I don't see any graphs for other services?

This is indeed true and was an oversight in my implementation, which i have fixed with my latest commit (37ffe7f).

Having the URLs in the custom var is also a bit error prone, maybe it's possible just to have the
metric per graph value and build the URL in PHP.

From my point, it was specifically designed that way to provide flexibility in modifying graphs for specific services only, which can be easily achieved by setting "Custom Variables" in the Icinga service.

  • Maybe we can improve the name grafanaimagefiltersarray and documentation for it (if we keep this implementation)

Yes, I agree that the name grafanaimagefiltersarray is the best and I am open for any suggestions (custom_vars is something that comes to my mind).
Of course, I can also provide documentation on the configuration part, if the implementation part is expected to be accepted.

The other parts like selecting specific metrics or displaying multiple panels can already be done with creating custom panels in Grafana and some fitting configuration in the Graphs section of the module.

But as far as I know, these panels can not be included in a service, but only shown in the "Grafana Graphs" Dashboard.

This PR adds a third layer of configuration to the module, besides the module configuration itself and what is done in Grafana. Now there is some configuration in Icinga 2, too.

this is already "solved" by putting packet loss and rta and different axes in a panel

Yes, but this only works for a maximum of two metrics per graph, by specifying the metrics, which should be separated into separate graphs. In Icinga, this can be achieved for more than two metrics. Additionaly, the configuration can be varied for different services.

I don't know how representative your example (hostalive/ping/icmp Graph) is, for the problem you want to solve

Here would be another example where it is also be very helpful to have separate graphs for these metrics inside the Icinga service:
image

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.

3 participants