-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature.customfields #59
Feature.customfields #59
Conversation
Release 2.0
2.1.0 Release
Release 2.1.1
Release 2.1.2
Release 2.1.3
Release 2.2.0
in style of Arrange Act Assert
Test_CustomFields_Jsonformatter_for_Splunk_Sink test works for new constructor
…s' into feature.customfields.wot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @patriklindstrom,
I think this is a great addition. Thanks so much for taking the time to create a PR!
I have noted a few items, also we are eager to stay with XUnit due to DotNet core concerns and consistency across the Serilog Organisation.
Notes.md.saved.bak
Outdated
@@ -0,0 +1,7 @@ | |||
Getting error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove this file
Notes.md
Outdated
@@ -0,0 +1,4 @@ | |||
Getting error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove this file
result.json
Outdated
@@ -0,0 +1,19 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an example event? Not sure of its purpose
sample/Sample/Program.cs
Outdated
const string SPLUNK_FULL_ENDPOINT = "http://ws2012-devops:8088/services/collector"; | ||
// Patrik local vm machine | ||
|
||
const string SPLUNK_ENDPOINT = "http://ws2012-devops:8088"; // Patrik local vm machine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patriklindstrom FYI your VM info is here. Would the Sample work better with some args? e.g. HEC token and host?
string sourceType, | ||
string host, | ||
string index, | ||
CustomFields customFields) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should CustomFields customFields
be an optional param. That seems to represent the HEC implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Should CustomFields customFields be an optional param?..."
Yes
But have I not done so by have two constructors one with customfields and one without? Or am I missing something? Do you want me to solve it differently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @patriklindstrom I missed the other constructor. 👍
@@ -49,7 +49,7 @@ public static class SplunkLoggingConfigurationExtensions | |||
/// <param name="restrictedToMinimumLevel">The minimum log event level required in order to write an event to the sink.</param> | |||
/// <param name="outputTemplate">The output template to be used when logging</param> | |||
/// <param name="formatProvider">Supplies culture-specific formatting information, or null.</param> | |||
/// <param name="renderTemplate">If ture, the message template will be rendered</param> | |||
/// <param name="renderTemplate">If true, the message template will be rendered</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice pickup!
public TestCustomFields Fields { get; set; } | ||
} | ||
[TestFixture] | ||
class SplunkCustomFieldsTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There area some great use cases here, can we move to the existing test project. We are keen to stay with XUnit due to a number of issues with NUnit & DotNet core.
As for the Arrange/Act/Assert. We should be able to achieve a similar pattern in XUnit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"We are keen to stay with XUnit "
Will do
"As for the Arrange/Act/Assert. We should be able to achieve a similar pattern in XUnit"
Hope so or I am lost. I'll give it a try.
Hi Matthew
Thanks for feedback
Ill start fixing issues you have mentioned.
And Ill move my test test to Xunit.
I will probably be done after this weekend.
Cheers
Patrik
2017-06-14 10:30 GMT+02:00 Matthew Erbs <[email protected]>:
… ***@***.**** requested changes on this pull request.
Hi @patriklindstrom <https://github.com/patriklindstrom>,
I think this is a great addition. Thanks so much for taking the time to
create a PR!
A have noted a few items, also we are eager to stay with XUnit due to
DotNet core concerns and consistency across the Serilog Organisation.
------------------------------
In Notes.md.saved.bak
<#59 (comment)>
:
> @@ -0,0 +1,7 @@
+Getting error.
Nit: Remove this file
------------------------------
In Notes.md
<#59 (comment)>
:
> @@ -0,0 +1,4 @@
+Getting error.
Nit: Remove this file
------------------------------
In result.json
<#59 (comment)>
:
> @@ -0,0 +1,19 @@
+{
Is this an example event? Not sure of its purpose
------------------------------
In sample/Sample/Program.cs
<#59 (comment)>
:
> using Serilog;
using Serilog.Sinks.Splunk;
namespace Sample
{
public class Program
{
- public static string EventCollectorToken = "2B94855F-1184-46F7-BFF1-56A3112F627E";
+ const string SPLUNK_FULL_ENDPOINT = "http://ws2012-devops:8088/services/collector";
+ // Patrik local vm machine
+
+ const string SPLUNK_ENDPOINT = "http://ws2012-devops:8088"; // Patrik local vm machine
@patriklindstrom <https://github.com/patriklindstrom> FYI your VM info is
here. Would the Sample work better with some args? e.g. HEC token and host?
------------------------------
In src/Serilog.Sinks.Splunk/Sinks/Splunk/SplunkJsonFormatter.cs
<#59 (comment)>
:
> + /// </summary>
+ /// <param name="formatProvider">Supplies culture-specific formatting information, or null.</param>
+ /// <param name="renderTemplate">If true, the template used will be rendered and written to the output as a property named MessageTemplate</param>
+ /// <param name="index">The Splunk index to log to</param>
+ /// <param name="source">The source of the event</param>
+ /// <param name="sourceType">The source type of the event</param>
+ /// <param name="host">The host of the event</param>
+ /// <param name="customFields">Object that describes extra splunk fields that should be indexed with event see: http://dev.splunk.com/view/event-collector/SP-CAAAFB6 </param>
+ public SplunkJsonFormatter(
+ bool renderTemplate,
+ IFormatProvider formatProvider,
+ string source,
+ string sourceType,
+ string host,
+ string index,
+ CustomFields customFields)
Should CustomFields customFields be an optional param. That seems to
represent the HEC implementation.
------------------------------
In src/Serilog.Sinks.Splunk/SplunkLoggingConfigurationExtensions.cs
<#59 (comment)>
:
> @@ -49,7 +49,7 @@ public static class SplunkLoggingConfigurationExtensions
/// <param name="restrictedToMinimumLevel">The minimum log event level required in order to write an event to the sink.</param>
/// <param name="outputTemplate">The output template to be used when logging</param>
/// <param name="formatProvider">Supplies culture-specific formatting information, or null.</param>
- /// <param name="renderTemplate">If ture, the message template will be rendered</param>
+ /// <param name="renderTemplate">If true, the message template will be rendered</param>
Nice pickup!
------------------------------
In test/Serilog.Sinks.Splunk.CustomFieldsTests/Serilog.Sinks.Splunk.
CustomFieldsTests/SplunkCustomFieldsTests.cs
<#59 (comment)>
:
> + public string Version { get; set; }
+ public string Rel { get; set; }
+ public List<string> Role { get; set; }
+ }
+ public class TestEventResultObject
+ {
+ public string Time { get; set; }
+ public Event @event { get; set; }
+ public string Source { get; set; }
+ public string Sourcetype { get; set; }
+ public string Host { get; set; }
+ public string Index { get; set; }
+ public TestCustomFields Fields { get; set; }
+ }
+ [TestFixture]
+ class SplunkCustomFieldsTests
There area some great use cases here, can we move to the existing test
project. We are keen to stay with XUnit due to a number of issues with
NUnit & DotNet core.
As for the Arrange/Act/Assert. We should be able to achieve in XUnit
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#59 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABv8Vhn-7pm62JhA0GJVT9MnM2aZ6Dg6ks5sD5ofgaJpZM4N5ACf>
.
--
===========================================================
I welcome VSRE emails. Learn more at http://vsre.info/
===========================================================
|
…s' into feature.customfields.wot
Small sloppmistake fixes
Hi @merbla |
@patriklindstrom a dev package should be available for this feature. It will go out with #61 |
Great thanks
Skickat från min iPhone
… 17 juni 2017 kl. 00:27 skrev Matthew Erbs ***@***.***>:
@patriklindstrom a dev package should be available for this feature. Serilog.Sinks.Splunk.2.3.0-dev-00175.nupkg. https://www.nuget.org/packages/Serilog.Sinks.Splunk/2.3.0-dev-00176
It will go out with #61
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@merbla Is it ok if I update the wiki with CustomFields examples? https://github.com/serilog/serilog-sinks-splunk/wiki/Configuring-the-sink ? |
Yep! I have not had a chance to test the dev package. |
Let me know if you do not have access. |
I have tried to add customfield into splunk using below code. But I am failed.
I could able to create log in splunk but custom fields are not visible in splunk. below is my Splunk JSON we design. { |
Add CustomField splunk feature in serilog-sink log constructor
Splunk has a feature for adding customfields that you want to be associated with the event and can be used in filters but not shown as part of the event. See http://dev.splunk.com/view/event-collector/SP-CAAAFB6 .
These index for CustomFields can be used in splunk filters like eg:
role::backend role::rest channel::verification
.These CustomFields can be string key value pairs or key array of values.
This feature (afaik) was missing in the current splunk sink.
I have made these two new classes: CustomField and CustomFieldsList also added constructor that takes fields as parameter and logic for adding this to _suffix
I added test to SplunkJsonFormatterTests. However I am not that good with Xunit test soo also I added project with NUnit test in style of Arrange,Act, Assert style.
I think these sort of filter capabilites is great at larger organisations that will make an abstract log layer around serilog and set these fields at deploy time with a config file and a tool like Octopus Deply or TFS Release Manager. That is nothing that the developer will care about. They will just focus on the what will be in de event. I think better big corporate Splunk Dasboard can be build with this feature where you want eg to search for all event for the last release on all dotnet core rest backendservices deployed to Linux machines to the Acceptance Test Channel.