(SLV-672) Add system class to manage system metric #30
Conversation
manifests/sar_metric.pp
Outdated
| cron { "${metrics_type}_metrics_tidy" : | ||
| ensure => $metric_ensure, | ||
| user => 'root', | ||
| hour => fqdn_rand(3, $metrics_type ), |
There was a problem hiding this comment.
It is the max for the hour. Basically will generate a random number between 0 and 3. This is so all the cron jobs don't run at the same time and possibly impact perf. It's just a copy pasta from pe_metric.pp
There was a problem hiding this comment.
Talked offline about this, and added a comment.
manifests/sar_metric.pp
Outdated
| ensure => $metric_ensure, | ||
| user => 'root', | ||
| hour => fqdn_rand(3, $metrics_type ), | ||
| minute => (5 * fqdn_rand(11, $metrics_type )), |
There was a problem hiding this comment.
another copy paste from pe_metric.pp generates a random number between 0 and 5 for the minute it should run to try an not run all metrics gathering at the same time.
There was a problem hiding this comment.
talked offline and added a comment
RandellP
force-pushed
the
SLV-672
branch
2 times, most recently
from
7ad2425 to
8235b24
Compare
manifests/sar_metric.pp
Outdated
| define puppet_metrics_collector::sar_metric ( | ||
| String $output_dir, | ||
| String $scripts_dir, | ||
| Enum['absent', 'present'] $metric_ensure = 'present', |
There was a problem hiding this comment.
It seems odd to have some of these aligned with extra spaces ($metric_ensure, $metrics_type, etc...) and the rest not aligned ($polling_frequency_seconds, $collection_frequency). Is this intentional?
There was a problem hiding this comment.
They look aligned with extra space on my screen?
manifests/sar_metric.pp
Outdated
| cron { "${metrics_type}_metrics_tidy" : | ||
| ensure => $metric_ensure, | ||
| user => 'root', | ||
| hour => fqdn_rand(3, $metrics_type ), |
There was a problem hiding this comment.
| hour => fqdn_rand(3, $metrics_type ), | |
| hour => fqdn_rand(3, $metrics_type), |
manifests/sar_metric.pp
Outdated
| ensure => $metric_ensure, | ||
| user => 'root', | ||
| hour => fqdn_rand(3, $metrics_type ), | ||
| minute => (5 * fqdn_rand(11, $metrics_type )), |
There was a problem hiding this comment.
| minute => (5 * fqdn_rand(11, $metrics_type )), | |
| minute => (5 * fqdn_rand(11, $metrics_type)), |
manifests/system_cpu.pp
Outdated
| } | ||
|
|
||
| puppet_metrics_collector::sar_metric { 'cpu' : | ||
| metric_ensure => $metrics_ensure, |
There was a problem hiding this comment.
If this is being aligned with the code above:
| metric_ensure => $metrics_ensure, | |
| metric_ensure => $metrics_ensure, |
otherwise:
| metric_ensure => $metrics_ensure, | |
| metric_ensure => $metrics_ensure, |
manifests/system_memory.pp
Outdated
| } | ||
|
|
||
| puppet_metrics_collector::sar_metric { 'memory' : | ||
| metric_ensure => $metrics_ensure, |
There was a problem hiding this comment.
See alignment suggestion in 'system_cpu ' above.
RandellP
force-pushed
the
SLV-672
branch
3 times, most recently
from
7dec69d to
84932b8
Compare
|
@RandellP this PR requires a rebase before it can be merged. |
RandellP
dismissed stale reviews from billclaytor and johnduarte
via
62fd050
RandellP
force-pushed
the
SLV-672
branch
3 times, most recently
from
43543af to
02fad23
Compare
|
I am running some tests with this now and will report back this afternoon with the results. |
manifests/system.pp
Outdated
| file { "${scripts_dir}/generate_system_metrics": | ||
| ensure => present, | ||
| mode => '0755', | ||
| source => 'puppet:///modules/puppet_metrics_collector/generate_system_metrics' | ||
| } |
There was a problem hiding this comment.
The generate_system_metrics file declaration should be outside of the if ! defined block since it is not declared anywhere else. This caused the generate_system_metrics to be missing on my system when both of the classes below were included in my test system.
include puppet_metrics_collector
include puppet_metrics_collector::system
aee4d5c to
fbb45f8
Compare
The new system class is very similar to the init.pp... this is to keep the two seperate during development. They may get merged in teh future. But the work flow is such that the master would have puppet metrics collector and sysystem included, while the compilers would only have system. To merge them would require a clean way to turn off all pe metrics for the compilers. Also added system_cpu and system_memory to use sar_metric to gather cpu and memory metrics.
Depends on #28
The new system class is very similar to the init.pp... this is to keep the two seperate during development.
They may get merged in teh future. But the work flow is such that the master would have puppet metrics collector and sysystem included, while the compilers would only have system. To merge them would require a clean way to turn off all pe metrics for the compilers.
Also added system_cpu and system_memory to use sar_metric to gather cpu and memory metrics.