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

Log exporter App #1056

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

Conversation

zbalkan
Copy link

@zbalkan zbalkan commented Sep 28, 2024

I created the initial draft for the log exporter app. This works like a forwarder agent, such as fluentd. The single input is the DNS server. The output is now simply file, HTTP and Syslog. It is opinionated when it comes to Syslog as it requires the new format. The HTTP client is very simple and it can be improved further. File logging works as expected by appending newline delimited JSON logs.

I intend to implement DnsTap logs as well but it will require some extra work for the wire protocol implementation. It needs more time.

@zbalkan zbalkan changed the title Log exporter Log exporter App Sep 28, 2024
@zbalkan
Copy link
Author

zbalkan commented Sep 28, 2024

I found that the cleanup extension touched some files out of the scope as well. I will clean them up, and squash the commits into a single one to ensure a clean commit history.

@zbalkan zbalkan marked this pull request as ready for review September 29, 2024 07:54
@zbalkan
Copy link
Author

zbalkan commented Sep 29, 2024

Finished the cleanup. Pending review.

@ShreyasZare
Copy link
Member

Thanks for the PR. Will check it soon.

@zbalkan
Copy link
Author

zbalkan commented Sep 29, 2024

Export Strategy

File Export Strategy

image

HTTP Export Strategy

This demo uses a simple dotnet core web API with single endpoint named /logs, and prints the received data to stdout. The logs are sent from the Log Exporter App to this

image

Syslog export strategy

When the Syslog server accepts the BSD log format (RFC 3164):

image

When the Syslog server accepts new format (RFC 5424):

image

@zbalkan
Copy link
Author

zbalkan commented Sep 29, 2024

FYI, the Syslog exporter now accepts only a single target. I am working on sending to multiple targets to be configurable. Is it going to be a send all, round robing or failover, is the issue. If you like, we can do it in a future version. But I believe I can finish it today.

@zbalkan
Copy link
Author

zbalkan commented Oct 3, 2024

I have made some major changes, especially to minimize the time, CPU and memory usage on string allocation for logs. Multiple syslog targets are ignored as well.

Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
@ShreyasZare
Copy link
Member

Thanks for the updates. Will check them soon.

@zbalkan
Copy link
Author

zbalkan commented Oct 17, 2024

FYI, after some benchmarks, I see that there is not much performance gain in all use cases. Under heavy loads, the memory allocation is minimized as expected while the processing time is higher. It would add latency and create issues. It is better to stick to the defaults of using System.Text.Json and StringBuilder. A new commit is coming soon.

@zbalkan
Copy link
Author

zbalkan commented Oct 20, 2024

I rolled back the changes. Since we are using the simplest way to serialize, and .NET 8 performance is optimal, this is now both easy to maintain and sufficiently performant. I know it is hard to accept external PRs and if the PR owner will not maintain in the future, it would be a burden on you.

@ShreyasZare
Copy link
Member

I rolled back the changes. Since we are using the simplest way to serialize, and .NET 8 performance is optimal, this is now both easy to maintain and sufficiently performant. I know it is hard to accept external PRs and if the PR owner will not maintain in the future, it would be a burden on you.

Thanks for the update. It may take some time for me to test this PR since I am already busy with few things.

@ShreyasZare
Copy link
Member

I just checked the changes briefly now and found that you are using SyslogNet.Client nuget package which is built for .NET Framework 4.0. This will have issues with deployments on platforms other than Windows. Can you please check and replace this with another package which is built for minimum .NET 5.0 so that there wont be any issues running it on all platforms. Thanks.

@zbalkan
Copy link
Author

zbalkan commented Nov 20, 2024

Hi @ShreyasZare,

I moved all solutions to Serilog which enhances the supported code as the dependency instead of maintaining my own. However, I am having issues on App loading phase. When loading the application assembly, I get the error within the DnsApplicationAssemblyLoadContext method: Could not load file or assembly 'Serilog.Sinks.Syslog, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

Because of this, weirdly all dependencies are loaded except for my own app.
image

The dependency is not loaded properly, so that the app fails to load. It is hard to debug things when reflection is used. Can you suggest anything for troubleshooting? I tried to check but I could not find anything.

The weird thing is that there is no Serilog.Sinks.Syslog DLL. It is Serilog.Sinks.SyslogMessages. It is searching for something that does no exist.
image

@ShreyasZare
Copy link
Member

Thanks for the updates. I think you will need to add the Serilog nuget package reference too since its a dependency for the other references. Let me know if that works.

<PackageReference Include="Serilog" Version="4.1.0" />

@zbalkan zbalkan marked this pull request as draft November 21, 2024 15:22
@zbalkan
Copy link
Author

zbalkan commented Nov 21, 2024

It looks like having more DLLs as dependencies is an issue. When the dependencies are not loaded earlier, the method appAssembly.GetTypes(); throws exception. In order to load the plugin properly, I added the z to the plugin DLL, so that it is loaded latest. In order to find the plugin, we need to enumerate types. But to enumerate types, we need to load dependencies earlier. It becomes a loop at that point.

Even putting it to the last was not enough. I created this monstrosity to be able to solve this. But this is a workaround, not a solution.
deps.patch

Edit: It would have been amazing if .Net allowed single file DLLs but it is allowed only for executables.

@ShreyasZare
Copy link
Member

It looks like having more DLLs as dependencies is an issue. When the dependencies are not loaded earlier, the method appAssembly.GetTypes(); throws exception. In order to load the plugin properly, I added the z to the plugin DLL, so that it is loaded latest. In order to find the plugin, we need to enumerate types. But to enumerate types, we need to load dependencies earlier. It becomes a loop at that point.

Even putting it to the last was not enough. I created this monstrosity to be able to solve this. But this is a workaround, not a solution. deps.patch

Edit: It would have been amazing if .Net allowed single file DLLs but it is allowed only for executables.

Its possible that the DLL is a native binary and thus its failing to load as a .NET assembly. I think this issue is due to some missing dependency. Try to add the nuget package for Serilog.Sinks.Syslog too as its what is being expected as seen in the error earlier.

@ShreyasZare
Copy link
Member

I just checked that you have changed async code to sync code. Was there some issue with using async? Its highly preferred to use async since it will not have any effect on the overall DNS server performance due to blocking of thread pool threads when not using async.

@zbalkan
Copy link
Author

zbalkan commented Nov 22, 2024

Hi @ShreyasZare,

I tried to add all transitive dependencies as a direct dependency, but it didn't make any difference. The loop takes the DLLs through a loop. If the dependencies are loaded before the plilugin, it's okay. But if not, it makes the type enumeration fail.

Second, I converted it to sync back as there was no internal. Async method and I was wrapping the function into a Task which seemed unnecessary. I can check what can I do and make it async if possible.

@ShreyasZare
Copy link
Member

I tried to add all transitive dependencies as a direct dependency, but it didn't make any difference. The loop takes the DLLs through a loop. If the dependencies are loaded before the plilugin, it's okay. But if not, it makes the type enumeration fail.

Will test this once and let you know.

Second, I converted it to sync back as there was no internal. Async method and I was wrapping the function into a Task which seemed unnecessary. I can check what can I do and make it async if possible.

Its best to use Task and call the methods async since otherwise it can potentially block threads in thread pool and cause issues with other tasks running on the server.

@ShreyasZare
Copy link
Member

I have found the issue with the dependency loader. Will share the solution soon.

@zbalkan
Copy link
Author

zbalkan commented Nov 22, 2024

Hi. I've been trying as well but it may be a tunnel vision. I built a dictionary to resemble a dependency graph in the directory. Then, I check dlls, add their dependencies. When the graph is populated, then I check and load them, so that the plugin is loaded the latest. It may be overkill.

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

Successfully merging this pull request may close these issues.

2 participants