Merge pull request #1063 from jbochi/metrics_percentile_fix

Fix percentiles on metrics filter

lib/logstash/filters/metrics.rb

@@ -48,6 +48,7 @@ require "logstash/namespace"
 # * "thing.max" - the maximum value seen for this metric
 # * "thing.stddev" - the standard deviation for this metric
 # * "thing.mean" - the mean for this metric
+# * "thing.pXX" - the XXth percentile for this metric (see `percentiles`)
 #
 # #### Example: computing event rate
 #
@@ -197,7 +198,7 @@ class LogStash::Filters::Metrics < LogStash::Filters::Base
       event["#{name}.mean"] = metric.mean
 
       @percentiles.each do |percentile|
-        event["#{name}.p#{percentile}"] = metric.snapshot.value(percentile / 100)
+        event["#{name}.p#{percentile}"] = metric.snapshot.value(percentile / 100.0)
       end
       metric.clear if should_clear?
     end

spec/filters/metrics.rb

@@ -111,12 +111,12 @@ describe LogStash::Filters::Metrics do
   context "with timer config" do
     context "on the first flush" do
       subject {
-        config = {"timer" => ["http.request_time", "request_time"]}
+        config = {"timer" => ["http.request_time", "%{request_time}"]}
         filter = LogStash::Filters::Metrics.new config
         filter.register
-        filter.filter LogStash::Event.new({"request_time" => 1})
+        filter.filter LogStash::Event.new({"request_time" => 10})
-        filter.filter LogStash::Event.new({"request_time" => 2})
+        filter.filter LogStash::Event.new({"request_time" => 20})
-        filter.filter LogStash::Event.new({"request_time" => 3})
+        filter.filter LogStash::Event.new({"request_time" => 30})
         filter.flush
       }
 
@@ -125,12 +125,32 @@ describe LogStash::Filters::Metrics do
         insist { subject.first["http.request_time.count"] } == 3
       end
 
-      it "should include rates and percentiles" do
+      it "should include rates and percentiles keys" do
         metrics = ["rate_1m", "rate_5m", "rate_15m", "p1", "p5", "p10", "p90", "p95", "p99"]
         metrics.each do |metric|
           insist { subject.first }.include? "http.request_time.#{metric}"
         end
       end
+
+      it "should include min value" do
+        insist { subject.first['http.request_time.min'] } == 10.0
+      end
+
+      it "should include mean value" do
+        insist { subject.first['http.request_time.mean'] } == 20.0
+      end
+
+      it "should include stddev value" do
+        insist { subject.first['http.request_time.stddev'] } == Math.sqrt(10.0)
+      end
+
+      it "should include max value" do
+        insist { subject.first['http.request_time.max'] } == 30.0
+      end
+
+      it "should include percentile value" do
+        insist { subject.first['http.request_time.p99'] } == 30.0
+      end
     end
   end