Change server_puppetserver_profiler and server_puppetserver_metrics defaults to true
#825
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
In 12154ce it was split into two different parameters. If metrics depend on the profiler, should we make it catalog compilation fail on it? |
|
Only some metrics depend on this setting. eg. the compiler and function metrics. With the profiler off, you still get the jvm and jruby metrics etc. |
alexjfisher
changed the title
server_puppetserver_profiler and server_puppetserver_profiler defaults to true
alexjfisher
changed the title
server_puppetserver_profiler and server_puppetserver_profiler defaults to trueserver_puppetserver_profiler and server_puppetserver_metrics defaults to true
ekohl
approved these changes
Apr 4, 2022
manifests/init.pp
Outdated
| @@ -439,10 +439,11 @@ | |||
| # Defaults to 30000, using the Jetty default of 30s | |||
| # | |||
| # $server_puppetserver_metrics:: Enable puppetserver http-client metrics | |||
| # Defaults to false because that's the Puppet Inc. default behaviour. | |||
| # Defaults to true because that's the Puppet Inc. default behaviour (since Puppetserver 5.0.0). | |||
There was a problem hiding this comment.
I wonder if we even need to mention the default at all, but perhaps shorten it?
Suggested change
| # Defaults to true because that's the Puppet Inc. default behaviour (since Puppetserver 5.0.0). | |
| # Defaults to true, matching defaults in Puppetserver 5+. |
There was a problem hiding this comment.
Updated and rebased.
manifests/init.pp
Outdated
| # | ||
| # $server_puppetserver_profiler:: Enable JRuby profiling. | ||
| # Defaults to false because that's the Puppet Inc. default behaviour. | ||
| # Defaults to true because that's the Puppet Inc. default behaviour (since Puppetserver 5.0.0). |
There was a problem hiding this comment.
Same comment here about the defaults.
|
Also, could you rebase? I'd expect the tests to pass then. |
Puppet Inc. enabled the profiler by default in Puppetserver 5.0.0 https://tickets.puppetlabs.com/browse/SERVER-1761 This module has been disabling by default though causing users to be missing metrics they might reasonably expect to be present after reading through the [Puppetserver documentation](https://puppet.com/docs/puppet/6/server/puppet_server_metrics.html#compiler-metrics).
The default in Puppetserver is actually `true`, not `false`. (IMO, `server_puppetserver_metrics` isn't a great parameter name as its actual purpose is to configure whether http client metrics are collected, and has no impact on the much greater number of other metrics still collected when this is set to `false`.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Puppet Inc. enabled the profiler by default in Puppetserver 5.0.0
https://tickets.puppetlabs.com/browse/SERVER-1761
This module has been disabling by default though causing users to be
missing metrics they might reasonably expect to be present after reading
through the Puppetserver documentation.
EDIT
Also changed
server_puppetserver_metrics(which enables http-client metrics) totrue