Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes #30029 - telemetry label filter #7765

Merged
merged 1 commit into from Sep 22, 2020
Merged

Fixes #30029 - telemetry label filter #7765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 72 additions & 0 deletions config/initializers/5_telemetry_metrics.rb
View file
Open in desktop
Expand Up @@ -29,3 +29,75 @@
telemetry.add_counter(:audit_records_created, 'Number of audit records created in the DB', [:type])
telemetry.add_counter(:audit_records_logged, 'Number of audit records sent into logger', [:type])
telemetry.add_counter(:config_report_metric_count, 'Number of config report status metrics', [:metric])

# To decrease amount of metrics, labels must be allowed explicitly
allowed_labels = {
controller: [
'api/v2/config_reports_controller',
'api/v2/hosts_controller',
'api/v2/puppet_hosts_controller',
'config_reports_controller',
'dashboard_controller',
'fact_values_controller',
'hostgroups_controller',
'hosts_controller',
'notification_recipients_controller',
],
class: [
'Architecture',
'AuthSource',
'Bookmark',
'CommonParameter',
'ComputeResource',
'ConfigReport',
'Domain',
'DomainParameter',
'Environment',
'FactName',
'FactValue',
'Filter',
'Filtering',
'GroupParameter',
'Host::Base',
'Host::Managed',
'Hostgroup',
'HostParameter',
'Image',
'JobTemplate',
'Location',
'LocationParameter',
'LookupKey',
'LookupValue',
'Model',
'Nic::Base',
'Nic::BMC',
'Nic::Bond',
'Nic::Bridge',
'Nic::Interface',
'Nic::Managed',
'Operatingsystem',
'Organization',
'OrganizationParameter',
'OsParameter',
'Parameter',
'ProvisioningTemplate',
'Ptable',
'PuppetclassLookupKey',
'Realm',
'RemoteExecutionFeature',
'Report',
'Role',
'SmartProxy',
'Subnet',
'SubnetParameter',
'Taxonomy',
'Template',
'TemplateInput',
'TemplateInvocation',
'TemplateKind',
'User',
'Usergroup',
'Widget',
],
}
telemetry.allowed_tags(allowed_labels)
24 changes: 23 additions & 1 deletion lib/foreman/telemetry.rb
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@ class Telemetry
extend Forwardable
attr_accessor :prefix, :sinks

DEFAULT_BUCKETS = [1, 5, 20, 50, 100, 250, 500, 2000].freeze
DEFAULT_BUCKETS = [100, 500, 3000].freeze

def initialize
@sinks = []
Expand Down Expand Up @@ -96,6 +96,14 @@ def enabled?
@sinks.count > 1
end

def allowed_tags(labels)
ezr-ondrej marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

We would need to allow nil here, so what renaming it and then using the variable?

Suggested change
def allowed_tags(labels)
def initialize_allowed_tags(labels)

@allowed_tags = {}
labels.each do |k, v|
@allowed_tags[k] = Regexp.compile('^(' + v.join('|') + ')$')
end
@allowed_tags
end

def add_counter(name, description, instance_labels = [])
@sinks.each { |x| x.add_counter("#{prefix}_#{name}", description, instance_labels) }
end
Expand All @@ -109,15 +117,29 @@ def add_histogram(name, description, instance_labels = [], buckets = DEFAULT_BUC
end

def increment_counter(name, value = 1, tags = {})
return unless allowed?(tags)
@sinks.each { |x| x.increment_counter("#{prefix}_#{name}", value, tags) }
end

def set_gauge(name, value, tags = {})
return unless allowed?(tags)
@sinks.each { |x| x.set_gauge("#{prefix}_#{name}", value, tags) }
end

def observe_histogram(name, value, tags = {})
return unless allowed?(tags)
@sinks.each { |x| x.observe_histogram("#{prefix}_#{name}", value, tags) }
end

private

def allowed?(tags)
result = true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
result = true
return true unless @allowed_tags
result = true

tags.each do |label, value|
regexp = @allowed_tags[label]
Copy link
Member

Choose a reason for hiding this comment

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

Should we do this? Or is something else running that method? Given it's memoized, I'd prefer calling a method instead of variable.

Suggested change
regexp = @allowed_tags[label]
regexp = allowed_tags[label]

result &&= !!regexp.match(value) if regexp
end
result
end
end
end
2 changes: 2 additions & 0 deletions lib/foreman/telemetry_sinks/prometheus_sink.rb
View file
Open in desktop
Expand Up @@ -7,6 +7,8 @@ def initialize(opts = {})
require 'prometheus/client/data_stores/direct_file_store'
# Set multiprocess-friendly data store
FileUtils.mkdir_p(PROMETHEUS_STORE_DIR)
# but clean it during startup as files will accumulate over time
FileUtils.rm_f(Dir.glob("#{PROMETHEUS_STORE_DIR}/*.bin"))
Prometheus::Client.config.data_store =
Prometheus::Client::DataStores::DirectFileStore.new(dir: PROMETHEUS_STORE_DIR)
@prom = ::Prometheus::Client.registry
Expand Down