toddmeinershagen / NLog.SignalR

Custom NLog target for sending logs to a SignalR hub. This release is based on SignalR 2.0.
Apache License 2.0
31 stars 9 forks source link

Uri in the Target? #15

Open da3dsoul opened 4 years ago

da3dsoul commented 4 years ago

Normally, I would expect to see something like await Hub.Clients.All.SendAsync("QueueCountChanged", currentCount);. It doesn't try to do anything fancy with the Uri or connection, because SignalR handles that.

I notice that you also don't mention

app.UseSignalR(conf =>
{
    conf.MapHub<LoggingHub>("/signalr/logging");
});

which would normally handle that for you.

As far as I can tell, the HubProxy doesn't do anything particularly special which would require breaking away from SignalR's built-in paradigm.

Is there a reason that we can't just do this?

LoggingHub stays the same.

Somewhere early in program setup:

LogManager.Configuration.AddTarget(new SignalRTarget { Name = "signalr"});
LogManager.Configuration.AddRule(LogLevel.Trace, LogLevel.Fatal, "signalr");
LogManager.ReconfigExistingLoggers();

Use SignalR

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
    app.UseAuthentication();

    app.UseSignalR(conf =>
    {
        conf.MapHub<LoggingHub>("/signalr/logging");
    });

    app.UseMvc();
}
manofthetent commented 4 years ago

@da3dsoul Thanks for your comments. This project was created many years ago and the sample code is based on OWIN-based server projects. I think you are talking about updating the sample code to reflect the new .NET Core middleware. And it also looks like the code sample for the SignalR hub signaling could be updated as well. If you want to issue a PR for those changes I would be open to review and publish.

da3dsoul commented 4 years ago

Ah fair. I saw activity this year, so I figured it was being kept updated. I may play around with it and see about a PR.

da3dsoul commented 4 years ago

I have given up on trying to do it the "right" way in netcore. Here's some things I ran into: You need to be able to access the Hub from the Target, but you can't easily inject a dependency (I think). If you only do the target code-first, then you could do it, but that's not conducive to the user experience. I don't know why, but a lot of people prefer config files over code-first. As a developer, I prefer control over what I see when a user hands me a log.

Theoretically, there are some possible solutions here, but I'm too tired to think about how well it would work. https://stackoverflow.com/questions/40611683/accessing-asp-net-core-di-container-from-static-factory-class.

da3dsoul commented 4 years ago

So I kind of started over, but you might find this approach to be more pleasant. I cleaned it up some and made a test server, but I didn't make a test client. This one has a built-in memory cache that allows sending some of the backlog on connection. It sadly doesn't easily support multiple signalr targets without extending LoggingHub and LoggingEmitter to look up another target by name.

EDIT: I'm strictly code first, so this sample project won't even obey an NLog.config.

https://github.com/da3dsoul/NLog.SignalR-Netcore

toddmeinershagen commented 4 years ago

@da3dsoul - Can you add to your zip an updated version of the sample I put together with a command line app and a web app that use this new version of the SignalR Target? The sample demonstrates multiple client types (command line, web) logging messages to the Hub and all of the clients see them.

I am having a hard time seeing the evolution to your version and what extra features it is adding. I do see the new ConnectMethodName - GetBackLogs - but I would like to see how a client would use that. Also, the Target seems to be missing the Uri and HubName properties.

Thanks, Todd

On Tue, Aug 4, 2020 at 7:38 AM da3dsoul notifications@github.com wrote:

So I kind of started over, but you might find this approach to be more pleasant. I cleaned it up some and made a test server, but I didn't make a test client. This one has a built-in memory cache that allows sending some of the backlog on connection. It sadly doesn't easily support multiple signalr targets without extending LoggingHub and LoggingEmitter to look up another target by name.

NLog.SignalR-Netcore.zip https://github.com/toddmeinershagen/NLog.SignalR/files/5021821/NLog.SignalR-Netcore.zip

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/toddmeinershagen/NLog.SignalR/issues/15#issuecomment-668544770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABLKZH2JSM6AZT2UG66FP3R67XLZANCNFSM4PRUJ4SA .

da3dsoul commented 4 years ago

I cleaned it up and put it on GitHub. I plan to expand the test projects.

toddmeinershagen commented 4 years ago

Thanks for writing back. I hope my questions for more details didn't cause you to start working on your own project. I just wanted to better understand what you were trying to accomplish. I think your repo is helping me understand why you wanted to set up the endpoint outside of the target. Do you want to collaborate to update the current version of NLog.SignalR to better accommodate some of the new SignalR concepts - or go it on your own?

On Wed, Aug 5, 2020 at 6:31 PM da3dsoul notifications@github.com wrote:

I cleaned it up and put it on GitHub. I plan to expand the test projects.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/toddmeinershagen/NLog.SignalR/issues/15#issuecomment-669554362, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABLKZDIJG5KMFQR3MEAPJLR7HMULANCNFSM4PRUJ4SA .

manofthetent commented 4 years ago

@da3dsoul So, am I correct in saying that you are trying to solve 2 problems with your new implementation? 1) Update the documentation to reflect how to set up the LoggingHub in ASP.NET Core and 2) Allow for some buffering of logs so that when new Clients connect to the Hub, they will receive up to that maximum buffered number of Logs?

da3dsoul commented 4 years ago

I'm fine with collaboration. I put it on GitHub because it was easier than dealing with a zip file. I started a new workspace because the old one wanted me to install too many older NDKs. 4.5 isn't even available anymore. I started cleaning it up, then decided it would be better for the project to just start a newer, more refined workspace.

da3dsoul commented 4 years ago

With the newer versions of asp.net and SignalR, all of the information you need for the endpoints can be found from the mappings in Startup.cs. With the older system, it was a mess. If we can get the generics and dependency injection figured out for multiple targets, then I could call it feature-complete. I targeted netstandard, so it theoretically could be backwards compatible, but I've never used an OWIN based service, only asp.net

manofthetent commented 4 years ago

@da3dsoul Okay - so I have been reacquainting myself with SignalR this weekend. It's been several years since I have worked with the technology and things have changed as you said.

One thing I learned is that there are two types of SignalR hubs - one for ASP.NET Web / OWIN and one for ASP.NET Core. The behavior is different with those two hubs so the Clients and Servers that communicate with those hubs are not interchangeable and require different client libraries. So, I think we would need to create the NLog.SignalR-NetCore as a separate nuget package, if we want to support the ASP.NET Core SignalR hub.

More details on the differences between the two can be found here.

da3dsoul commented 4 years ago

That's fair. I have a repo up already for that. I know most places are acquainted with this version, including the NLog official page. We can link each other's repositories, migrate mine over here, create an organization, or something.

snakefoot commented 1 year ago

See also #20