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

Improve support for FOSS puppet dashboards #49

Merged
merged 1 commit into from
Apr 25, 2019
Merged

Improve support for FOSS puppet dashboards #49

merged 1 commit into from
Apr 25, 2019

Conversation

suckatrash
Copy link
Contributor

@suckatrash suckatrash commented Apr 23, 2019

This addresses #47. Before this commit, the Telegraf Puppetserver Performance dashboard had pe- prefixed metrics, and didn't work with FOSS puppet.

This commit adds a conditional to the puppet_metrics_dashboard::dashboards::telegraf class that points to a new dashboard without the prefixes. This is dependent on the presence of a pe_server_version fact that will not be in a FOSS install.

@suckatrash suckatrash requested a review from Sharpie April 23, 2019 16:44
@Sharpie
Copy link
Member

Sharpie commented Apr 23, 2019

Pulling this into a FOSS install does result in a usable dashboard. However, I'm hemming and hawing a bit as dropping the pe- prefix does prevent the boards from working on 2017.2.x or older, along with archival data.

It seems like this problem could be solved with a constant variable that allowed pe- to be selectively set on import. However, the Grafana HTTP API appears to have no support for setting variables on import :(

The other solution would be to switch from files to templates, but that makes updating the boards more difficult going forward.

So, I suppose the question is:

  • Is templating worth it?

  • Should we cut a major release dropping support for Puppet 4 and PE versions older than 2017.3?

  • Or, since the FOSS impact is really just Telegraf_Puppetserver_Performance.json, should we create a copy of just that template without the pe- prefix and use it if $facts['pe_server_version'] is undef?

After typing all this out, I'm starting to lean towards Option 3.

@suckatrash suckatrash changed the title remove 'pe-' prefix from metrics Improve support for FOSS puppet dashboards Apr 25, 2019
Copy link
Member

@Sharpie Sharpie left a comment

Choose a reason for hiding this comment

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

Looks good to me! Tested against Puppet Server 5 and everything is showing up 👍

@suckatrash suckatrash merged commit e4512a6 into puppetlabs:master Apr 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants