diff --git a/src/Medidata.ZipkinTracer.Core/IZipkinConfig.cs b/src/Medidata.ZipkinTracer.Core/IZipkinConfig.cs index a4bd6f3..e07290d 100644 --- a/src/Medidata.ZipkinTracer.Core/IZipkinConfig.cs +++ b/src/Medidata.ZipkinTracer.Core/IZipkinConfig.cs @@ -22,7 +22,7 @@ public interface IZipkinConfig bool Create128BitTraceId { get; set; } - bool ShouldBeSampled(IOwinContext context, string sampled); + bool ShouldBeSampled(string sampled, string requestPath); void Validate(); } diff --git a/src/Medidata.ZipkinTracer.Core/TraceProvider.cs b/src/Medidata.ZipkinTracer.Core/TraceProvider.cs index 0691078..516f2c9 100644 --- a/src/Medidata.ZipkinTracer.Core/TraceProvider.cs +++ b/src/Medidata.ZipkinTracer.Core/TraceProvider.cs @@ -51,6 +51,7 @@ internal TraceProvider(IZipkinConfig config, IOwinContext context = null) string headerSpanId = null; string headerParentSpanId = null; string headerSampled = null; + string requestPath = null; if (context != null) { @@ -71,12 +72,14 @@ internal TraceProvider(IZipkinConfig config, IOwinContext context = null) headerSpanId = context.Request.Headers[SpanIdHeaderName]; headerParentSpanId = context.Request.Headers[ParentSpanIdHeaderName]; headerSampled = context.Request.Headers[SampledHeaderName]; + + requestPath = context.Request.Path.ToString(); } TraceId = headerTraceId.IsParsableTo128Or64Bit() ? headerTraceId : GenerateNewTraceId(config.Create128BitTraceId); SpanId = headerSpanId.IsParsableToLong() ? headerSpanId : GenerateHexEncodedInt64Id(); ParentSpanId = headerParentSpanId.IsParsableToLong() ? headerParentSpanId : string.Empty; - IsSampled = config.ShouldBeSampled(context, headerSampled); + IsSampled = config.ShouldBeSampled(headerSampled, requestPath); if (SpanId == ParentSpanId) { diff --git a/src/Medidata.ZipkinTracer.Core/ZipkinConfig.cs b/src/Medidata.ZipkinTracer.Core/ZipkinConfig.cs index 5686f39..681da45 100644 --- a/src/Medidata.ZipkinTracer.Core/ZipkinConfig.cs +++ b/src/Medidata.ZipkinTracer.Core/ZipkinConfig.cs @@ -7,6 +7,8 @@ namespace Medidata.ZipkinTracer.Core { public class ZipkinConfig : IZipkinConfig { + private Random random = new Random(); + public Predicate Bypass { get; set; } = r => false; public Uri ZipkinBaseUri { get; set; } public Func Domain { get; set; } @@ -49,28 +51,20 @@ public void Validate() } } - public bool ShouldBeSampled(IOwinContext context, string sampled) + /// + /// Checks if sampled flag from headers has value if not decide if need to sample or not using sample rate + /// + /// + /// + /// + public bool ShouldBeSampled(string sampledFlag, string requestPath) { - if (context == null) - { - return false; - } - bool result; - if (!string.IsNullOrWhiteSpace(sampled) && Boolean.TryParse(sampled, out result)) - { - return result; - } + if (TryParseSampledFlagToBool(sampledFlag, out result)) return result; - if (!IsInDontSampleList(context.Request.Path.ToString())) - { - var random = new Random(); - if (random.NextDouble() <= SampleRate) - { - return true; - } - } - return false; + if (IsInDontSampleList(requestPath)) return false; + + return random.NextDouble() <= SampleRate; } internal bool IsInDontSampleList(string path) @@ -84,5 +78,30 @@ internal bool IsInDontSampleList(string path) } return false; } + + /// + /// Try parse sampledFlag to bool + /// + /// + /// + /// returns true if sampledFlag can be parsed to bool + private bool TryParseSampledFlagToBool(string sampledFlag, out bool booleanValue) + { + booleanValue = false; + + if (string.IsNullOrWhiteSpace(sampledFlag)) return false; + + switch (sampledFlag) + { + case "0": + booleanValue = false; + return true; + case "1": + booleanValue = true; + return true; + default: + return bool.TryParse(sampledFlag, out booleanValue); + } + } } } \ No newline at end of file diff --git a/tests/Medidata.ZipkinTracer.Core.Test/TraceProviderTests.cs b/tests/Medidata.ZipkinTracer.Core.Test/TraceProviderTests.cs index 608fe6c..6d6d0e3 100644 --- a/tests/Medidata.ZipkinTracer.Core.Test/TraceProviderTests.cs +++ b/tests/Medidata.ZipkinTracer.Core.Test/TraceProviderTests.cs @@ -177,7 +177,7 @@ public void Constructor_AcceptingHeadersWithOutIsSampled() var expectedIsSampled = fixture.Create(); var sampleFilter = MockRepository.GenerateStub(); - sampleFilter.Expect(x => x.ShouldBeSampled(context, null)).Return(expectedIsSampled); + sampleFilter.Expect(x => x.ShouldBeSampled(Arg.Is.Null, Arg.Is.Anything)).Return(expectedIsSampled); // Act var sut = new TraceProvider(sampleFilter, context); @@ -207,7 +207,7 @@ public void Constructor_AcceptingHeadersWithInvalidIdValues() var expectedIsSampled = fixture.Create(); var sampleFilter = MockRepository.GenerateStub(); - sampleFilter.Expect(x => x.ShouldBeSampled(context, isSampled)).Return(expectedIsSampled); + sampleFilter.Expect(x => x.ShouldBeSampled(Arg.Is(isSampled), Arg.Is.Anything)).Return(expectedIsSampled); // Act var sut = new TraceProvider(sampleFilter, context); diff --git a/tests/Medidata.ZipkinTracer.Core.Test/ZipkinConfigTests.cs b/tests/Medidata.ZipkinTracer.Core.Test/ZipkinConfigTests.cs index bf07dcf..499eed8 100644 --- a/tests/Medidata.ZipkinTracer.Core.Test/ZipkinConfigTests.cs +++ b/tests/Medidata.ZipkinTracer.Core.Test/ZipkinConfigTests.cs @@ -2,8 +2,6 @@ using System.Collections.Generic; using Microsoft.VisualStudio.TestTools.UnitTesting; using Ploeh.AutoFixture; -using Rhino.Mocks; -using Microsoft.Owin; namespace Medidata.ZipkinTracer.Core.Test { @@ -73,195 +71,79 @@ public void ValidateWithNullNotToBeDisplayedDomainList() _sut.Validate(); } - [TestMethod] - public void ShouldBeSampled_NullContext() - { - // Arrange - var fixture = new Fixture(); - - // Act - var result = _sut.ShouldBeSampled(null, "true"); - - // Assert - Assert.IsFalse(result); - } - - [TestMethod] - public void ShouldBeSampled_NullSampledString() - { - // Arrange - var fixture = new Fixture(); - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, null); - - // Assert - Assert.IsFalse(result); - } - - [TestMethod] - public void ShouldBeSampled_WhiteSpaceSampledString() - { - // Arrange - var fixture = new Fixture(); - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, string.Empty); - - // Assert - Assert.IsFalse(result); - } - - [TestMethod] - public void ShouldBeSampled_InvalidSampledString() - { - // Arrange - var fixture = new Fixture(); - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, "weirdValue"); - - // Assert - Assert.IsFalse(result); - } - - [TestMethod] - public void ShouldBeSampled_PathNotInBlacktListSampleRate1_SampledStringTrue() - { - // Arrange - var fixture = new Fixture(); - - _sut.SampleRate = 1; - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result =_sut.ShouldBeSampled(context, "true"); - - // Assert - Assert.IsTrue(result); - } - - // TODO: Prepared for supporting 1/0 for sampled header - //[TestMethod] - //public void ShouldBeSampled_PathNotInBlacktListSampleRate1_SampledString1() - //{ - // // Arrange - // var fixture = new Fixture(); - - // _sut.SampleRate = 1; - - // var context = MockRepository.GenerateStub(); - // var request = MockRepository.GenerateStub(); - // request.Path = new PathString("/samplePath"); - // context.Stub(x => x.Request).Return(request); - - // // Act - // var result = _sut.ShouldBeSampled(context, "1"); - - // // Assert - // Assert.IsTrue(result); - //} - - [TestMethod] - public void ShouldBeSampled_PathNotInBlacktListSampleRate1_SampledStringFalse() - { - // Arrange - var fixture = new Fixture(); - - _sut.SampleRate = 1; - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, "false"); - - // Assert - Assert.IsFalse(result); - } - - // TODO: Prepared for supporting 1/0 for sampled header - //[TestMethod] - //public void ShouldBeSampled_PathNotInBlacktListSampleRate1_SampledString0() - //{ - // // Arrange - // var fixture = new Fixture(); - - // _sut.SampleRate = 1; - - // var context = MockRepository.GenerateStub(); - // var request = MockRepository.GenerateStub(); - // request.Path = new PathString("/samplePath"); - // context.Stub(x => x.Request).Return(request); - - // // Act - // var result = _sut.ShouldBeSampled(context, "0"); - - // // Assert - // Assert.IsFalse(result); - //} - - [TestMethod] - public void ShouldBeSampled_PathNotInBlacktListSampleRate0() + /// + /// TODO: Use XUnit to do easier unit test for inline data + /// + private class ShouldBeSampledCondition { - // Arrange - var fixture = new Fixture(); - - _sut.SampleRate = 0; - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString("/samplePath"); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, null); - - // Assert - Assert.IsFalse(result); + public string SampledFlag { get; set; } + public string RequestPath { get; set; } + public double SampleRate { get; set; } + public List ExcludedPathList { get; set; } + public bool ExpectedOutcome { get; set; } + + public ShouldBeSampledCondition( + string sampledFlag, + string requestPath, + double sampleRate, + List excludedPathList, + bool expectedOutcome) + { + SampledFlag = sampledFlag; + RequestPath = requestPath; + SampleRate = sampleRate; + ExcludedPathList = excludedPathList; + ExpectedOutcome = expectedOutcome; + } } [TestMethod] - public void ShouldBeSampled_PathInBlacktList() + public void ShouldBeSampled() { // Arrange - var fixture = new Fixture(); - - _sut.SampleRate = 0; - var path = "/samplePath"; - _sut.NotToBeDisplayedDomainList.Add(path); - - var context = MockRepository.GenerateStub(); - var request = MockRepository.GenerateStub(); - request.Path = new PathString(path); - context.Stub(x => x.Request).Return(request); - - // Act - var result = _sut.ShouldBeSampled(context, null); - - // Assert - Assert.IsFalse(result); + List testScenarios = new List() { + // sampledFlag has a valid bool string value + { new ShouldBeSampledCondition("0", null, 0, new List(), false) }, + { new ShouldBeSampledCondition("1", null, 0, new List(), true) }, + { new ShouldBeSampledCondition("false", null, 0, new List(), false) }, + { new ShouldBeSampledCondition("true", null, 0, new List(), true) }, + { new ShouldBeSampledCondition("FALSE", null, 0, new List(), false) }, + { new ShouldBeSampledCondition("TRUE", null, 0, new List(), true) }, + { new ShouldBeSampledCondition("FalSe", null, 0, new List(), false) }, + { new ShouldBeSampledCondition("TrUe", null, 0, new List(), true) }, + // sampledFlag has an invalid bool string value and requestPath is IsInDontSampleList + { new ShouldBeSampledCondition(null, "/x", 0, new List { "/x" }, false) }, + { new ShouldBeSampledCondition("", "/x", 0, new List { "/x" }, false) }, + { new ShouldBeSampledCondition("invalidValue", "/x", 0, new List() { "/x" }, false) }, + // sampledFlag has an invalid bool string value, requestPath not in IsInDontSampleList, and sample rate is 0 + { new ShouldBeSampledCondition(null, null, 0, new List(), false) }, + { new ShouldBeSampledCondition(null, "/x", 0, new List(), false) }, + // sampledFlag has an invalid bool string value, requestPath not in IsInDontSampleList, and sample rate is 1 + { new ShouldBeSampledCondition(null, null, 1, new List(), true) }, + }; + + foreach (var testScenario in testScenarios) + { + var fixture = new Fixture(); + _sut = new ZipkinConfig + { + ExcludedPathList = testScenario.ExcludedPathList, + SampleRate = testScenario.SampleRate + }; + + // Act + var result = _sut.ShouldBeSampled(testScenario.SampledFlag, testScenario.RequestPath); + + // Assert + Assert.AreEqual( + testScenario.ExpectedOutcome, + result, + "Scenario: " + + $"SampledFlag({testScenario.SampledFlag ?? "null"}), " + + $"RequestPath({testScenario.RequestPath ?? "null"}), " + + $"SampleRate({testScenario.SampleRate}), " + + $"ExcludedPathList({string.Join(",", testScenario.ExcludedPathList)}),"); + } } } } \ No newline at end of file