(PE-2675) Change the order that filters are applied for events

Gemfile

@@ -9,5 +9,6 @@ group :test do
 
   gem 'puppet', :require => false
 
-  gem 'beaker', '0.0.0'
+  gem 'beaker', '~> 1.0'
+  gem 'mocha', '~> 1.0'
 end

documentation/api/query/v3/aggregate-event-counts.markdown

@@ -33,6 +33,10 @@ This query is forwarded to the [`event-counts`][event-counts] endpoint - see the
 the final event-counts output, but before the results are aggregated. Supported operators are `=`, `>`, `<`,
 `>=`, and `<=`. Supported fields are `failures`, `successes`, `noops`, and `skips`.
 
+* `distinct-resources`: Optional.  (EXPERIMENTAL: it is possible that the behavior
+of this parameter may change in future releases.)  This parameter is passed along
+to the [`event`][events] query - see there for additional documentation.
+
 ##### Operators
 
 This endpoint builds on top of the [`event-counts`][event-counts] and therefore supports all of the same operators.

documentation/api/query/v3/event-counts.markdown

@@ -35,6 +35,10 @@ and `certname`.
 is applied to the final event counts output. Supported operators are `=`, `>`, `<`, `>=`, and `<=`.
 Supported fields are `failures`, `successes`, `noops`, and `skips`.
 
+* `distinct-resources`: Optional.  (EXPERIMENTAL: it is possible that the behavior
+of this parameter may change in future releases.)  This parameter is passed along
+to the [`event`][events] query - see there for additional documentation.
+
 ##### Operators
 
 This endpoint builds on top of the [`events`][events] endpoint and therefore supports all of the same operators.

documentation/api/query/v3/events.markdown

@@ -52,7 +52,24 @@ type 'Service':
 
 For more information on the available values for `FIELD`, see the [fields](#fields) section below.
 
-* `limit`: Optional.  If specified, this should be an integer value that will be used to override the `event-query-limit` [configuration setting](../../../configure.html#event_query_limit)
+* `distinct-resources`: Optional.  (EXPERIMENTAL: it is possible that the behavior
+of this parameter may change in future releases.) If specified, then in addition
+to the normal event query filtering, the result set will be limited to only
+returning the most recent event for a given resource on a given node.
+
+So, for example, if the resource `File[/tmp/foo]` was failing on a certain node,
+but has since been fixed and is now succeeding, then a "normal" event query might
+return both the success and failure events.  Using the `distinct-resources` flag
+ensures that your query only returns the most recent event for a given resource,
+so you would only get the success event in your result set.
+
+The `distinct-resources` query can be expensive; therefore, it requires two
+additional query parameters to go along with it:  `distinct-start-time` and
+`distinct-end-time`.  (Issuing a `distinct-resources` query without
+specifying both of these parameters will cause an error.)  The values of these
+parameters should be timestamp strings in the same format as used with the normal
+"timestamp" field, and will define a window of time in which to find the most
+recent event for each resource.
 
 ##### Operators
 

puppet/lib/puppet/indirector/catalog/puppetdb.rb

@@ -16,9 +16,7 @@ def find(request)
     nil
   end
 
-  # TODO: I think that almost everything below this line should be
-  #  private, but I don't want to break all the tests right now...
-
+  # @api private
   def extract_extra_request_data(request)
     {
       :transaction_uuid => request.options[:transaction_uuid]

puppet/lib/puppet/reports/puppetdb.rb

@@ -19,8 +19,6 @@ def process
     submit_command(self.host, report_to_hash, CommandStoreReport, 2)
   end
 
-  private
-
   # TODO: It seems unfortunate that we have to access puppet_version and
   # report_format directly as instance variables.  I've filed the following
   # ticket / pull req against puppet to expose them via accessors, which
@@ -29,16 +27,21 @@ def process
   # the accessors until version 3.x of puppet is our oldest supported version.
   #
   # This was resolved in puppet 3.x via ticket #16139 (puppet pull request #1073).
+
+  # @api private
   def report_format
     @report_format
   end
 
+  # @api private
   def puppet_version
     @puppet_version
   end
 
-  ### Convert `self` (an instance of `Puppet::Transaction::Report`) to a hash
-  ### suitable for sending over the wire to PuppetDB
+  # Convert `self` (an instance of `Puppet::Transaction::Report`) to a hash
+  # suitable for sending over the wire to PuppetDB
+  #
+  # @api private
   def report_to_hash
     add_v4_fields_to_report(
       {
@@ -52,6 +55,7 @@ def report_to_hash
       })
   end
 
+  # @api private
   def build_events_list
     filter_events(resource_statuses.inject([]) do |events, status_entry|
       _, status = *status_entry
@@ -71,6 +75,7 @@ def build_events_list
     end)
   end
 
+  # @api private
   def run_duration
     # TODO: this is wrong in puppet.  I am consistently seeing reports where
     # start-time + this value is less than the timestamp on the individual
@@ -81,8 +86,10 @@ def run_duration
     metrics["time"]["total"]
   end
 
-  ## Convert an instance of `Puppet::Transaction::Event` to a hash
-  ## suitable for sending over the wire to PuppetDB
+  # Convert an instance of `Puppet::Transaction::Event` to a hash
+  # suitable for sending over the wire to PuppetDB
+  #
+  # @api private
   def event_to_hash(resource_status, event)
     add_v4_fields_to_event(resource_status,
       {
@@ -99,9 +106,11 @@ def event_to_hash(resource_status, event)
       })
   end
 
-  ## Given an instance of `Puppet::Resource::Status` and a status string,
-  ## this method fabricates a PuppetDB event object with the provided
-  ## `"status"`.
+  # Given an instance of `Puppet::Resource::Status` and a status string,
+  # this method fabricates a PuppetDB event object with the provided
+  # `"status"`.
+  #
+  # @api private
   def fabricate_event(resource_status, event_status)
     add_v4_fields_to_event(resource_status,
       {
@@ -118,7 +127,9 @@ def fabricate_event(resource_status, event_status)
       })
   end
 
-  ## Backwards compatibility with versions of Puppet prior to report format 4
+  # Backwards compatibility with versions of Puppet prior to report format 4
+  #
+  # @api private
   def add_v4_fields_to_report(report_hash)
     if report_format >= 4
       report_hash.merge("transaction-uuid" => transaction_uuid)
@@ -127,7 +138,9 @@ def add_v4_fields_to_report(report_hash)
     end
   end
 
-  ## Backwards compatibility with versions of Puppet prior to report format 4
+  # Backwards compatibility with versions of Puppet prior to report format 4
+  #
+  # @api private
   def add_v4_fields_to_event(resource_status, event_hash)
     if report_format >= 4
       event_hash.merge("containment-path" => resource_status.containment_path)
@@ -136,7 +149,9 @@ def add_v4_fields_to_event(resource_status, event_hash)
     end
   end
 
-  ## Filter out blacklisted events, if we're configured to do so
+  # Filter out blacklisted events, if we're configured to do so
+  #
+  # @api private
   def filter_events(events)
     if config.ignore_blacklisted_events?
       events.select { |e| ! config.is_event_blacklisted?(e) }
@@ -145,7 +160,9 @@ def filter_events(events)
     end
   end
 
-  ## Helper method for accessing the puppetdb configuration
+  # Helper method for accessing the puppetdb configuration
+  #
+  # @api private
   def config
     Puppet::Util::Puppetdb.config
   end

puppet/lib/puppet/util/puppetdb.rb

@@ -39,8 +39,8 @@ def self.included(child)
     child.extend ClassMethods
   end
 
-  ## Given an instance of ruby's Time class, this method converts it to a String
-  ## that conforms to PuppetDB's wire format for representing a date/time.
+  # Given an instance of ruby's Time class, this method converts it to a String
+  # that conforms to PuppetDB's wire format for representing a date/time.
   def self.to_wire_time(time)
     # The current implementation simply calls iso8601, but having this method
     # allows us to change that in the future if needed w/o being forced to
@@ -58,22 +58,21 @@ def self.to_bool(value)
     end
   end
 
-  # Public instance methods
+  # @!group Public instance methods
 
   def submit_command(certname, payload, command_name, version)
     command = Puppet::Util::Puppetdb::Command.new(command_name, version, certname, payload)
     command.submit
   end
 
-  private
-
-  ## Private instance methods
+  # @!group Private instance methods
 
+  # @api private
   def config
     Puppet::Util::Puppetdb.config
   end
 
-
+  # @api private
   def log_x_deprecation_header(response)
     if warning = response['x-deprecation']
       Puppet.deprecation_warning "Deprecation from PuppetDB: #{warning}"