From ce82ad38dc37352bb32168f97b2e65bc308f2173 Mon Sep 17 00:00:00 2001
From: Luke Venediger <lukev@lukev.net>
Date: Sat, 13 Jul 2013 00:28:39 +0200
Subject: [PATCH] Fixed a bug in LatencyBucket where adding the first datapoint
 would skip all checks, and fixed a min/max problem. Closes #11

---
 .../Infrastructure/TestUtility.cs             | 12 +++++++++-
 .../TimedLatencyAggregatorBlockTests.cs       | 22 +++++++++++++++++++
 statsd.net.shared/Structures/LatencyBucket.cs | 21 ++++++++++++------
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/statsd.net-Tests/Infrastructure/TestUtility.cs b/statsd.net-Tests/Infrastructure/TestUtility.cs
index b5d573d..b9f3f2d 100644
--- a/statsd.net-Tests/Infrastructure/TestUtility.cs
+++ b/statsd.net-Tests/Infrastructure/TestUtility.cs
@@ -6,8 +6,10 @@
 
 namespace statsd.net_Tests.Infrastructure
 {
-  class TestUtility
+  internal class TestUtility
   {
+    private static Random _random = new Random();
+
     public static List<int> Range(int max, bool zeroBased = true)
     {
       var items = new List<int>();
@@ -25,5 +27,13 @@ public static TimeSpan OneSecondTimeout
         return new TimeSpan(0, 0, 1);
       }
     }
+
+    public static int NextInteger
+    {
+      get
+      {
+        return _random.Next();
+      }
+    }
   }
 }
diff --git a/statsd.net-Tests/TimedLatencyAggregatorBlockTests.cs b/statsd.net-Tests/TimedLatencyAggregatorBlockTests.cs
index 28da477..0dab913 100644
--- a/statsd.net-Tests/TimedLatencyAggregatorBlockTests.cs
+++ b/statsd.net-Tests/TimedLatencyAggregatorBlockTests.cs
@@ -108,5 +108,27 @@ public void WriteLatenciesToTwoBuckets_MeasurementsSeparate_Success()
 
       Assert.AreEqual(10, _outputBuffer.Items.Count);
     }
+
+
+    [TestMethod]
+    public void WriteMinAndMaxLatencies_Success()
+    {
+      _block = TimedLatencyAggregatorBlockFactory.CreateBlock(_outputBuffer,
+        String.Empty,
+        _intervalService,
+        _log.Object);
+      var pulseDate = DateTime.Now;
+
+      _block.Post(new Timing("foo.bar", 100));
+      _block.Post(new Timing("foo.bar", 200));
+      _block.WaitUntilAllItemsProcessed();
+      _intervalService.Pulse(pulseDate);
+
+      Assert.AreEqual(100, _outputBuffer["foo.bar.min"]);
+      Assert.AreEqual(200, _outputBuffer["foo.bar.max"]);
+      Assert.AreEqual(150, _outputBuffer["foo.bar.mean"]);
+      Assert.AreEqual(300, _outputBuffer["foo.bar.sum"]);
+      Assert.AreEqual(2, _outputBuffer["foo.bar.count"]);
+    }
   }
 }
diff --git a/statsd.net.shared/Structures/LatencyBucket.cs b/statsd.net.shared/Structures/LatencyBucket.cs
index 4925f50..e38fdfd 100644
--- a/statsd.net.shared/Structures/LatencyBucket.cs
+++ b/statsd.net.shared/Structures/LatencyBucket.cs
@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace statsd.net.shared.Structures
@@ -12,8 +14,10 @@ public class LatencyBucket : DatapointBox
 
     public int Min { get; private set; }
     public int Max { get; private set; }
-    public int Count { get; private set; }
-    public int Sum { get; private set; }
+    private int _count;
+    public int Count { get { return _count; } }
+    private int _sum;
+    public int Sum { get { return _sum; } }
     public int Mean
     {
       get
@@ -26,17 +30,20 @@ public LatencyBucket(int maxItems = 1000, int? firstDataPoint = null)
       : base(maxItems)
     {
       _sync = new Object();
-      if (firstDataPoint.HasValue) AddInternal(firstDataPoint.Value);
+      Min = -1;
+      Max = -1;
+      _count = 0;
+      if (firstDataPoint.HasValue) Add(firstDataPoint.Value);
     }
 
     public override void Add(int dataPoint)
     {
+      Interlocked.Add(ref _sum, dataPoint);
+      Interlocked.Increment(ref _count);
       lock (_sync)
       {
-        Sum += dataPoint;
-        Count++;
-        if (Min > dataPoint) Min = dataPoint;
-        if (Max < dataPoint) Max = dataPoint;
+        if (Min == -1 || dataPoint < Min) Min = dataPoint;
+        if (Max == -1 || dataPoint > Max) Max = dataPoint;
 
         base.AddInternal(dataPoint);
       }