Browse files

Merge pull request #872 from kbarber/ticket/master/PDB-476-terminus-p…

…rofiling

PDB-476 Decorate the terminus code with Puppet profiling blocks
  • Loading branch information...
2 parents 3c8bd7a + bfc45a0 commit 95ed9db45f343a39b7dffeb60721c4921858b96d @senior senior committed Feb 27, 2014
View
8 documentation/puppetdb-faq.markdown
@@ -138,3 +138,11 @@ The catalog duplication rate can be found on the
heavier I/O load on the database. Refer to the [Troubleshooting Low
Catalog Duplication guide][low_catalog_dupe] for steps to diagnose the
problem.
+
+## My puppet master is going slow since enabling PuppetDB. How can I profile it?
+
+In Puppet 3.x a new profiling capability was introduced that we have leveraged in the PuppetDB terminus client code. By simply adding `profile=true` to your `puppet.conf` you can enable detailed profiling of all apsects of Puppet including the PuppetDB terminus. For this to work you must enable debugging on your master instance as well.
+
+Of course use your common sense, any profiling mechanism will add more load which can increase your problems when you already have limited capacity. Enabling profiling in production should only be done with care and for a very short amount of time.
+
+All PuppetDB profiling events are prefixed with `PuppetDB:` so can by easily searched for. This information is helpful to our developers to debug performance issues also, so feel free to include these details when raising tickets against PuppetDB.
View
369 puppet/lib/puppet/indirector/catalog/puppetdb.rb
@@ -12,9 +12,11 @@ def initialize
end
def save(request)
- catalog = munge_catalog(request.instance, extract_extra_request_data(request))
+ profile "catalog#save" do
+ catalog = munge_catalog(request.instance, extract_extra_request_data(request))
- submit_command(request.key, catalog, CommandReplaceCatalog, 3)
+ submit_command(request.key, catalog, CommandReplaceCatalog, 3)
+ end
end
def find(request)
@@ -29,20 +31,24 @@ def extract_extra_request_data(request)
end
def munge_catalog(catalog, extra_request_data = {})
- hash = catalog.to_pson_data_hash
+ profile "Munge catalog" do
+ hash = profile "Convert catalog to PSON data hash" do
+ catalog.to_pson_data_hash
+ end
- data = hash['data']
+ data = hash['data']
- add_parameters_if_missing(data)
- add_namevar_aliases(data, catalog)
- stringify_titles(data)
- sort_unordered_metaparams(data)
- munge_edges(data)
- synthesize_edges(data, catalog)
- filter_keys(hash)
- add_transaction_uuid(data, extra_request_data[:transaction_uuid])
+ add_parameters_if_missing(data)
+ add_namevar_aliases(data, catalog)
+ stringify_titles(data)
+ sort_unordered_metaparams(data)
+ munge_edges(data)
+ synthesize_edges(data, catalog)
+ filter_keys(hash)
+ add_transaction_uuid(data, extra_request_data[:transaction_uuid])
- hash
+ hash
+ end
end
Relationships = {
@@ -63,214 +69,245 @@ def add_transaction_uuid(hash, transaction_uuid)
end
def stringify_titles(hash)
- hash['resources'].each do |resource|
- resource['title'] = resource['title'].to_s
+ resources = hash['resources']
+ profile "Stringify titles (resource count: #{resources.count})" do
+ resources.each do |resource|
+ resource['title'] = resource['title'].to_s
+ end
end
hash
end
def add_parameters_if_missing(hash)
- hash['resources'].each do |resource|
- resource['parameters'] ||= {}
+ resources = hash['resources']
+ profile "Add parameters if missing (resource count: #{resources.count})" do
+ resources.each do |resource|
+ resource['parameters'] ||= {}
+ end
end
hash
end
def add_namevar_aliases(hash, catalog)
- hash['resources'].each do |resource|
- real_resource = catalog.resource(resource['type'], resource['title'])
-
- # Resources with composite namevars can't be referred to by
- # anything other than their title when declaring
- # relationships. Trying to snag the :alias for these resources
- # will only return _part_ of the name (a problem with Puppet
- # proper), so skipping the adding of aliases for these resources
- # is both an optimization and a safeguard.
- next if real_resource.key_attributes.count > 1
-
- aliases = [real_resource[:alias]].flatten.compact
-
- # Non-isomorphic resources aren't unique based on namevar, so we can't
- # use it as an alias
- type = real_resource.resource_type
- if !type.respond_to?(:isomorphic?) or type.isomorphic?
- # This makes me a little sad. It turns out that the "to_hash" method
- # of Puppet::Resource can have side effects. In particular, if the
- # resource type specifies a title_pattern, calling "to_hash" will trigger
- # the title_pattern processing, which can have the side effect of
- # populating the namevar (potentially with a munged value). Thus,
- # it is important that we search for namevar aliases in that hash
- # rather than in the resource itself.
- real_resource_hash = real_resource.to_hash
-
- name = real_resource_hash[real_resource.send(:namevar)]
- unless name.nil? or real_resource.title == name or aliases.include?(name)
- aliases << name
+ resources = hash['resources']
+ profile "Add namevar aliases (resource count: #{resources.count})" do
+ resources.each do |resource|
+ real_resource = catalog.resource(resource['type'], resource['title'])
+
+ # Resources with composite namevars can't be referred to by
+ # anything other than their title when declaring
+ # relationships. Trying to snag the :alias for these resources
+ # will only return _part_ of the name (a problem with Puppet
+ # proper), so skipping the adding of aliases for these resources
+ # is both an optimization and a safeguard.
+ next if real_resource.key_attributes.count > 1
+
+ aliases = [real_resource[:alias]].flatten.compact
+
+ # Non-isomorphic resources aren't unique based on namevar, so we can't
+ # use it as an alias
+ type = real_resource.resource_type
+ if !type.respond_to?(:isomorphic?) or type.isomorphic?
+ # This makes me a little sad. It turns out that the "to_hash" method
+ # of Puppet::Resource can have side effects. In particular, if the
+ # resource type specifies a title_pattern, calling "to_hash" will trigger
+ # the title_pattern processing, which can have the side effect of
+ # populating the namevar (potentially with a munged value). Thus,
+ # it is important that we search for namevar aliases in that hash
+ # rather than in the resource itself.
+ real_resource_hash = real_resource.to_hash
+
+ name = real_resource_hash[real_resource.send(:namevar)]
+ unless name.nil? or real_resource.title == name or aliases.include?(name)
+ aliases << name
+ end
end
- end
- resource['parameters']['alias'] = aliases unless aliases.empty?
+ resource['parameters']['alias'] = aliases unless aliases.empty?
+ end
end
hash
end
def sort_unordered_metaparams(hash)
- hash['resources'].each do |resource|
- params = resource['parameters']
- UnorderedMetaparams.each do |metaparam|
- if params[metaparam].kind_of? Array then
- values = params[metaparam].sort
- params[metaparam] = values unless values.empty?
+ resources = hash['resources']
+ profile "Sort unordered metaparams (resource count: #{resources.count})" do
+ resources.each do |resource|
+ params = resource['parameters']
+ UnorderedMetaparams.each do |metaparam|
+ if params[metaparam].kind_of? Array then
+ values = params[metaparam].sort
+ params[metaparam] = values unless values.empty?
+ end
end
end
end
+
hash
end
def munge_edges(hash)
- hash['edges'].each do |edge|
- %w[source target].each do |vertex|
- edge[vertex] = resource_ref_to_hash(edge[vertex]) if edge[vertex].is_a?(String)
+ edges = hash['edges']
+ profile "Munge edges (edge count: #{edges.count})" do
+ edges.each do |edge|
+ %w[source target].each do |vertex|
+ edge[vertex] = resource_ref_to_hash(edge[vertex]) if edge[vertex].is_a?(String)
+ end
+ edge['relationship'] ||= 'contains'
end
- edge['relationship'] ||= 'contains'
- end
- hash
+ hash
+ end
end
def map_aliases_to_title(hash)
+ resources = hash['resources']
aliases = {}
- hash['resources'].each do |resource|
- names = resource['parameters']['alias'] || []
- resource_hash = {'type' => resource['type'], 'title' => resource['title']}
- names.each do |name|
- alias_array = [resource['type'], name]
- aliases[alias_array] = resource_hash
+
+ profile "Map aliases to title (resource count: #{resources.count})" do
+ resources.each do |resource|
+ names = resource['parameters']['alias'] || []
+ resource_hash = {'type' => resource['type'], 'title' => resource['title']}
+ names.each do |name|
+ alias_array = [resource['type'], name]
+ aliases[alias_array] = resource_hash
+ end
end
end
+
aliases
end
def synthesize_edges(hash, catalog)
- aliases = map_aliases_to_title(hash)
-
- resource_table = {}
- hash['resources'].each do |resource|
- resource_table[ [resource['type'], resource['title']] ] = resource
- end
+ profile "Synthesize edges" do
+ aliases = map_aliases_to_title(hash)
- hash['resources'].each do |resource|
- # Standard virtual resources don't appear in the catalog. However,
- # exported resources which haven't been also collected will appears as
- # exported and virtual (collected ones will only be exported). They will
- # eventually be removed from the catalog, so we can't add edges involving
- # them. Puppet::Resource#to_pson_data_hash omits 'virtual', so we have to
- # look it up in the catalog to find that information. This isn't done in
- # a separate step because we don't actually want to send the field (it
- # will always be false). See ticket #16472.
- #
- # The outer conditional is here because Class[main] can't properly be
- # looked up using catalog.resource and will return nil. See ticket
- # #16473. Yay.
- if real_resource = catalog.resource(resource['type'], resource['title'])
- next if real_resource.virtual?
+ resource_table = {}
+ profile "Build up resource_table" do
+ hash['resources'].each do |resource|
+ resource_table[ [resource['type'], resource['title']] ] = resource
+ end
end
- Relationships.each do |param,relation|
- if value = resource['parameters'][param]
- [value].flatten.each do |other_ref|
- edge = {'relationship' => relation[:relationship]}
-
- resource_hash = {'type' => resource['type'], 'title' => resource['title']}
- other_hash = resource_ref_to_hash(other_ref)
-
- # Puppet doesn't always seem to check this correctly. If we don't
- # users will later get an invalid relationship error instead.
- #
- # Primarily we are trying to catch the non-capitalized resourceref
- # case problem here: http://projects.puppetlabs.com/issues/19474
- # Once that problem is solved and older versions of Puppet that have
- # the bug are no longer supported we can probably remove this code.
- unless other_ref =~ /^[A-Z][a-z0-9_]*(::[A-Z][a-z0-9_]*)*\[.*\]/
- rel = edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)
- raise Puppet::Error, "Invalid relationship: #{rel}, because " +
- "#{other_ref} doesn't seem to be in the correct format. " +
- "Resource references should be formatted as: " +
- "Classname['title'] or Modulename::Classname['title'] (take " +
- "careful note of the capitalization)."
- end
-
- # This is an unfortunate hack. Puppet does some weird things w/rt
- # munging off trailing slashes from file resources, and users may
- # legally specify relationships using a different number of trailing
- # slashes than the resource was originally declared with.
- # We do know that for any file resource in the catalog, there should
- # be a canonical entry for it that contains no trailing slashes. So,
- # here, in case someone has specified a relationship to a file resource
- # and has used one or more trailing slashes when specifying the
- # relationship, we will munge off the trailing slashes before
- # we look up the resource in the catalog to create the edge.
- if other_hash['type'] == 'File' and other_hash['title'] =~ /\/$/
- other_hash['title'] = other_hash['title'].sub(/\/+$/, '')
- end
-
- other_array = [other_hash['type'], other_hash['title']]
-
- # Try to find the resource by type/title or look it up as an alias
- # and try that
- other_resource = resource_table[other_array]
- if other_resource.nil? and alias_hash = aliases[other_array]
- other_resource = resource_table[ alias_hash.values_at('type', 'title') ]
- end
+ profile "Primary synthesis" do
+ hash['resources'].each do |resource|
+ # Standard virtual resources don't appear in the catalog. However,
+ # exported resources which haven't been also collected will appears as
+ # exported and virtual (collected ones will only be exported). They will
+ # eventually be removed from the catalog, so we can't add edges involving
+ # them. Puppet::Resource#to_pson_data_hash omits 'virtual', so we have to
+ # look it up in the catalog to find that information. This isn't done in
+ # a separate step because we don't actually want to send the field (it
+ # will always be false). See ticket #16472.
+ #
+ # The outer conditional is here because Class[main] can't properly be
+ # looked up using catalog.resource and will return nil. See ticket
+ # #16473. Yay.
+ if real_resource = catalog.resource(resource['type'], resource['title'])
+ next if real_resource.virtual?
+ end
- raise Puppet::Error, "Invalid relationship: #{edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)}, because #{other_ref} doesn't seem to be in the catalog" unless other_resource
-
- # As above, virtual exported resources will eventually be removed,
- # so if a real resource refers to one, it's wrong. Non-virtual
- # exported resources are exported resources that were also
- # collected in this catalog, so they're okay. Virtual non-exported
- # resources can't appear in the catalog in the first place, so it
- # suffices to check for virtual.
- if other_real_resource = catalog.resource(other_resource['type'], other_resource['title'])
- if other_real_resource.virtual?
- raise Puppet::Error, "Invalid relationship: #{edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)}, because #{other_ref} is exported but not collected"
+ Relationships.each do |param,relation|
+ if value = resource['parameters'][param]
+ [value].flatten.each do |other_ref|
+ edge = {'relationship' => relation[:relationship]}
+
+ resource_hash = {'type' => resource['type'], 'title' => resource['title']}
+ other_hash = resource_ref_to_hash(other_ref)
+
+ # Puppet doesn't always seem to check this correctly. If we don't
+ # users will later get an invalid relationship error instead.
+ #
+ # Primarily we are trying to catch the non-capitalized resourceref
+ # case problem here: http://projects.puppetlabs.com/issues/19474
+ # Once that problem is solved and older versions of Puppet that have
+ # the bug are no longer supported we can probably remove this code.
+ unless other_ref =~ /^[A-Z][a-z0-9_]*(::[A-Z][a-z0-9_]*)*\[.*\]/
+ rel = edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)
+ raise Puppet::Error, "Invalid relationship: #{rel}, because " +
+ "#{other_ref} doesn't seem to be in the correct format. " +
+ "Resource references should be formatted as: " +
+ "Classname['title'] or Modulename::Classname['title'] (take " +
+ "careful note of the capitalization)."
+ end
+
+ # This is an unfortunate hack. Puppet does some weird things w/rt
+ # munging off trailing slashes from file resources, and users may
+ # legally specify relationships using a different number of trailing
+ # slashes than the resource was originally declared with.
+ # We do know that for any file resource in the catalog, there should
+ # be a canonical entry for it that contains no trailing slashes. So,
+ # here, in case someone has specified a relationship to a file resource
+ # and has used one or more trailing slashes when specifying the
+ # relationship, we will munge off the trailing slashes before
+ # we look up the resource in the catalog to create the edge.
+ if other_hash['type'] == 'File' and other_hash['title'] =~ /\/$/
+ other_hash['title'] = other_hash['title'].sub(/\/+$/, '')
+ end
+
+ other_array = [other_hash['type'], other_hash['title']]
+
+ # Try to find the resource by type/title or look it up as an alias
+ # and try that
+ other_resource = resource_table[other_array]
+ if other_resource.nil? and alias_hash = aliases[other_array]
+ other_resource = resource_table[ alias_hash.values_at('type', 'title') ]
+ end
+
+ raise Puppet::Error, "Invalid relationship: #{edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)}, because #{other_ref} doesn't seem to be in the catalog" unless other_resource
+
+ # As above, virtual exported resources will eventually be removed,
+ # so if a real resource refers to one, it's wrong. Non-virtual
+ # exported resources are exported resources that were also
+ # collected in this catalog, so they're okay. Virtual non-exported
+ # resources can't appear in the catalog in the first place, so it
+ # suffices to check for virtual.
+ if other_real_resource = catalog.resource(other_resource['type'], other_resource['title'])
+ if other_real_resource.virtual?
+ raise Puppet::Error, "Invalid relationship: #{edge_to_s(resource_hash_to_ref(resource_hash), other_ref, param)}, because #{other_ref} is exported but not collected"
+ end
+ end
+
+ # If the ref was an alias, it will have a different title, so use
+ # that
+ other_hash['title'] = other_resource['title']
+
+ if relation[:direction] == :forward
+ edge.merge!('source' => resource_hash, 'target' => other_hash)
+ else
+ edge.merge!('source' => other_hash, 'target' => resource_hash)
+ end
+ hash['edges'] << edge
end
end
-
- # If the ref was an alias, it will have a different title, so use
- # that
- other_hash['title'] = other_resource['title']
-
- if relation[:direction] == :forward
- edge.merge!('source' => resource_hash, 'target' => other_hash)
- else
- edge.merge!('source' => other_hash, 'target' => resource_hash)
- end
- hash['edges'] << edge
end
end
end
- end
- hash['edges'].uniq!
+ profile "Make edges unique" do
+ hash['edges'].uniq!
+ end
- hash
+ hash
+ end
end
def filter_keys(hash)
- hash['metadata'].delete_if do |k,v|
- k != 'api_version'
- end
+ profile "Filter extraneous keys from the catalog" do
+ hash['metadata'].delete_if do |k,v|
+ k != 'api_version'
+ end
- hash['data'].delete_if do |k,v|
- ! ['name', 'version', 'edges', 'resources'].include?(k)
- end
+ hash['data'].delete_if do |k,v|
+ ! ['name', 'version', 'edges', 'resources'].include?(k)
+ end
- hash.delete_if do |k,v|
- ! ['metadata', 'data'].include?(k)
+ hash.delete_if do |k,v|
+ ! ['metadata', 'data'].include?(k)
+ end
end
end
View
131 puppet/lib/puppet/indirector/facts/puppetdb.rb
@@ -1,3 +1,4 @@
+require 'uri'
require 'puppet/node/facts'
require 'puppet/indirector/rest'
require 'puppet/util/puppetdb'
@@ -13,40 +14,51 @@ def initialize
end
def save(request)
- facts = request.instance.dup
- facts.values = facts.values.dup
- facts.stringify
- payload = {"name" => facts.name, "values" => facts.values}.to_pson
+ profile "facts#save" do
+ payload = profile "Encode facts command submission payload" do
+ facts = request.instance.dup
+ facts.values = facts.values.dup
+ facts.stringify
+ {"name" => facts.name, "values" => facts.values}.to_json
+ end
- submit_command(request.key, payload, CommandReplaceFacts, 1)
+ submit_command(request.key, payload, CommandReplaceFacts, 1)
+ end
end
def find(request)
- begin
- response = http_get(request, "/v3/nodes/#{CGI.escape(request.key)}/facts", headers)
- log_x_deprecation_header(response)
-
- if response.is_a? Net::HTTPSuccess
- result = JSON.parse(response.body)
- # Note: the Inventory Service API appears to expect us to return nil here
- # if the node isn't found. However, PuppetDB returns an empty array in
- # this case; for now we will just look for that condition and assume that
- # it means that the node wasn't found, so we will return nil. In the
- # future we may want to improve the logic such that we can distinguish
- # between the "node not found" and the "no facts for this node" cases.
- if result.empty?
- return nil
+ profile "facts#find" do
+ begin
+ url = "/v3/nodes/#{CGI.escape(request.key)}/facts"
+ response = profile "Query for nodes facts: #{url}" do
+ http_get(request, url, headers)
end
- facts = result.inject({}) do |a,h|
- a.merge(h['name'] => h['value'])
+ log_x_deprecation_header(response)
+
+ if response.is_a? Net::HTTPSuccess
+ profile "Parse fact query response (size: #{response.body.size})" do
+ result = JSON.parse(response.body)
+ # Note: the Inventory Service API appears to expect us to return nil here
+ # if the node isn't found. However, PuppetDB returns an empty array in
+ # this case; for now we will just look for that condition and assume that
+ # it means that the node wasn't found, so we will return nil. In the
+ # future we may want to improve the logic such that we can distinguish
+ # between the "node not found" and the "no facts for this node" cases.
+ if result.empty?
+ return nil
+ end
+ facts = result.inject({}) do |a,h|
+ a.merge(h['name'] => h['value'])
+ end
+ Puppet::Node::Facts.new(request.key, facts)
+ end
+ else
+ # Newline characters cause an HTTP error, so strip them
+ raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
end
- Puppet::Node::Facts.new(request.key, facts)
- else
- # Newline characters cause an HTTP error, so strip them
- raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
+ rescue => e
+ raise Puppet::Error, "Failed to find facts from PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
end
- rescue => e
- raise Puppet::Error, "Failed to find facts from PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
end
end
@@ -62,40 +74,47 @@ def find(request)
# `operator` may be one of {eq, ne, lt, gt, le, ge}, and will default to 'eq'
# if unspecified.
def search(request)
- return [] unless request.options
- operator_map = {
- 'eq' => '=',
- 'gt' => '>',
- 'lt' => '<',
- 'ge' => '>=',
- 'le' => '<=',
- }
- filters = request.options.sort.map do |key,value|
- type, name, operator = key.to_s.split('.')
- operator ||= 'eq'
- raise Puppet::Error, "Fact search against keys of type '#{type}' is unsupported" unless type == 'facts'
- if operator == 'ne'
- ['not', ['=', ['fact', name], value]]
- else
- [operator_map[operator], ['fact', name], value]
+ profile "facts#search" do
+ return [] unless request.options
+ operator_map = {
+ 'eq' => '=',
+ 'gt' => '>',
+ 'lt' => '<',
+ 'ge' => '>=',
+ 'le' => '<=',
+ }
+ filters = request.options.sort.map do |key,value|
+ type, name, operator = key.to_s.split('.')
+ operator ||= 'eq'
+ raise Puppet::Error, "Fact search against keys of type '#{type}' is unsupported" unless type == 'facts'
+ if operator == 'ne'
+ ['not', ['=', ['fact', name], value]]
+ else
+ [operator_map[operator], ['fact', name], value]
+ end
end
- end
- query = ["and"] + filters
- query_param = CGI.escape(query.to_json)
+ query = ["and"] + filters
+ query_param = CGI.escape(query.to_json)
- begin
- response = http_get(request, "/v3/nodes?query=#{query_param}", headers)
- log_x_deprecation_header(response)
+ begin
+ url = "/v3/nodes?query=#{query_param}"
+ response = profile "Fact query request: #{URI.unescape(url)}" do
+ http_get(request, url, headers)
+ end
+ log_x_deprecation_header(response)
- if response.is_a? Net::HTTPSuccess
- JSON.parse(response.body).collect {|s| s["name"]}
- else
- # Newline characters cause an HTTP error, so strip them
- raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
+ if response.is_a? Net::HTTPSuccess
+ profile "Parse fact query response (size: #{response.body.size})" do
+ JSON.parse(response.body).collect {|s| s["name"]}
+ end
+ else
+ # Newline characters cause an HTTP error, so strip them
+ raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
+ end
+ rescue => e
+ raise Puppet::Error, "Could not perform inventory search from PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
end
- rescue => e
- raise Puppet::Error, "Could not perform inventory search from PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
end
end
View
82 puppet/lib/puppet/indirector/resource/puppetdb.rb
@@ -1,6 +1,7 @@
require 'puppet/indirector/rest'
require 'puppet/util/puppetdb'
require 'json'
+require 'uri'
class Puppet::Resource::Puppetdb < Puppet::Indirector::REST
include Puppet::Util::Puppetdb
@@ -11,46 +12,55 @@ def initialize
end
def search(request)
- type = request.key
- host = request.options[:host]
- filter = request.options[:filter]
- scope = request.options[:scope]
-
- # At minimum, we want to filter to the right type of exported resources.
- expr = ['and',
- ['=', 'type', type],
- ['=', 'exported', true],
- ['not',
- ['=', 'certname', host]]]
-
- filter_expr = build_expression(filter)
- expr << filter_expr if filter_expr
-
- query_param = CGI.escape(expr.to_json)
-
- begin
- response = http_get(request, "/v3/resources?query=#{query_param}", headers)
- log_x_deprecation_header(response)
-
- unless response.is_a? Net::HTTPSuccess
- # Newline characters cause an HTTP error, so strip them
- raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
+ profile "resource#search" do
+ type = request.key
+ host = request.options[:host]
+ filter = request.options[:filter]
+ scope = request.options[:scope]
+
+ # At minimum, we want to filter to the right type of exported resources.
+ expr = ['and',
+ ['=', 'type', type],
+ ['=', 'exported', true],
+ ['not',
+ ['=', 'certname', host]]]
+
+ filter_expr = build_expression(filter)
+ expr << filter_expr if filter_expr
+
+ query_param = CGI.escape(expr.to_json)
+
+ begin
+ url = "/v3/resources?query=#{query_param}"
+ response = profile "Resources query: #{URI.unescape(url)}" do
+ http_get(request, url, headers)
+ end
+ log_x_deprecation_header(response)
+
+ unless response.is_a? Net::HTTPSuccess
+ # Newline characters cause an HTTP error, so strip them
+ raise "[#{response.code} #{response.message}] #{response.body.gsub(/[\r\n]/, '')}"
+ end
+ rescue => e
+ raise Puppet::Error, "Could not retrieve resources from the PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
end
- rescue => e
- raise Puppet::Error, "Could not retrieve resources from the PuppetDB at #{self.class.server}:#{self.class.port}: #{e}"
- end
- resources = JSON.load(response.body)
+ resources = profile "Parse resource query response (size: #{response.body.size})" do
+ JSON.load(response.body)
+ end
- resources.map do |res|
- params = res['parameters'] || {}
- params = params.map do |name,value|
- Puppet::Parser::Resource::Param.new(:name => name, :value => value)
+ profile "Build up collected resource objects (count: #{resources.count})" do
+ resources.map do |res|
+ params = res['parameters'] || {}
+ params = params.map do |name,value|
+ Puppet::Parser::Resource::Param.new(:name => name, :value => value)
+ end
+ attrs = {:parameters => params, :scope => scope}
+ result = Puppet::Parser::Resource.new(res['type'], res['title'], attrs)
+ result.collector_id = "#{res['certname']}|#{res['type']}|#{res['title']}"
+ result
+ end
end
- attrs = {:parameters => params, :scope => scope}
- result = Puppet::Parser::Resource.new(res['type'], res['title'], attrs)
- result.collector_id = "#{res['certname']}|#{res['type']}|#{res['title']}"
- result
end
end
View
64 puppet/lib/puppet/reports/puppetdb.rb
@@ -17,7 +17,9 @@
def process
- submit_command(self.host, report_to_hash, CommandStoreReport, 2)
+ profile "report#process" do
+ submit_command(self.host, report_to_hash, CommandStoreReport, 2)
+ end
end
# TODO: It seems unfortunate that we have to access puppet_version and
@@ -44,36 +46,40 @@ def puppet_version
#
# @api private
def report_to_hash
- add_v4_fields_to_report(
- {
- "certname" => host,
- "puppet-version" => puppet_version,
- "report-format" => report_format,
- "configuration-version" => configuration_version.to_s,
- "start-time" => Puppet::Util::Puppetdb.to_wire_time(time),
- "end-time" => Puppet::Util::Puppetdb.to_wire_time(time + run_duration),
- "resource-events" => build_events_list
- })
+ profile "Convert report to wire format hash" do
+ add_v4_fields_to_report(
+ {
+ "certname" => host,
+ "puppet-version" => puppet_version,
+ "report-format" => report_format,
+ "configuration-version" => configuration_version.to_s,
+ "start-time" => Puppet::Util::Puppetdb.to_wire_time(time),
+ "end-time" => Puppet::Util::Puppetdb.to_wire_time(time + run_duration),
+ "resource-events" => build_events_list
+ })
+ end
end
# @api private
def build_events_list
- filter_events(resource_statuses.inject([]) do |events, status_entry|
- _, status = *status_entry
- if ! (status.events.empty?)
- events.concat(status.events.map { |event| event_to_hash(status, event) })
- elsif status.skipped
- events.concat([fabricate_event(status, "skipped")])
- elsif status.failed
- # PP-254:
- # We have to fabricate resource events here due to a bug/s in report providers
- # that causes them not to include events on a resource status that has failed.
- # When PuppetDB is able to make a hard break from older version of Puppet that
- # have this bug, we can remove this behavior.
- events.concat([fabricate_event(status, "failure")])
- end
- events
- end)
+ profile "Build events list (count: #{resource_statuses.count})" do
+ filter_events(resource_statuses.inject([]) do |events, status_entry|
+ _, status = *status_entry
+ if ! (status.events.empty?)
+ events.concat(status.events.map { |event| event_to_hash(status, event) })
+ elsif status.skipped
+ events.concat([fabricate_event(status, "skipped")])
+ elsif status.failed
+ # PP-254:
+ # We have to fabricate resource events here due to a bug/s in report providers
+ # that causes them not to include events on a resource status that has failed.
+ # When PuppetDB is able to make a hard break from older version of Puppet that
+ # have this bug, we can remove this behavior.
+ events.concat([fabricate_event(status, "failure")])
+ end
+ events
+ end)
+ end
end
# @api private
@@ -162,7 +168,9 @@ def add_v4_fields_to_event(resource_status, event_hash)
# @api private
def filter_events(events)
if config.ignore_blacklisted_events?
- events.select { |e| ! config.is_event_blacklisted?(e) }
+ profile "Filter blacklisted events" do
+ events.select { |e| ! config.is_event_blacklisted?(e) }
+ end
else
events
end
View
28 puppet/lib/puppet/util/puppetdb.rb
@@ -1,5 +1,6 @@
require 'puppet/util'
require 'puppet/util/logging'
+require 'puppet/util/profiler'
require 'puppet/util/puppetdb/global_check'
require 'puppet/util/puppetdb/command_names'
require 'puppet/util/puppetdb/command'
@@ -61,9 +62,32 @@ def self.to_bool(value)
# @!group Public instance methods
+ # Submit a command to PuppetDB.
+ #
+ # @param certname [String] hostname name of puppetdb instance
+ # @param payload [String] payload
+ # @param command_name [String] name of command
+ # @param version [Number] version number of command
def submit_command(certname, payload, command_name, version)
- command = Puppet::Util::Puppetdb::Command.new(command_name, version, certname, payload)
- command.submit
+ profile "Submitted command '#{command_name}' version '#{version}'" do
+ command = Puppet::Util::Puppetdb::Command.new(command_name, version, certname, payload)
+ command.submit
+ end
+ end
+
+ # Profile a block of code and log the time it took to execute.
+ #
+ # This outputs logs entries to the Puppet masters logging destination
+ # providing the time it took, a message describing the profiled code
+ # and a leaf location marking where the profile method was called
+ # in the profiled hierachy.
+ #
+ # @param message [String] A description of the profiled event
+ # @param block [Block] The segment of code to profile
+ # @api public
+ def profile(message, &block)
+ message = "PuppetDB: " + message
+ Puppet::Util::Profiler.profile(message, &block)
end
# @!group Private instance methods
View
17 puppet/lib/puppet/util/puppetdb/command.rb
@@ -6,6 +6,7 @@
require 'json'
class Puppet::Util::Puppetdb::Command
+ include Puppet::Util::Puppetdb
include Puppet::Util::Puppetdb::CommandNames
Url = "/v3/commands"
@@ -24,19 +25,27 @@ def initialize(command, version, certname, payload)
@command = command
@version = version
@certname = certname
- @payload = self.class.format_payload(command, version, payload)
+ profile "Format payload" do
+ @payload = self.class.format_payload(command, version, payload)
+ end
end
attr_reader :command, :version, :certname, :payload
def submit
checksum = Digest::SHA1.hexdigest(payload)
- escaped_payload = CGI.escape(payload)
+
+ escaped_payload = profile "URI escape command payload (size: #{payload.size})" do
+ CGI.escape(payload)
+ end
+
for_whom = " for #{certname}" if certname
begin
- http = Puppet::Network::HttpPool.http_instance(config.server, config.port)
- response = http.post(Url, "checksum=#{checksum}&payload=#{escaped_payload}", headers)
+ response = profile "Submit command HTTP post" do
+ http = Puppet::Network::HttpPool.http_instance(config.server, config.port)
+ http.post(Url, "checksum=#{checksum}&payload=#{escaped_payload}", headers)
+ end
Puppet::Util::Puppetdb.log_x_deprecation_header(response)

0 comments on commit 95ed9db

Please sign in to comment.