Add specific tag selection #10
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
June 17, 2016 16:00
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
Avoid possible edge cases when converting from Dictionary<object, object> to Dictionary<string, string>. 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.
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.
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.
- This can be done using the target filter condition feature in NLog. - Separate check for exception from condition of skipping logging.
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.
Due to the old API being deprecated, use the new API, which requires the sentry event to be constructed in a different way.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is dependent on #9 and contains the commits from that branch within it.
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.
Upgrade SharpRaven to 2.1.0
Due to the old API being deprecated, use the new API, which requires the sentry event to be constructed in a different way.
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.
Therefore the target has no need for a custom layout.