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

Pick a logging abstraction #18

Closed
rogeralsing opened this issue Jan 20, 2017 · 10 comments
Closed

Pick a logging abstraction #18

rogeralsing opened this issue Jan 20, 2017 · 10 comments

Comments

@rogeralsing
Copy link
Contributor

My impression is that the https://www.nuget.org/packages/microsoft.extensions.logging.abstractions/ is what will become the de-facto standard for logging in .NET.
After all, this is what is baked into Asp.NET Core.

Thoughts, should we aim for this?

@cpx86
Copy link
Contributor

cpx86 commented Jan 20, 2017

Seems to be the standard for typical logging scenarios - see comment in the aspnet/Logging repo here: aspnet/Logging#332 (comment)

Another option that could be interesting is EventSource/ETW logging. E.g. if we want to log diagnostics.

@adamhathcock
Copy link
Member

I'm interested in taking a stab at this. How should this be done?
For internal logging only?
Added to the context like Akka.net? (I guess this is correct?)

I'm used to dependency injection so if something needs logging it just asks for it.

@adamhathcock
Copy link
Member

Giving this a go and running into chicken-and-egg problems.

I can't just replace all usages of Console with logger because things like EventStream are always static. There's a lot of this.

I want to solve this by changing Props to really be IServiceProvider and use IoC to create objects. I'm guessing this is probably an unwelcome change.

@adamhathcock
Copy link
Member

Any thoughts about using IoC to resolve/create the proto actor primatives to inject logging? I would use the interfaces and stuff from the Microsoft.Extensions like logging.

I can try my best with a static implementation if that's desired instead.

@rogerasling @cpx86

@cpx86
Copy link
Contributor

cpx86 commented Mar 13, 2017

I'm a bit undecided at the moment, to be honest. I'd like to avoid having any container abstraction or service locator within the library. One question is for which scenarios we need logging. E.g. for general purpose actor logging, it can be injected via the actor producer. To log e.g. all received messages, it can be added as a middleware. If we want to log mailbox events, that can be done with the mailbox statistics extension. If there are more use cases, perhaps we can provide similar extension points for those?

@adamhathcock
Copy link
Member

Actor logging can be done themselves via the producer. I was going to optionally allow usage from the context because I figured the library itself would use that.

However I don't know how a logger factory can be shared among the primatives. Maybe nothing is needed.

@rogeralsing
Copy link
Contributor Author

In Go, we have a special EventStream where we push a LogMessage onto.
That way, anyone can subscribe to that and apply whatever log we want.
I think that would be the cleanest here too.

e.g.
Proto.Log.Stream.Subscribe( e => { myLogger.Warn(..) })

@adamhathcock
Copy link
Member

Cool. I'll look at ripping that off.

@rogeralsing
Copy link
Contributor Author

I've refactored the eventstream a little bit so it is possible to create a generic instance of it.
e.g.:

var stream = new EventStream<LogMessage>();

@adamhathcock adamhathcock mentioned this issue Mar 14, 2017
@cpx86
Copy link
Contributor

cpx86 commented Mar 22, 2017

Fixed by #81

@cpx86 cpx86 closed this as completed Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants