serilog-contrib / serilog-sinks-splunk

A Serilog sink that writes to Splunk
https://splunk.com
Apache License 2.0
46 stars 47 forks source link

Feature/161 subsecond decimals #172

Closed EEParker closed 6 months ago

EEParker commented 6 months ago

The first commit adds a sub-second decimal option to address #161.

The second commit adds a flag for disabling RenderedMessage per #167.

These both seem optional, but could be nice to have. I've included them here for review.

EEParker commented 6 months ago

@VictorioBerra If you have time I would like your opinion/review on these changes. Thank you

VictorioBerra commented 6 months ago

Looks like the link to the docs is dead: http://dev.splunk.com/view/SP-CAAAE6P

Is there a more up to date link describing the tiemstamp format and the precision?

EEParker commented 6 months ago

https://docs.splunk.com/Documentation/Splunk/9.2.0/SearchReference/Commontimeformatvariables it looks like 3, 6 or 9 decimals are supported, so the PR probably needs to be updated to use an enum for millisecond, microseconds or nanoseconds.

VictorioBerra commented 6 months ago

That is very helpful! Thank you.

On Sat, Mar 16, 2024, 4:08 PM Jeff Parker, PE @.***> wrote:

https://docs.splunk.com/Documentation/Splunk/9.2.0/SearchReference/Commontimeformatvariables it looks like 3, 6 or 9 decimals are supported, so the PR probably needs to be updated to use an enum for millisecond, microseconds or nanoseconds.

— Reply to this email directly, view it on GitHub https://github.com/serilog-contrib/serilog-sinks-splunk/pull/172#issuecomment-2002133231, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWMN27KKG6WPJCQJGRZHT3YYSYFPAVCNFSM6AAAAABEY7NQ7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBSGEZTGMRTGE . You are receiving this because you were mentioned.Message ID: @.***>

EEParker commented 6 months ago

I've updated this PR with the enum, added unit tests and updated documentation links.

EEParker commented 6 months ago

I've updated this PR with the enum, added unit tests and updated documentation links.

EEParker commented 6 months ago

@VictorioBerra I added the test code and splunk props.conf to make this work in the sample project.

I've also verified that microseconds (6) work, but I couldn't get Splunk ingestion to work with nanoseconds (9). I believe this is on the Splunk side but I'm not sure what is needed.

image

VictorioBerra commented 6 months ago

Thanks for doing that, @EEParker can you share the changes you made to the props.conf? Maybe I could post on the Splunk forums about this?

EEParker commented 6 months ago

Thanks for doing that, @EEParker can you share the changes you made to the props.conf? Maybe I could post on the Splunk forums about this?

props.conf

TIME_PREFIX = \"time\"\:\s*\"
TIME_FORMAT = %Y-%m-%dT%H:%M:%S.%9N%Z

https://github.com/serilog-contrib/serilog-sinks-splunk/pull/172/files#diff-7bdfdc66d9929e53da33186cc68b1ccf09bb584d6b50ce4c0be241b259420395R1-R2

EEParker commented 6 months ago

Are you comfortable with merging this? I think a follow-up PR can be made to address any splunk samples that need to be changed. I would like to merge this in before #174.

VictorioBerra commented 6 months ago

@EEParker Yeah id say go ahead, I like the addition of the .editoconfig and the docker additions.