From ecf4a2c95ac73cc62b38155433c873791325b95a Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 16:00:54 +0100 Subject: [PATCH 1/8] Remove catching of all exceptions NLog handles exceptions from targets internally, with it's default behaviour set to hide all exceptions and not rethrow them. Exceptions will only be propagated if the "throwExceptions" property is set to true on the root "nlog" node. Having a try-catch in the target makes it more difficult to diagnose configuration issues. See: https://github.com/NLog/NLog/wiki/Logging-troubleshooting --- NLog.Targets.Sentry/SentryTarget.cs | 39 ++++++++++++----------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index 3ebbc2b..0a3b065 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -72,34 +72,27 @@ internal SentryTarget(IRavenClient ravenClient) : this() /// Logging event to be written out. protected override void Write(LogEventInfo logEvent) { - try - { - var tags = SendLogEventInfoPropertiesAsTags - ? logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()) - : null; + var tags = SendLogEventInfoPropertiesAsTags + ? logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()) + : null; - var extras = SendLogEventInfoPropertiesAsTags - ? null - : logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()); + var extras = SendLogEventInfoPropertiesAsTags + ? null + : logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()); - client.Value.Logger = logEvent.LoggerName; + client.Value.Logger = logEvent.LoggerName; - // If the log event did not contain an exception and we're not ignoring - // those kinds of events then we'll send a "Message" to Sentry - if (logEvent.Exception == null && !IgnoreEventsWithNoException) - { - var sentryMessage = new SentryMessage(Layout.Render(logEvent)); - client.Value.CaptureMessage(sentryMessage, LoggingLevelMap[logEvent.Level], extra: extras, tags: tags); - } - else if (logEvent.Exception != null) - { - var sentryMessage = new SentryMessage(logEvent.FormattedMessage); - client.Value.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); - } + // If the log event did not contain an exception and we're not ignoring + // those kinds of events then we'll send a "Message" to Sentry + if (logEvent.Exception == null && !IgnoreEventsWithNoException) + { + var sentryMessage = new SentryMessage(Layout.Render(logEvent)); + client.Value.CaptureMessage(sentryMessage, LoggingLevelMap[logEvent.Level], extra: extras, tags: tags); } - catch (Exception e) + else if (logEvent.Exception != null) { - InternalLogger.Error("Unable to send Sentry request: {0}", e.Message); + var sentryMessage = new SentryMessage(logEvent.FormattedMessage); + client.Value.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); } } } From 418a48848be7b3adedbb526bda2e1f3f93c03343 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 17:25:22 +0100 Subject: [PATCH 2/8] Make conversion to string dictionary more robust Avoid possible edge cases when converting from Dictionary to Dictionary. All cases fairly unlikely, but wouldn't want to cause messages to be missed so best to be defensive here. Possible issues: 1. Values of a dictionary can be null. Also filtering out blank string values. 2. Two distinct key objects can be converted to the same string representation. When this happens, concatenate all the matching values together as a workaround. 3. ToString could return null. Therefore, remove all properties will a null key or value string representation. --- NLog.Targets.Sentry/SentryTarget.cs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index 0a3b065..fbaf2f0 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -72,13 +72,21 @@ internal SentryTarget(IRavenClient ravenClient) : this() /// Logging event to be written out. protected override void Write(LogEventInfo logEvent) { + var propertiesAsStrings = ( + from property in logEvent.Properties + let stringKey = ToStringOrNull(property.Key) + let stringValue = ToStringOrNull(property.Value) + where stringKey != null && stringValue != null + group stringValue by stringKey) + .ToDictionary(x => x.Key, x => string.Join(",", x)); + var tags = SendLogEventInfoPropertiesAsTags - ? logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()) + ? propertiesAsStrings : null; var extras = SendLogEventInfoPropertiesAsTags ? null - : logEvent.Properties.ToDictionary(x => x.Key.ToString(), x => x.Value.ToString()); + : propertiesAsStrings; client.Value.Logger = logEvent.LoggerName; @@ -95,6 +103,16 @@ protected override void Write(LogEventInfo logEvent) client.Value.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); } } + + private static string ToStringOrNull(object obj) + { + if (obj == null) + { + return null; + } + + return obj.ToString(); + } } } From ce4fec093ab8527732d2d7069f7cb16e55b61b1b Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 17:40:06 +0100 Subject: [PATCH 3/8] Make the client thread-safe The RavenClient is not marked as being thread-safe, but a single instance is used within the target. In addition, we mutate the Logger property to match the logger of the current event, meaning there could be a race condition between two threads changing the Logger and sending their message, causing messages to come from the wrong logger. If this is a performance bottleneck, then perhaps a client could be created per logger (assuming that the rest of the client is thread-safe). Parsing the DSN can fail with exceptions, therefore, this is preferable to fail at the point of creating the client, rather than which trying to de-serialize the config. A specific use case for this delayed failure is when using NLog with autoReload enabled, it will now only fail when trying to write messages, but can then still be re-configured as long as the lazy has not been successfully initialized. --- .../SentryTargetTests.cs | 19 ++++++++++-- NLog.Targets.Sentry/SentryTarget.cs | 31 +++++++++---------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs index f6453f9..ce0d36f 100644 --- a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs +++ b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs @@ -40,7 +40,20 @@ public void TestPublicConstructor() [Test] public void TestBadDsn() { - Assert.Throws(() => new SentryTarget(null) { Dsn = "http://localhost" }); + var sentryTarget = new SentryTarget { Dsn = "http://localhost" }; + var configuration = new LoggingConfiguration(); + configuration.AddTarget("NLogSentry", sentryTarget); + configuration.LoggingRules.Add(new LoggingRule("*", LogLevel.Trace, sentryTarget)); + LogManager.Configuration = configuration; + try + { + LogManager.GetCurrentClassLogger().Info("Test"); + Assert.Fail("Expected exception not raised"); + } + catch (NLogRuntimeException ex) + { + Assert.IsInstanceOf(ex.InnerException); + } } [Test] @@ -62,7 +75,7 @@ public void TestLoggingToSentry() .Returns("Done"); // Setup NLog - var sentryTarget = new SentryTarget(sentryClient.Object) + var sentryTarget = new SentryTarget(() => sentryClient.Object) { Dsn = "http://25e27038b1df4930b93c96c170d95527:d87ac60bb07b4be8908845b23e914dae@test/4", }; @@ -108,7 +121,7 @@ public void TestLoggingToSentry_SendLogEventInfoPropertiesAsTags() .Returns("Done"); // Setup NLog - var sentryTarget = new SentryTarget(sentryClient.Object) + var sentryTarget = new SentryTarget(() => sentryClient.Object) { Dsn = "http://25e27038b1df4930b93c96c170d95527:d87ac60bb07b4be8908845b23e914dae@test/4", SendLogEventInfoPropertiesAsTags = true, diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index fbaf2f0..5c5d5f6 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using NLog.Common; using NLog.Config; using SharpRaven; using SharpRaven.Data; @@ -13,8 +12,7 @@ namespace NLog.Targets [Target("Sentry")] public class SentryTarget : TargetWithLayout { - private Dsn dsn; - private readonly Lazy client; + private readonly Func ravenClientFactory; /// /// Map of NLog log levels to Raven/Sentry log levels @@ -33,11 +31,7 @@ public class SentryTarget : TargetWithLayout /// The DSN for the Sentry host /// [RequiredParameter] - public string Dsn - { - get { return dsn == null ? null : dsn.ToString(); } - set { dsn = new Dsn(value); } - } + public string Dsn { get; set; } /// /// Determines whether events with no exceptions will be send to Sentry or not @@ -54,16 +48,15 @@ public string Dsn /// public SentryTarget() { - client = new Lazy(() => new RavenClient(dsn)); } /// /// Internal constructor, used for unit-testing /// - /// A - internal SentryTarget(IRavenClient ravenClient) : this() + /// Constructor of a + internal SentryTarget(Func createRavenClient) { - client = new Lazy(() => ravenClient); + this.ravenClientFactory = createRavenClient; } /// @@ -88,22 +81,28 @@ from property in logEvent.Properties ? null : propertiesAsStrings; - client.Value.Logger = logEvent.LoggerName; - + var client = CreateClient(logEvent); // If the log event did not contain an exception and we're not ignoring // those kinds of events then we'll send a "Message" to Sentry if (logEvent.Exception == null && !IgnoreEventsWithNoException) { var sentryMessage = new SentryMessage(Layout.Render(logEvent)); - client.Value.CaptureMessage(sentryMessage, LoggingLevelMap[logEvent.Level], extra: extras, tags: tags); + client.CaptureMessage(sentryMessage, LoggingLevelMap[logEvent.Level], extra: extras, tags: tags); } else if (logEvent.Exception != null) { var sentryMessage = new SentryMessage(logEvent.FormattedMessage); - client.Value.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); + client.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); } } + private IRavenClient CreateClient(LogEventInfo logEvent) + { + var client = ravenClientFactory != null ? ravenClientFactory() : new RavenClient(new Dsn(Dsn)); + client.Logger = logEvent.LoggerName; + return client; + } + private static string ToStringOrNull(object obj) { if (obj == null) From a32e77c78e78f12f957c33ac0f620a6f4633b1b3 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 22:33:16 +0100 Subject: [PATCH 4/8] Make mapping of levels safer Add coverage for the "Off" level (though should never get through this far in theory). Add testing for all expected levels, including coverage of if the ordinals were to be changed. --- .../SentryTargetTests.cs | 15 ++++++ NLog.Targets.Sentry/SentryTarget.cs | 49 +++++++++++++------ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs index ce0d36f..9680263 100644 --- a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs +++ b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs @@ -150,5 +150,20 @@ public void TestLoggingToSentry_SendLogEventInfoPropertiesAsTags() Assert.IsTrue(lTags != null); Assert.IsTrue(lErrorLevel == ErrorLevel.Error); } + + [TestCase("Trace", 0, ErrorLevel.Debug)] + [TestCase("Debug", 1, ErrorLevel.Debug)] + [TestCase("Info", 2, ErrorLevel.Info)] + [TestCase("Warn", 3, ErrorLevel.Warning)] + [TestCase("Error", 4, ErrorLevel.Error)] + [TestCase("Fatal", 5, ErrorLevel.Fatal)] + [TestCase("Off", 6, null)] + public void TestLevelMappings(string name, int ordinal, ErrorLevel? expectedErrorLevel) + { + var level = LogLevel.FromString(name); + Assert.AreEqual(level, LogLevel.FromOrdinal(ordinal)); + var errorLevel = SentryTarget.TryGetErrorLevel(level); + Assert.AreEqual(expectedErrorLevel, errorLevel); + } } } diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index 5c5d5f6..f526e05 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -14,19 +14,6 @@ public class SentryTarget : TargetWithLayout { private readonly Func ravenClientFactory; - /// - /// Map of NLog log levels to Raven/Sentry log levels - /// - protected static readonly IDictionary LoggingLevelMap = new Dictionary - { - {LogLevel.Debug, ErrorLevel.Debug}, - {LogLevel.Error, ErrorLevel.Error}, - {LogLevel.Fatal, ErrorLevel.Fatal}, - {LogLevel.Info, ErrorLevel.Info}, - {LogLevel.Trace, ErrorLevel.Debug}, - {LogLevel.Warn, ErrorLevel.Warning}, - }; - /// /// The DSN for the Sentry host /// @@ -65,6 +52,10 @@ internal SentryTarget(Func createRavenClient) /// Logging event to be written out. protected override void Write(LogEventInfo logEvent) { + var level = TryGetErrorLevel(logEvent.Level); + // Level is set to "Off", so exit. + if (level == null) return; + var propertiesAsStrings = ( from property in logEvent.Properties let stringKey = ToStringOrNull(property.Key) @@ -87,12 +78,12 @@ from property in logEvent.Properties if (logEvent.Exception == null && !IgnoreEventsWithNoException) { var sentryMessage = new SentryMessage(Layout.Render(logEvent)); - client.CaptureMessage(sentryMessage, LoggingLevelMap[logEvent.Level], extra: extras, tags: tags); + client.CaptureMessage(sentryMessage, level.Value, extra: extras, tags: tags); } else if (logEvent.Exception != null) { var sentryMessage = new SentryMessage(logEvent.FormattedMessage); - client.CaptureException(logEvent.Exception, extra: extras, level: LoggingLevelMap[logEvent.Level], message: sentryMessage, tags: tags); + client.CaptureException(logEvent.Exception, extra: extras, level: level.Value, message: sentryMessage, tags: tags); } } @@ -112,6 +103,34 @@ private static string ToStringOrNull(object obj) return obj.ToString(); } + + internal static ErrorLevel? TryGetErrorLevel(LogLevel level) + { + if (level == null) + { + return null; + } + + // For ordinals, see https://github.com/NLog/NLog/blob/master/src/NLog/LogLevel.cs + switch (level.Ordinal) + { + case 0: // Trace + case 1: // Debug + return ErrorLevel.Debug; + case 2: + return ErrorLevel.Info; + case 3: + return ErrorLevel.Warning; + case 4: + return ErrorLevel.Error; + case 5: + return ErrorLevel.Fatal; + case 6: // Off + return null; + default: + throw new Exception(string.Format("Unable to map NLog LogLevel of {0} to a Sentry ErrorLevel", level)); + } + } } } From 30809513a9bc8720ffeebab7241bb0f6d69ffabf Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 18:24:24 +0100 Subject: [PATCH 5/8] Make IgnoreEventsWithNoException obsolete - This can be done using the target filter condition feature in NLog. - Separate check for exception from condition of skipping logging. --- NLog.Targets.Sentry/SentryTarget.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index f526e05..160e5c0 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -23,6 +23,7 @@ public class SentryTarget : TargetWithLayout /// /// Determines whether events with no exceptions will be send to Sentry or not /// + [Obsolete("Use target filter conditions instead. See: https://github.com/NLog/NLog/wiki/Conditions")] public bool IgnoreEventsWithNoException { get; set; } /// @@ -75,12 +76,15 @@ from property in logEvent.Properties var client = CreateClient(logEvent); // If the log event did not contain an exception and we're not ignoring // those kinds of events then we'll send a "Message" to Sentry - if (logEvent.Exception == null && !IgnoreEventsWithNoException) + if (logEvent.Exception == null) { - var sentryMessage = new SentryMessage(Layout.Render(logEvent)); - client.CaptureMessage(sentryMessage, level.Value, extra: extras, tags: tags); + if (!IgnoreEventsWithNoException) + { + var sentryMessage = new SentryMessage(Layout.Render(logEvent)); + client.CaptureMessage(sentryMessage, level.Value, extra: extras, tags: tags); + } } - else if (logEvent.Exception != null) + else { var sentryMessage = new SentryMessage(logEvent.FormattedMessage); client.CaptureException(logEvent.Exception, extra: extras, level: level.Value, message: sentryMessage, tags: tags); From acc1ebaeba8e71f2c7d6f349b5966236147c54a5 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 18:25:43 +0100 Subject: [PATCH 6/8] Always use the log message as the sentry message Treat all NLog messages the same and avoid differences in behaviour depending on if we have an exception. This kind of target has no need for a custom layout. --- NLog.Targets.Sentry/SentryTarget.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index 160e5c0..c49c8d8 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -10,7 +10,7 @@ namespace NLog.Targets // ReSharper restore CheckNamespace { [Target("Sentry")] - public class SentryTarget : TargetWithLayout + public class SentryTarget : Target { private readonly Func ravenClientFactory; @@ -80,7 +80,7 @@ from property in logEvent.Properties { if (!IgnoreEventsWithNoException) { - var sentryMessage = new SentryMessage(Layout.Render(logEvent)); + var sentryMessage = new SentryMessage(logEvent.FormattedMessage); client.CaptureMessage(sentryMessage, level.Value, extra: extras, tags: tags); } } From 58b7fd9a48f1c58af72320bda37c99ec437f4074 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 18:29:02 +0100 Subject: [PATCH 7/8] Upgrade SharpRaven Due to the old API being deprecated, use the new API, which requires the sentry event to be constructed in a different way. --- .../NLog.Targets.Sentry.UnitTests.csproj | 5 +- .../SentryTargetTests.cs | 36 ++++----- NLog.Targets.Sentry.UnitTests/packages.config | 2 +- .../NLog.Targets.Sentry.csproj | 5 +- NLog.Targets.Sentry/Packages.config | 2 +- NLog.Targets.Sentry/SentryTarget.cs | 81 ++++++++++++------- 6 files changed, 76 insertions(+), 55 deletions(-) diff --git a/NLog.Targets.Sentry.UnitTests/NLog.Targets.Sentry.UnitTests.csproj b/NLog.Targets.Sentry.UnitTests/NLog.Targets.Sentry.UnitTests.csproj index 1059b63..c780a9c 100644 --- a/NLog.Targets.Sentry.UnitTests/NLog.Targets.Sentry.UnitTests.csproj +++ b/NLog.Targets.Sentry.UnitTests/NLog.Targets.Sentry.UnitTests.csproj @@ -42,8 +42,9 @@ ..\packages\NUnit.2.6.4\lib\nunit.framework.dll - - ..\packages\SharpRaven.1.4.3\lib\net45\SharpRaven.dll + + ..\packages\SharpRaven.2.1.0\lib\net45\SharpRaven.dll + True diff --git a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs index 9680263..7c38a9e 100644 --- a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs +++ b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs @@ -60,17 +60,13 @@ public void TestBadDsn() public void TestLoggingToSentry() { var sentryClient = new Mock(); - ErrorLevel lErrorLevel = ErrorLevel.Debug; - IDictionary lTags = null; - Exception lException = null; + SentryEvent lastSentryEvent = null; sentryClient - .Setup(x => x.CaptureException(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny())) - .Callback((Exception exception, SentryMessage msg, ErrorLevel lvl, IDictionary d, object extra) => + .Setup(x => x.Capture(It.IsAny())) + .Callback((SentryEvent sentryEvent) => { - lException = exception; - lErrorLevel = lvl; - lTags = d; + lastSentryEvent = sentryEvent; }) .Returns("Done"); @@ -94,9 +90,9 @@ public void TestLoggingToSentry() logger.ErrorException("Error Message", e); } - Assert.IsTrue(lException.Message == "Oh No!"); - Assert.IsTrue(lTags == null); - Assert.IsTrue(lErrorLevel == ErrorLevel.Error); + Assert.IsTrue(lastSentryEvent.Message == "Oh No!"); + Assert.IsEmpty(lastSentryEvent.Tags); + Assert.IsTrue(lastSentryEvent.Level == ErrorLevel.Error); } @@ -106,17 +102,13 @@ public void TestLoggingToSentry() public void TestLoggingToSentry_SendLogEventInfoPropertiesAsTags() { var sentryClient = new Mock(); - ErrorLevel lErrorLevel = ErrorLevel.Debug; - IDictionary lTags = null; - Exception lException = null; + SentryEvent lastSentryEvent = null; sentryClient - .Setup(x => x.CaptureException(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny())) - .Callback((Exception exception, SentryMessage msg, ErrorLevel lvl, IDictionary d, object extra) => + .Setup(x => x.Capture(It.IsAny())) + .Callback((SentryEvent sentryEvent) => { - lException = exception; - lErrorLevel = lvl; - lTags = d; + lastSentryEvent = sentryEvent; }) .Returns("Done"); @@ -146,9 +138,9 @@ public void TestLoggingToSentry_SendLogEventInfoPropertiesAsTags() logger.Log(logEventInfo); } - Assert.IsTrue(lException.Message == "Oh No!"); - Assert.IsTrue(lTags != null); - Assert.IsTrue(lErrorLevel == ErrorLevel.Error); + Assert.IsTrue(lastSentryEvent.Message == "Oh No!"); + CollectionAssert.AreEqual(new Dictionary { { "tag1", tag1Value } }, lastSentryEvent.Tags); + Assert.IsTrue(lastSentryEvent.Level == ErrorLevel.Error); } [TestCase("Trace", 0, ErrorLevel.Debug)] diff --git a/NLog.Targets.Sentry.UnitTests/packages.config b/NLog.Targets.Sentry.UnitTests/packages.config index 94a48ab..93c7dd8 100644 --- a/NLog.Targets.Sentry.UnitTests/packages.config +++ b/NLog.Targets.Sentry.UnitTests/packages.config @@ -4,5 +4,5 @@ - + \ No newline at end of file diff --git a/NLog.Targets.Sentry/NLog.Targets.Sentry.csproj b/NLog.Targets.Sentry/NLog.Targets.Sentry.csproj index db60fb9..22a3647 100755 --- a/NLog.Targets.Sentry/NLog.Targets.Sentry.csproj +++ b/NLog.Targets.Sentry/NLog.Targets.Sentry.csproj @@ -36,8 +36,9 @@ ..\packages\NLog.3.2.0.0\lib\net45\NLog.dll - - ..\packages\SharpRaven.1.4.3\lib\net45\SharpRaven.dll + + ..\packages\SharpRaven.2.1.0\lib\net45\SharpRaven.dll + True diff --git a/NLog.Targets.Sentry/Packages.config b/NLog.Targets.Sentry/Packages.config index 1349288..64026af 100755 --- a/NLog.Targets.Sentry/Packages.config +++ b/NLog.Targets.Sentry/Packages.config @@ -2,5 +2,5 @@ - + \ No newline at end of file diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index c49c8d8..2a787a7 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -52,50 +52,77 @@ internal SentryTarget(Func createRavenClient) /// /// Logging event to be written out. protected override void Write(LogEventInfo logEvent) + { + var sentryEvent = ToSentryEvent(logEvent); + if (sentryEvent == null) return; + var client = CreateClient(logEvent); + client.Capture(sentryEvent); + } + + private IRavenClient CreateClient(LogEventInfo logEvent) + { + var client = ravenClientFactory != null ? ravenClientFactory() : new RavenClient(new Dsn(Dsn)); + client.Logger = logEvent.LoggerName; + return client; + } + + private SentryEvent ToSentryEvent(LogEventInfo logEvent) { var level = TryGetErrorLevel(logEvent.Level); // Level is set to "Off", so exit. - if (level == null) return; + if (level == null) + { + return null; + } - var propertiesAsStrings = ( - from property in logEvent.Properties - let stringKey = ToStringOrNull(property.Key) - let stringValue = ToStringOrNull(property.Value) - where stringKey != null && stringValue != null - group stringValue by stringKey) - .ToDictionary(x => x.Key, x => string.Join(",", x)); + var sentryEvent = CreateSentryEvent(logEvent); + if (IgnoreEventsWithNoException && sentryEvent.Exception == null) + { + return null; + } - var tags = SendLogEventInfoPropertiesAsTags - ? propertiesAsStrings - : null; + sentryEvent.Level = level.Value; + AppendEventDetails(sentryEvent, logEvent.Properties); + return sentryEvent; + } - var extras = SendLogEventInfoPropertiesAsTags - ? null - : propertiesAsStrings; + private static SentryEvent CreateSentryEvent(LogEventInfo logEvent) + { + if (logEvent.Exception != null) + { + return new SentryEvent(logEvent.Exception); + } + else + { + return new SentryEvent(new SentryMessage(logEvent.FormattedMessage)); + } + } - var client = CreateClient(logEvent); - // If the log event did not contain an exception and we're not ignoring - // those kinds of events then we'll send a "Message" to Sentry - if (logEvent.Exception == null) + private void AppendEventDetails(SentryEvent sentryEvent, IDictionary properties) + { + var propertiesAsStrings = ConvertPropertiesToStrings(properties); + if (SendLogEventInfoPropertiesAsTags) { - if (!IgnoreEventsWithNoException) + foreach (var tag in propertiesAsStrings) { - var sentryMessage = new SentryMessage(logEvent.FormattedMessage); - client.CaptureMessage(sentryMessage, level.Value, extra: extras, tags: tags); + sentryEvent.Tags.Add(tag); } } else { - var sentryMessage = new SentryMessage(logEvent.FormattedMessage); - client.CaptureException(logEvent.Exception, extra: extras, level: level.Value, message: sentryMessage, tags: tags); + sentryEvent.Extra = propertiesAsStrings; } } - private IRavenClient CreateClient(LogEventInfo logEvent) + private static Dictionary ConvertPropertiesToStrings(IDictionary properties) { - var client = ravenClientFactory != null ? ravenClientFactory() : new RavenClient(new Dsn(Dsn)); - client.Logger = logEvent.LoggerName; - return client; + return ( + from property in properties + let stringKey = ToStringOrNull(property.Key) + let stringValue = ToStringOrNull(property.Value) + where stringKey != null && stringValue != null + group stringValue by stringKey) + .ToDictionary(x => x.Key, x => string.Join(",", x)); } private static string ToStringOrNull(object obj) From fd539db38a2fe48f5a65836db51bc3121e27eee7 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Fri, 17 Jun 2016 23:27:18 +0100 Subject: [PATCH 8/8] Specific which properties should become tags If SendLogEventInfoPropertiesAsTags is set to true, then all properties will still be used as tags. However, if set to false, then onlly the properties specified in the new TagProperties will be assigned as tags. --- .../SentryTargetTests.cs | 52 +++++++++++++++++++ NLog.Targets.Sentry/SentryTarget.cs | 26 ++++++++++ 2 files changed, 78 insertions(+) diff --git a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs index 7c38a9e..8884bdd 100644 --- a/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs +++ b/NLog.Targets.Sentry.UnitTests/SentryTargetTests.cs @@ -143,6 +143,58 @@ public void TestLoggingToSentry_SendLogEventInfoPropertiesAsTags() Assert.IsTrue(lastSentryEvent.Level == ErrorLevel.Error); } + + [Test] + public void TestLoggingToSentry_SendSpecifiedPropertiesAsTags() + { + var sentryClient = new Mock(); + SentryEvent lastSentryEvent = null; + + sentryClient + .Setup(x => x.Capture(It.IsAny())) + .Callback((SentryEvent sentryEvent) => + { + lastSentryEvent = sentryEvent; + }) + .Returns("Done"); + + // Setup NLog + var tag1 = "tag1"; + var tag2 = "tag2"; + var tag1Value = "abcde"; + var tag2Value = "fghij"; + + var sentryTarget = new SentryTarget(() => sentryClient.Object) + { + Dsn = "http://25e27038b1df4930b93c96c170d95527:d87ac60bb07b4be8908845b23e914dae@test/4", + TagProperties = tag1, + }; + var configuration = new LoggingConfiguration(); + configuration.AddTarget("NLogSentry", sentryTarget); + configuration.LoggingRules.Add(new LoggingRule("*", LogLevel.Trace, sentryTarget)); + LogManager.Configuration = configuration; + + + try + { + throw new Exception("Oh No!"); + } + catch (Exception e) + { + var logger = LogManager.GetCurrentClassLogger(); + + var logEventInfo = LogEventInfo.Create(LogLevel.Error, "default", "Error Message", e); + logEventInfo.Properties.Add(tag1, tag1Value); + logEventInfo.Properties.Add(tag2, tag2Value); + logger.Log(logEventInfo); + } + + Assert.IsTrue(lastSentryEvent.Message == "Oh No!"); + CollectionAssert.AreEqual(new Dictionary { { tag1, tag1Value } }, lastSentryEvent.Tags); + CollectionAssert.AreEqual(new Dictionary { { tag2, tag2Value } }, (Dictionary)lastSentryEvent.Extra); + Assert.IsTrue(lastSentryEvent.Level == ErrorLevel.Error); + } + [TestCase("Trace", 0, ErrorLevel.Debug)] [TestCase("Debug", 1, ErrorLevel.Debug)] [TestCase("Info", 2, ErrorLevel.Info)] diff --git a/NLog.Targets.Sentry/SentryTarget.cs b/NLog.Targets.Sentry/SentryTarget.cs index 2a787a7..22832fc 100755 --- a/NLog.Targets.Sentry/SentryTarget.cs +++ b/NLog.Targets.Sentry/SentryTarget.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using NLog.Config; +using NLog.Layouts; using SharpRaven; using SharpRaven.Data; @@ -31,6 +32,11 @@ public class SentryTarget : Target /// public bool SendLogEventInfoPropertiesAsTags { get; set; } + /// + /// A comma separated list of NLog property names to be used as tags. + /// + public string TagProperties { get; set; } + /// /// Constructor /// @@ -110,10 +116,30 @@ private void AppendEventDetails(SentryEvent sentryEvent, IDictionary RenderTagProperties() + { + if (string.IsNullOrWhiteSpace(TagProperties)) + { + return Enumerable.Empty(); + } + + return new HashSet(TagProperties.Split(',').Select(s => s.Trim())); + } + private static Dictionary ConvertPropertiesToStrings(IDictionary properties) { return (