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

Add grafana dashboard to pgagroal #152

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Conversation

An-DJ
Copy link
Contributor

@An-DJ An-DJ commented Jun 17, 2021

Feature

Add grafana dashboard to pgagroal.

How to use it

Importing the configuration file by following the steps in contrib/grafana/README.md.

You can get the grafana dashboard of pgagroal.

@jesperpedersen
Copy link
Collaborator

This is a great start to your GSoC project !

I think you can assume Grafana 8.x for this work, so you can use the new features as described in https://grafana.com/docs/grafana/latest/whatsnew/whats-new-in-v8-0/ - like the bar charts and histograms.

I have updated my Prometheus/Grafana image @ https://github.com/jesperpedersen/podman-pgsql/tree/master/pgsql13-grafana-centos8 and added Prometheus as the default data source out-of-the-box.

Some comments for your pull request.

For the README.md -- I would split step 2 into 2 separate steps as one is in Prometheus and the other is in Grafana; that makes it more clear for users I think.

For the dashboard;

I think it is a good idea to split the dashboard into categories -- maybe add an "Overview" category that has the most important information - like pgagroal_state, percentage of pgagroal_active_connection / pgagroal_max_connection and so on.

"Authentication" - I think that one is good.

"Session time" - likely better as a histogram, and having the time on the X-axis, and the count on the Y-axis.

"Connection details" - I think if we have percentage of pgagroal_active_connection / pgagroal_max_connection in the "Overview" then this can be removed.

"Connection tread" - Rename that to "Connection events".

"Connection state" - Rename that one to "Connections" and move it to the "Overview" panel.

"Failed servers" - I think just the number of failed servers is enough to display. Needs to be moved to "Overview" as well.

We will likely have to move more stuff around once we get further, but that should be an easier task. See if you can split the "Server errors" panel into separate lines per server (eg. based on name).

Thanks !

@jesperpedersen jesperpedersen self-assigned this Jun 18, 2021
@An-DJ
Copy link
Contributor Author

An-DJ commented Jun 20, 2021

This is a great start to your GSoC project !

I think you can assume Grafana 8.x for this work, so you can use the new features as described in https://grafana.com/docs/grafana/latest/whatsnew/whats-new-in-v8-0/ - like the bar charts and histograms.

I have updated my Prometheus/Grafana image @ https://github.com/jesperpedersen/podman-pgsql/tree/master/pgsql13-grafana-centos8 and added Prometheus as the default data source out-of-the-box.

Some comments for your pull request.

For the README.md -- I would split step 2 into 2 separate steps as one is in Prometheus and the other is in Grafana; that makes it more clear for users I think.

For the dashboard;

I think it is a good idea to split the dashboard into categories -- maybe add an "Overview" category that has the most important information - like pgagroal_state, percentage of pgagroal_active_connection / pgagroal_max_connection and so on.

"Authentication" - I think that one is good.

"Session time" - likely better as a histogram, and having the time on the X-axis, and the count on the Y-axis.

"Connection details" - I think if we have percentage of pgagroal_active_connection / pgagroal_max_connection in the "Overview" then this can be removed.

"Connection tread" - Rename that to "Connection events".

"Connection state" - Rename that one to "Connections" and move it to the "Overview" panel.

"Failed servers" - I think just the number of failed servers is enough to display. Needs to be moved to "Overview" as well.

We will likely have to move more stuff around once we get further, but that should be an easier task. See if you can split the "Server errors" panel into separate lines per server (eg. based on name).

Thanks !

  • Overview: added
  • Session time: I have adjusted it to bar gauge, which I think may be better
  • Panels such as "connection state" have been moved into Overview area

New panel

  • Server connections: This panel displays the connection amout per database.
  • Server errors: This panel displays the errors per name.

Is there anything else I need to adjust? : )

@jesperpedersen
Copy link
Collaborator

I took a look at the various panels.

Overview:

State: You need a value mapping for '2' (GRACEFUL) - likely yellow
Active connection rate: Likely better as a Gauge component with 1.0 == pgagroal_max_connections. Also rename to "Active connections"
Session time: Yeah, that is a good panel component; maybe just use a basic color (purple ?) for the bars.
Failed servers: Likely better as a Stat component.

Connection:

Connection event: Rename to Connection events.

Server:

Server connections: I don't know if we can remove any mappings with 0 such that "Value" doesn't show up - not critical right now. We have to figure out how to represent it over time...

Could you add your name to the AUTHORS file as well ?

Then squash all your commits, and force push your branch - and I'll merge it :)

@An-DJ
Copy link
Contributor Author

An-DJ commented Jun 22, 2021

I took a look at the various panels.

Overview:

State: You need a value mapping for '2' (GRACEFUL) - likely yellow

Done.

Active connection rate: Likely better as a Gauge component with 1.0 == pgagroal_max_connections. Also rename to "Active connections"

Done.

Session time: Yeah, that is a good panel component; maybe just use a basic color (purple ?) for the bars.

Done.

Failed servers: Likely better as a Stat component.

Done.

Connection:

Connection event: Rename to Connection events.

Done.

Server:

Server connections: I don't know if we can remove any mappings with 0 such that "Value" doesn't show up - not critical right now. We have to figure out how to represent it over time...

Done.

Could you add your name to the AUTHORS file as well ?

Done.

Then squash all your commits, and force push your branch - and I'll merge it :)

Done.

Thanks 😄

@jesperpedersen jesperpedersen merged commit 1443ddb into agroal:master Jun 22, 2021
@jesperpedersen
Copy link
Collaborator

Merged.

Thanks for your contribution !

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

Successfully merging this pull request may close these issues.

2 participants