Skip to content
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

Just updated from 3.7.0 to 4.0.0/4.0.1 and I cant get splunk to index the data #183

Closed
MatthewHays opened this issue Apr 5, 2024 · 18 comments · Fixed by #185
Closed

Just updated from 3.7.0 to 4.0.0/4.0.1 and I cant get splunk to index the data #183

MatthewHays opened this issue Apr 5, 2024 · 18 comments · Fixed by #185

Comments

@MatthewHays
Copy link

I updated my project to the latest serilog splunk package and now my logs no longer appear in splunk.
I'm using Serilog 3.1.1 and I have a test case to log a handful of lines to Splunk that works fine with 3.7.0 but doesn't with 4.0.0 and 4.0.1.
I've enabled self logging and get nothing, I've force flushed the log and disposed, and setting the batch size to 1 etc and nothing seems to work anymore. Have I missed something obvious? Our current Splunk backend is v7.3.1

Apologies if this is the wrong place to raise this, happy to redirect my issue elsewhere (StackOverflow?)

Thanks

@MatthewHays
Copy link
Author

For what its worth, this is our test case. We've had this for a number of years and it worked fine before

[Test, Explicit("Used for diagnosing Splunk connectivity issues")]
public void WriteToSplunk()
{
    var splunkEndpoint = "http://fx-uat-splunk:8088";
    var splunkToken = "<redacted>";

    var loggerConfig = new LoggerConfiguration();

    Serilog.Debugging.SelfLog.Enable(msg => Debug.WriteLine(msg));

    loggerConfig
        .MinimumLevel.Verbose()
        .WriteTo.EventCollector(splunkEndpoint, splunkToken, restrictedToMinimumLevel: LogEventLevel.Debug);

    using (var logger = loggerConfig.CreateLogger())
    {
        Serilog.Log.Logger = logger;

        logger.Information("Information message {@param}", new { Property1 = 1, Property2 = 2 });
        logger.Warning("Warning message {@param}", "Hello this is a string");
        logger.Error(new Exception("Bang"), "Error message");
    }
    Serilog.Log.CloseAndFlush();
 }

@EEParker
Copy link
Collaborator

EEParker commented Apr 5, 2024

Thanks for the report, Do you have some more information about your environment, e.g. dotnet target framework, host operating system?

@EEParker
Copy link
Collaborator

EEParker commented Apr 5, 2024

Additionally, have you tried setting your splunk endpoint to use the full event collector address?
e.g. http://splunk:8088/services/collector/event

ref: https://docs.splunk.com/Documentation/SplunkCloud/latest/Data/UsetheHTTPEventCollector#Send_data_to_HTTP_Event_Collector_on_Splunk_Enterprise

@MatthewHays
Copy link
Author

MatthewHays commented Apr 5, 2024

Hi, I've just debugged through this, and hacked the new and old code about in VS, and it looks like a bug with the new time logic
ToEpoch in 3.7.0 returns a correct time, but in 4.0.1 it appears to generate a time one hour into the future, which is why splunk isnt showing my logs. I'm the UK and currently at UTC+1

image

image

@MatthewHays
Copy link
Author

MatthewHays commented Apr 5, 2024

Just to add, value being passed in is
image

new Epoch func returns = Friday, 5 April 2024 16:55:48.667 (GMT)
old Epoch func returns = Friday, 5 April 2024 15:55:48.667 (GMT)

@MatthewHays
Copy link
Author

Converting value to UTC first fixes this, ie

ToSeconds(value.ToUniversalTime().Ticks - Epoch.Ticks);

EEParker added a commit that referenced this issue Apr 5, 2024
* Attempt creating a reproduction for #183

* Add unit tests for local timezones

* Reset docker version
@EEParker EEParker reopened this Apr 5, 2024
@EEParker
Copy link
Collaborator

EEParker commented Apr 5, 2024

@MatthewHays I've updated the code with your suggestions and added some unit tests for this condition. Can you try nuget version 4.0.1-dev-00023?

@EEParker EEParker added the bug label Apr 5, 2024
@MatthewHays
Copy link
Author

MatthewHays commented Apr 5, 2024

@EEParker Thanks for the fast turnaround. I'll be on Easter holidays for a bit, but that change looks good,. I'll throw the new nuget package into our code base when I get back.

@EEParker
Copy link
Collaborator

EEParker commented Apr 5, 2024

I'm going to go ahead and release the 4.0.2 as a patch to fix this time error. If it doesn't resolve your issue, I'll reconnect with you after the weekend.

@VictorioBerra
Copy link
Member

@MatthewHays Thank you for sharing that test.

A small tidbit, we just updated our servers to Windows 2022, and in our .NET 4.8 app Splunk logging stopped. Took Wireshark to tell us that the .NET app was trying to use TLS 1.0. We needed to add a registry key to fix it which forces Windows Server to use tls 1.2+ for .NET. Something like this: https://www.seequent.com/how-to-enable-tls-1-2-as-default-in-windows/

I just point this out as even a unit test like the one you have would not have caught that. Maybe someone from Google will find this someday too.

@MatthewHays
Copy link
Author

It's not specifically a unit test, its just something we run when something changes on our side related to Splunk (firewalls, IPs, libraries, versions etc) and we want to manually confirm that logs are still being indexed. I just threw this into this code base locally to drive the code in 3.7.0 and 4.0.1 to find the difference in the debugger that caused the prob. We could programmatically query Splunk via the API to check the messages appear or I supposed we could regression test that the exact same payload gets sent on the wire as you expect, by mocking out the httpclient and comparing against a canned result, but will leave that up to you guys as to what you think is best. (And yes, no suite of tests will catch every possible scenario)

@EEParker
Copy link
Collaborator

EEParker commented Apr 9, 2024

@MatthewHays Thank you for sharing that test.

A small tidbit, we just updated our servers to Windows 2022, and in our .NET 4.8 app Splunk logging stopped. Took Wireshark to tell us that the .NET app was trying to use TLS 1.0. We needed to add a registry key to fix it which forces Windows Server to use tls 1.2+ for .NET. Something like this: https://www.seequent.com/how-to-enable-tls-1-2-as-default-in-windows/

I just point this out as even a unit test like the one you have would not have caught that. Maybe someone from Google will find this someday too.

I just ran into this as well,

Fix for TLS 1.2 on windows servers

reg add HKLM\SOFTWARE\Microsoft\.NETFramework\v4.0.30319 /v SchUseStrongCrypto /t REG_DWORD /d 1

Note: this may have widespread impacts, so be sure you need it.

@VictorioBerra
Copy link
Member

@EEParker Good point. For us, we had just provisioned new 2022 servers using an in-place upgrade. We were not sure why our .NET 4.8 app + Serilog Splunk was trying to use TLS 1.0. Wireshark showed us the handshake was failing. Upon setting that registry key everything started flowing.

@EEParker
Copy link
Collaborator

EEParker commented Apr 9, 2024

I added this to the wiki. There might be a better spot, but I wanted to highlight it. https://github.com/serilog-contrib/serilog-sinks-splunk/wiki#windows-tls-12

@EEParker
Copy link
Collaborator

EEParker commented Apr 9, 2024

@MatthewHays can you confirm if 4.0.3 and the TLS configuration resolve this issue for you?

https://www.nuget.org/packages/Serilog.Sinks.Splunk/4.0.3

@MatthewHays
Copy link
Author

MatthewHays commented Apr 12, 2024

@EEParker I'll be back in the office on the 15th so will check it then, this would break anyone pushing logs from a + GMT timezone (likely why it wasn't spotted in the US, as it will work but just the timestamp will be wrong). Happy for this to be closed, the fix is the same I did locally to get it working.

@MatthewHays
Copy link
Author

@EEParker yep working, all good thanks.

@EEParker
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants