Skip to content

Commit

Permalink
Remove log4net dependency and make logging injectable
Browse files Browse the repository at this point in the history
  • Loading branch information
@robhruska
robhruska committed Apr 3, 2017
1 parent 0b0bbdc commit 5dd8295
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 71 deletions.
26 changes: 13 additions & 13 deletions Hudl.Mjolnir.Tests/Breaker/FailurePercentageCircuitBreakerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ public void MarkSuccess_ImmediatelyAfterTrippingButStartedBeforeTripped_DoesntIm
mockConfig.Setup(m => m.GetTrippedDurationMillis(It.IsAny<GroupKey>())).Returns(30000);
mockConfig.Setup(m => m.GetForceTripped(It.IsAny<GroupKey>())).Returns(false);
mockConfig.Setup(m => m.GetForceFixed(It.IsAny<GroupKey>())).Returns(false);

// 5 ops, 1% failure required to break.
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand Down Expand Up @@ -150,9 +150,9 @@ public void MarkSuccess_ForLongRunningSingleTest_FixesBreaker()
mockConfig.Setup(m => m.GetTrippedDurationMillis(It.IsAny<GroupKey>())).Returns(trippedDurationMillis);
mockConfig.Setup(m => m.GetForceTripped(It.IsAny<GroupKey>())).Returns(false);
mockConfig.Setup(m => m.GetForceFixed(It.IsAny<GroupKey>())).Returns(false);

// 1 ops, 1% failure required to break.
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand Down Expand Up @@ -193,8 +193,8 @@ public void IsAllowing_WhenPropertiesForceTripped_Rejects()
mockConfig.Setup(m => m.GetTrippedDurationMillis(It.IsAny<GroupKey>())).Returns(30000);
mockConfig.Setup(m => m.GetForceTripped(It.IsAny<GroupKey>())).Returns(true); // The config we're testing here.
mockConfig.Setup(m => m.GetForceFixed(It.IsAny<GroupKey>())).Returns(false);

var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand All @@ -221,8 +221,8 @@ public void IsAllowing_WhenPropertiesForceFixed_Allows()
mockConfig.Setup(m => m.GetTrippedDurationMillis(It.IsAny<GroupKey>())).Returns(30000);
mockConfig.Setup(m => m.GetForceTripped(It.IsAny<GroupKey>())).Returns(false);
mockConfig.Setup(m => m.GetForceFixed(It.IsAny<GroupKey>())).Returns(true); // The config we're testing here.

var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand All @@ -249,8 +249,8 @@ public void IsAllowing_WhenBothForcePropertiesSet_Rejects()
mockConfig.Setup(m => m.GetTrippedDurationMillis(It.IsAny<GroupKey>())).Returns(30000);
mockConfig.Setup(m => m.GetForceTripped(It.IsAny<GroupKey>())).Returns(true); // The config we're testing here.
mockConfig.Setup(m => m.GetForceFixed(It.IsAny<GroupKey>())).Returns(true); // The config we're testing here.

var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand Down Expand Up @@ -286,7 +286,7 @@ public void IsAllowing_WhenPropertiesForceFixedButBreakerWouldNormallyTrip_Silen
.Returns(true)
.Returns(false);

var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object);
var breaker = new FailurePercentageCircuitBreaker(AnyKey, manualClock, mockMetrics.Object, mockEvents.Object, mockConfig.Object, new DefaultMjolnirLogFactory());


// Act / Assert
Expand Down Expand Up @@ -583,7 +583,7 @@ public void Run()
{
var mockMetrics = CreateMockMetricsWithSnapshot(_metricsTotal, _metricsPercent);
var config = CreateMockBreakerConfig(_breakerTotal, _breakerPercent, 30000);
var breaker = new FailurePercentageCircuitBreaker(GroupKey.Named("Test"), mockMetrics.Object, new IgnoringMetricEvents(), config.Object);
var breaker = new FailurePercentageCircuitBreaker(GroupKey.Named("Test"), mockMetrics.Object, new IgnoringMetricEvents(), config.Object, new DefaultMjolnirLogFactory());

Assert.NotEqual(_shouldTrip, breaker.IsAllowing());
}
Expand Down Expand Up @@ -657,7 +657,7 @@ public BreakerBuilder WithMetricEvents(IMetricEvents metricEvents)
public FailurePercentageCircuitBreaker Create()
{
var config = FailurePercentageCircuitBreakerTests.CreateMockBreakerConfig(_minimumOperations, _failurePercent, _waitMillis);
return new FailurePercentageCircuitBreaker(GroupKey.Named(_key), _clock, _mockMetrics.Object, _metricEvents, config.Object);
return new FailurePercentageCircuitBreaker(GroupKey.Named(_key), _clock, _mockMetrics.Object, _metricEvents, config.Object, new DefaultMjolnirLogFactory());
}
}
}
5 changes: 3 additions & 2 deletions Hudl.Mjolnir.Tests/Helper/AlwaysSuccessfulCircuitBreaker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Hudl.Mjolnir.External;
using Hudl.Mjolnir.Key;
using Hudl.Mjolnir.Metrics;
using Moq;

namespace Hudl.Mjolnir.Tests.Helper
{
Expand All @@ -26,8 +27,8 @@ public ICommandMetrics Metrics
{
var config = new DefaultValueConfig();
var metricsConfig = new StandardCommandMetricsConfig(config);

return new StandardCommandMetrics(GroupKey.Named("Test"), metricsConfig);
return new StandardCommandMetrics(GroupKey.Named("Test"), metricsConfig, new DefaultMjolnirLogFactory());
}
}

Expand Down
4 changes: 0 additions & 4 deletions Hudl.Mjolnir.Tests/Hudl.Mjolnir.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@
<Reference Include="Castle.Core">
<HintPath>..\packages\Castle.Core.2.5.1\lib\NET35\Castle.Core.dll</HintPath>
</Reference>
<Reference Include="log4net, Version=1.2.11.0, Culture=neutral, PublicKeyToken=669e0ddf0bb1aa2a, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\packages\log4net.2.0.0\lib\net40-full\log4net.dll</HintPath>
</Reference>
<Reference Include="Moq">
<HintPath>..\packages\Moq.4.1.1309.1617\lib\net40\Moq.dll</HintPath>
</Reference>
Expand Down
8 changes: 4 additions & 4 deletions Hudl.Mjolnir.Tests/Metrics/ResettingNumbersBucketTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public void Increment_AfterPeriodExceeded_ResetsBeforeIncrementing()

var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(It.IsAny<GroupKey>())).Returns(periodMillis);

var clock = new ManualTestClock();
var bucket = new ResettingNumbersBucket(GroupKey.Named("any"), clock, mockConfig.Object);
var bucket = new ResettingNumbersBucket(GroupKey.Named("any"), clock, mockConfig.Object, new DefaultMjolnirLogFactory());

bucket.Increment(CounterMetric.CommandSuccess);

Expand All @@ -60,9 +60,9 @@ private ResettingNumbersBucket CreateBucket()
{
var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(It.IsAny<GroupKey>())).Returns(10000);

var clock = new ManualTestClock();
return new ResettingNumbersBucket(GroupKey.Named("any"), clock, mockConfig.Object);
return new ResettingNumbersBucket(GroupKey.Named("any"), clock, mockConfig.Object, new DefaultMjolnirLogFactory());
}
}
}
16 changes: 8 additions & 8 deletions Hudl.Mjolnir.Tests/Metrics/StandardCommandMetricsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public void MarkCommandSuccess_BeforeFirstSnapshot_GetsIncludedInSnapshot()
var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(key)).Returns(30000);
mockConfig.Setup(m => m.GetSnapshotTtlMillis(key)).Returns(1000);

var metrics = new StandardCommandMetrics(key, mockConfig.Object);
var metrics = new StandardCommandMetrics(key, mockConfig.Object, new DefaultMjolnirLogFactory());
metrics.MarkCommandSuccess();

var snapshot = metrics.GetSnapshot();
Expand All @@ -35,8 +35,8 @@ public void MarkCommandFailure_BeforeFirstSnapshot_GetsIncludedInSnapshot()
var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(key)).Returns(30000);
mockConfig.Setup(m => m.GetSnapshotTtlMillis(key)).Returns(1000);

var metrics = new StandardCommandMetrics(key, mockConfig.Object);
var metrics = new StandardCommandMetrics(key, mockConfig.Object, new DefaultMjolnirLogFactory());
metrics.MarkCommandFailure();

var snapshot = metrics.GetSnapshot();
Expand Down Expand Up @@ -76,11 +76,11 @@ public void GetSnapshot_WithinCachePeriod_ReturnsPreviousSnapshot()
var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(key)).Returns(10000);
mockConfig.Setup(m => m.GetSnapshotTtlMillis(key)).Returns(1000);

// Within the metrics, the last snapshot timestamp will probably be zero.
// Let's start our clock with something far away from zero.
clock.AddMilliseconds(new UtcSystemClock().GetMillisecondTimestamp());
var metrics = new StandardCommandMetrics(key, mockConfig.Object, clock);
var metrics = new StandardCommandMetrics(key, mockConfig.Object, clock, new DefaultMjolnirLogFactory());

metrics.MarkCommandSuccess();
metrics.GetSnapshot(); // Take the first snapshot to cache it.
Expand All @@ -98,8 +98,8 @@ private MetricsSnapshot SnapshotFor(int success, int failure)
var mockConfig = new Mock<IStandardCommandMetricsConfig>(MockBehavior.Strict);
mockConfig.Setup(m => m.GetWindowMillis(key)).Returns(10000);
mockConfig.Setup(m => m.GetSnapshotTtlMillis(key)).Returns(0); // Don't cache snapshots.

var metrics = new StandardCommandMetrics(key, mockConfig.Object);
var metrics = new StandardCommandMetrics(key, mockConfig.Object, new DefaultMjolnirLogFactory());

for (var i = 0; i < success; i++)
{
Expand Down
1 change: 0 additions & 1 deletion Hudl.Mjolnir.Tests/packages.config
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Castle.Core" version="2.5.1" targetFramework="net45" />
<package id="log4net" version="2.0.0" targetFramework="net45" />
<package id="Moq" version="4.1.1309.1617" targetFramework="net45" />
<package id="xunit" version="2.0.0" targetFramework="net45" />
<package id="xunit.abstractions" version="2.0.0" targetFramework="net45" />
Expand Down
29 changes: 18 additions & 11 deletions Hudl.Mjolnir/Breaker/FailurePercentageCircuitBreaker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Hudl.Mjolnir.Key;
using Hudl.Mjolnir.Metrics;
using Hudl.Mjolnir.Util;
using log4net;
using Hudl.Mjolnir.Clock;

namespace Hudl.Mjolnir.Breaker
Expand All @@ -18,10 +17,6 @@ namespace Hudl.Mjolnir.Breaker
/// </summary>
internal class FailurePercentageCircuitBreaker : ICircuitBreaker
{
//internal readonly FailurePercentageCircuitBreakerProperties Properties;

private static readonly ILog Log = LogManager.GetLogger(typeof (FailurePercentageCircuitBreaker));

private readonly object _stateChangeLock = new { };
private readonly object _singleTestLock = new { };

Expand All @@ -31,6 +26,7 @@ internal class FailurePercentageCircuitBreaker : ICircuitBreaker
private readonly GroupKey _key;
private readonly IMetricEvents _metricEvents;
private readonly IFailurePercentageCircuitBreakerConfig _config;
private readonly IMjolnirLog _log;

// ReSharper disable NotAccessedField.Local
// Don't let these get garbage collected.
Expand All @@ -40,10 +36,10 @@ internal class FailurePercentageCircuitBreaker : ICircuitBreaker
private volatile State _state;
private long _lastTrippedTimestamp;

internal FailurePercentageCircuitBreaker(GroupKey key, ICommandMetrics metrics, IMetricEvents metricEvents, IFailurePercentageCircuitBreakerConfig config)
: this(key, new UtcSystemClock(), metrics, metricEvents, config) {}
internal FailurePercentageCircuitBreaker(GroupKey key, ICommandMetrics metrics, IMetricEvents metricEvents, IFailurePercentageCircuitBreakerConfig config, IMjolnirLogFactory logFactory)
: this(key, new UtcSystemClock(), metrics, metricEvents, config, logFactory) {}

internal FailurePercentageCircuitBreaker(GroupKey key, IClock clock, ICommandMetrics metrics, IMetricEvents metricEvents, IFailurePercentageCircuitBreakerConfig config)
internal FailurePercentageCircuitBreaker(GroupKey key, IClock clock, ICommandMetrics metrics, IMetricEvents metricEvents, IFailurePercentageCircuitBreakerConfig config, IMjolnirLogFactory logFactory)
{
_key = key;
_clock = clock;
Expand All @@ -59,8 +55,19 @@ internal FailurePercentageCircuitBreaker(GroupKey key, IClock clock, ICommandMet
throw new ArgumentNullException(nameof(metricEvents));
}

if (logFactory == null)
{
throw new ArgumentNullException(nameof(logFactory));
}

_config = config;
_metricEvents = metricEvents;
_log = logFactory.CreateLog(typeof(FailurePercentageCircuitBreaker));

if (_log == null)
{
throw new InvalidOperationException($"{nameof(IMjolnirLogFactory)} implementation returned null from {nameof(IMjolnirLogFactory.CreateLog)} for type {typeof(FailurePercentageCircuitBreaker)}, please make sure the implementation returns a non-null log for all calls to {nameof(IMjolnirLogFactory.CreateLog)}");
}

_state = State.Fixed; // Start off assuming everything's fixed.
_lastTrippedTimestamp = 0; // 0 is fine since it'll be far less than the first compared value.
Expand Down Expand Up @@ -100,7 +107,7 @@ public void MarkSuccess(long elapsedMillis)
return;
}

Log.Info($"Fixed Breaker={_key}");
_log.Info($"Fixed Breaker={_key}");

_state = State.Fixed;
_metrics.Reset();
Expand Down Expand Up @@ -150,7 +157,7 @@ private bool AllowSingleTest()
if (_state == State.Tripped && IsPastWaitDuration())
{
_lastTrippedTimestamp = _clock.GetMillisecondTimestamp();
Log.Info($"Allowing single test operation Breaker={_key}");
_log.Info($"Allowing single test operation Breaker={_key}");
return true;
}

Expand Down Expand Up @@ -205,7 +212,7 @@ private bool CheckAndSetTripped()
_lastTrippedTimestamp = _clock.GetMillisecondTimestamp();

_metricEvents.BreakerTripped(Name);
Log.Error($"Tripped Breaker={_key} Operations={snapshot.Total} ErrorPercentage={snapshot.ErrorPercentage} Wait={_config.GetTrippedDurationMillis(_key)}");
_log.Error($"Tripped Breaker={_key} Operations={snapshot.Total} ErrorPercentage={snapshot.ErrorPercentage} Wait={_config.GetTrippedDurationMillis(_key)}");

return true;
}
Expand Down

0 comments on commit 5dd8295

Please sign in to comment.