Skip to content

Commit

Permalink
reports: Update to use report_prefix
Browse files Browse the repository at this point in the history
This commit removes the report_filename config parameter and adds a
report_prefix parameter.  AFAICT the current code suggests that it will
write all host data to the same file.  However the code writes the old
data to the prom file giving all values -1 which is incorrect

I looked at why this was and i can't find much information on the
original issue[1] and PR[2].  further to this the current implementation
is very racy if one sets REPORT_FILENAME

This also updates the clean stale function toi only act on files with
the report_prefix and avoid deleting files which may have been placed
there by none puppetserver process

[1]voxpupuli#5
[2]voxpupuli#7
  • Loading branch information
b4ldr committed Oct 2, 2023
1 parent cee6ebd commit 172dd8a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 26 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ textfile_directory: /var/lib/prometheus-dropzone
Configuration options include:
- `textfile_directory` - [String] Location of the node_exporter `collector.textfile.directory` (Required)
- `report_filename` - [String] If specified, saves all reports to a single file (must end with .prom)
- `report_prefix` - [String] the prefix to add to the prometheus reprots. By default this is `puppet_report_`.
- `environments` - [Array] If specified, only creates metrics on reports from these environments
- `reports` - [Array] If specified, only creates metrics from reports of this type (changes, events, resources, time)
- `stale_time` - [Integer] If specified, delete metric files for nodes that haven't sent reports in X days
Expand Down
29 changes: 4 additions & 25 deletions lib/puppet/reports/prometheus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
config = YAML.load_file(configfile)

TEXTFILE_DIRECTORY = config['textfile_directory']
REPORT_FILENAME = config['report_filename']
REPORT_PREFIX = config.fetch('report_prefix', 'puppet_report_')
ENVIRONMENTS = config['environments']
REPORTS = config['reports']
STALE_TIME = config['stale_time']
Expand All @@ -27,19 +27,10 @@
raise(Puppet::ParseError, "#{configfile}: textfile_directory is not set or is missing.")
end

unless REPORT_FILENAME.nil? || REPORT_FILENAME.end_with?('.prom')
raise(Puppet::ParseError, "#{configfile}: report_filename does not ends with .prom")
end

def process
return unless ENVIRONMENTS.nil? || ENVIRONMENTS.include?(environment)
namevar = if REPORT_FILENAME.nil?
host + '.prom'
else
REPORT_FILENAME
end
namevar = "#{REPORT_PREFIX}#{host}.prom"

yaml_filename = File.join(TEXTFILE_DIRECTORY, '.' + namevar + '.yaml')
filename = File.join(TEXTFILE_DIRECTORY, namevar)

common_values = {
Expand Down Expand Up @@ -139,30 +130,18 @@ def process

File.open(filename, 'w') do |file|
file.write(definitions)
if File.exist?(yaml_filename)
file.write("# Old metrics\n")
existing_metrics = YAML.load_file(yaml_filename)
existing_metrics.each do |k, _v|
file.write("#{k} -1\n") unless new_metrics.include?(k)
end
end

file.write("# New metrics\n")
new_metrics.each do |k, v|
file.write("#{k} #{v}\n")
end
end

File.open(yaml_filename, 'w') do |yaml_file|
yaml_file.write new_metrics.to_yaml
end
File.chmod(0755, filename)

clean_stale_reports
end

def clean_stale_reports
return if STALE_TIME.nil? || STALE_TIME < 1
Dir.chdir(TEXTFILE_DIRECTORY)
Dir.glob('*.prom').each { |filename| File.delete(filename) if (Time.now - File.mtime(filename)) / (24 * 3600) > STALE_TIME }
Dir.glob("#{REPORT_PREFIX}*.prom").each { |filename| File.delete(filename) if (Time.now - File.mtime(filename)) / (24 * 3600) > STALE_TIME }
end
end

0 comments on commit 172dd8a

Please sign in to comment.