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

Logging #81

Merged
merged 3 commits into from
Mar 19, 2017
Merged

Logging #81

merged 3 commits into from
Mar 19, 2017

Conversation

adamhathcock
Copy link
Member

Use a static log object. Instrument some code and use an example.

I recognize this isn't what is discussed in: #18

Essentially, if I try to put an EventStream in I'll have to fan-in from all usages of ILogger (or some other interface) to the stream and put data in a custom LogMessage. Then fan-out again to ILogger to allow usage of ILoggerFactory and all the logging implementations that use that.

Ideally, you want to use ILogger or ILoggerFactory directly in the code to be instrumented. The problem was always how to get one of those objects to the code.

An event stream doesn't feel like a fit because the LoggerFactory itself is a pub/sub type construct in which all the implementations are subbing to the factory and the code is pubbing.

If this doesn't fit the vision at all, that's cool. I guess I just don't see how log messages should be involved in an actual event stream.

@CLAassistant
Copy link

CLAassistant commented Mar 14, 2017

CLA assistant check
All committers have signed the CLA.

@adamhathcock
Copy link
Member Author

Closing in favor of #86

@adamhathcock adamhathcock deleted the logging branch March 16, 2017 16:51
* dev:
  Add CurrentSynchronizationContextDispatcher
  added nuget section
  Updated readme with simple initial content
  ClusterProvider interface
  cluster skeleton
@adamhathcock adamhathcock reopened this Mar 17, 2017
* commit 'a62d6b203c935c0af1086cca05b1cc81d296856a':
  Bumped version
  added sender middleware example
  Handle null responses in FutureProcess
  some docs
  Throw exception when a future gets a message it does not expect.
  Disable/hide bounded mailbox because it has issues
  Increase sleep in mailbox tests
  Re-added lost line of code
  Reschedule mailbox after completion of non-completed task
  Fix proj versions
  Package info
@adamhathcock
Copy link
Member Author

@cpx86 reopened this.

I have a static logger factory so we don't need to use DI.

I was wondering if the log should be moved to Proto.Mailbox or it's own project as Proto.Actor isn't the lowest level library.

@adamhathcock adamhathcock mentioned this pull request Mar 17, 2017
@cpx86
Copy link
Contributor

cpx86 commented Mar 17, 2017

I think Proto.Actor is the best fit. The reason Mailbox is separate is because we want to make it possible to use Mailboxes for other purposes than Actors. But Proto.Actor is really the core library. Having a separate Proto.Logging feels a bit overkill since it probably wouldn't be usable outside of Proto.Actor.

@adamhathcock
Copy link
Member Author

No problem. Then I guess this is ready unless the style isn't liked.

@rogeralsing rogeralsing merged commit 7b41116 into asynkron:dev Mar 19, 2017
@cpx86 cpx86 mentioned this pull request Mar 22, 2017
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.

4 participants