-
Notifications
You must be signed in to change notification settings - Fork 28
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
(PE-28451) switch from the v1 to v2 metrics api #57
Conversation
With this commit ... We switch from the v1 to v2 Metrics API for additional metrics (for PuppetDB), and drop metrics for unsupported versions of PE. (This is required in response to PE-28451 / PE-28468. Note these additional metrics generate errors in PE 2019.4 via v2: puppetlabs.puppetdb.ha:name=failed-request-counter puppetlabs.puppetdb.mq:name=global.generate-retry-message-time puppetlabs.puppetdb.mq:name=global.retry-persistence-time
I've manually tested this on 2018111, 201921, and 201940; and metrics for puppetdb, puppetserver, and orchestrator are collected without error (after commenting out the three additional metrics for puppetdb as per the commit message). |
Like Krylon: No runs, no drips, no errors ...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
A couple notes about ActiveMQ metrics (I don't think we need to change anything there?) and a stray line of 2016.4/2016.5 logic that can be dropped.
Tested 2018.1.12 and 2018.1.13 w/ HA enabled and the only differences in output were the three metrics that are purposefully excluded:
- puppetlabs.puppetdb.ha:name=failed-request-counter
- puppetlabs.puppetdb.mq:name=global.generate-retry-message-time
- puppetlabs.puppetdb.mq:name=global.retry-persistence-time
These are problematic because the counters don't exist until the event happens for the first time. However, they seem a bit off the beaten path so we can re-enable them later if the need arises.
Restore additional activemq metrics. Delete PE 2016 conditionals. Restore counters that don't exist until a failure occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Just have to get the Travis tests green and this is ready to merge.
manifests/service/puppetdb.pp
Outdated
/^2017.(1|2)/ => {'catalogs' => 9, 'facts' => 5, 'reports' => 8}, | ||
default => {'catalogs' => 9, 'facts' => 5, 'reports' => 8}, | ||
} | ||
$version = {'catalogs' => 9, 'facts' => 5, 'reports' => 8}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a trailing comma at the end of this line that is failing the Travis tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit for this pushed ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
…-support-script Add double quotes to copy_drop_match
With this commit ...
We switch from the v1 to v2 Metrics API for additional metrics (for PuppetDB),
and drop metrics for unsupported versions of PE.
(This is required in response to PE-28451 / PE-28468.
Note these additional metrics generate errors in PE 2019.4 via v2:
puppetlabs.puppetdb.ha:name=failed-request-counter
puppetlabs.puppetdb.mq:name=global.generate-retry-message-time
puppetlabs.puppetdb.mq:name=global.retry-persistence-time