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 before the default log store is accessed results in dropped messages #115

Open
NickEntin opened this issue Mar 1, 2021 · 2 comments
Labels
bug Something isn't working as expected

Comments

@NickEntin
Copy link
Collaborator

The default log store get lazily created the first time you access the defaultLogStore property. If you follow the one-line setup approach, this happens immediately since we access the default log store in order to create the bug reporter. If you don't use this setup, however, instead opting to create the bug reporter later, calling log(...) will send the message nowhere (or really will send the message to other log observers you've added, but not the default log store).

I think the expected behavior would be to create the default log store the first time log(...) is called if it doesn't already exist.

@NickEntin NickEntin added the bug Something isn't working as expected label Mar 1, 2021
@dfed
Copy link
Collaborator

dfed commented Mar 1, 2021

I'm not sure we should autocreate the default log store – if a team does not want to utilize the default store, should we make them?

@NickEntin
Copy link
Collaborator Author

I think the fact that the log distributor exposes a defaultLogStore implies that there will be some default log store (i.e. if you don't explicitly provide one, we'll create it for you). Perhaps it's reasonable to use the distributor without a log store, but in that case we'd probably want to change the API to make it clear the default log store may not exist.

Right now the getter for defaultLogStore changes behavior, which seems like something we should definitely avoid.

log("Hello world") // This goes nowhere.

_ = ARKLogDistributor.default().defaultLogStore
log("Hello world") // This goes to the log store.

At the very least, I think we should autocreate the default log store when you call log(...) when there are no other log observers, so the logs have somewhere to go. But the side-effecty getter still seems problematic.

What do you think about doing a two part change:

  1. Update -[ARKLogDistributor _logMessage_inLogDistributingQueue:] to create the default log store if it doesn't already exist in a new patch release.
  2. File an issue to investigate updating ARKLogDistributor's API to better support not having a default log store, which would be for the next major release of CoreAardvark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

2 participants