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

Minimal memento package? #105

Open
rofinn opened this issue Sep 26, 2018 · 10 comments
Open

Minimal memento package? #105

rofinn opened this issue Sep 26, 2018 · 10 comments

Comments

@rofinn
Copy link
Member

rofinn commented Sep 26, 2018

It may make sense break up Memento.jl into a minimal base package (Minimento?) which doesn't have any external dependencies and the more feature rich package that we (at Invenia) want to use.

@rofinn
Copy link
Member Author

rofinn commented Apr 2, 2019

I think I'd prefer that we work towards integrating Memento.jl better into the base julia logging system. Ideally, the flexible configuration system from Memento can be ported into the Logging stdlib and Memento would just define a bunch of default record, handler and formatters that handle our use cases.

@nicoleepp Maybe you have some thoughts on how best to approach this (e.g., API changes to make, features to scrap)?

@nicoleepp
Copy link
Contributor

I will take a more in-depth look into the base julia logging system and get back on this.

@nicoleepp
Copy link
Contributor

As a first step it might be useful to try to better integrate with base logging as is (first make the changes in Memento), and then work on porting Memento's configuration system to the Logging stdlib. We can follow the MicroLogging example and integrate Memento with the base julia logging system that way.

The biggest change for this would be having a "Logger" that subtypes Base's AbstractLogger that would need to get set in modules as the global logger instead of Base's default (we already have a CoreLogger we would just need to tie that into the rest of Memento and documentation for user workflow). The rest of Memento's api seems to me that it can stay the same except for maybe the actual logging calls as discussed in the paragraph below. Some more changes might become apparent once we are actively working on this stuff.

Something to consider is that we can either continue having logging functions info(), warn(), etc. that would then call base logging with the logger to use as the group (so that we can control loggers via name) similar to what depwarn does. Or we can just have a logger per module and use the base logging macros @info, @warn, etc. plus add the few extras like @notice in the same way that base does. The second option means logging statements in other modules won't be able to hook into a logger in another package/module like Memento currently allows but should allow different config levels/formatting per module. Note that the first option might make it harder to add this to base in the future, as it changes the base logging api by requiring a logger name to be passed in to every logging call.

@nicoleepp
Copy link
Contributor

Also, I'm not sure using group would work without requiring changes to base, probably this function: https://github.com/JuliaLang/julia/blob/master/base/logging.jl#L382

@rofinn
Copy link
Member Author

rofinn commented Apr 4, 2019

The biggest change for this would be having a "Logger" that subtypes Base's AbstractLogger that would need to get set in modules as the global logger instead of Base's default (we already have a CoreLogger we would just need to tie that into the rest of Memento and documentation for user workflow).

Yeah, that's was idea behind the CoreLogger. My only concern was that piping julia logs through the CoreLogger has previously been significantly slower than julia's logging. This is largely because all logs currently flow through the base logging system before being filtered by Memento. It'd be nice if we could somehow insert Memento a little earlier in the pipeline to filter out records sooner (e.g., before the line number, filename are extracted).

The second option means logging statements in other modules won't be able to hook into a logger in another package/module like Memento currently allows but should allow different config levels/formatting per module.

Could we support both? It seems like the second option already works and folks could hook into other loggers by just changing the _module=mod value being passed to the logging macro (that's what the CoreLogger is using to figure how which memento logger to use anyways? NOTE: It'd be nice if those keywords were cleaner. I don't think we should reintroduce the functional interface back into base/stdlib, but we could choose to continue supporting it in Memento.

@nicoleepp
Copy link
Contributor

nicoleepp commented Apr 4, 2019

It'd be nice if we could somehow insert Memento a little earlier in the pipeline to filter out records sooner (e.g., before the line number, filename are extracted).

I think our current Memento setup is the best way to do this, as this stuff gets done pretty much at the start when base logging macros get called.

My only concern was that piping julia logs through the CoreLogger has previously been significantly slower than julia's logging.

Is Memento itself a lot slower than julia's logging or is it the combination of Memento + base logging that makes it slow?

I don't think we should reintroduce the functional interface back into base/stdlib, but we could choose to continue supporting it in Memento.

If going through base logging is indeed slow/slower than just using Memento, I don't see if it is worth doing option one ie. changing Memento's logging functions to call base logmsg which is the thing that extracts line numbers, filenames, etc as it would make option one just as slow as option two. Option two is pretty much supported you're right but we could have some additional documentation on it to make it clear I guess.

@rofinn
Copy link
Member Author

rofinn commented Apr 4, 2019

Is Memento itself a lot slower than julia's logging or is it the combination of Memento + base logging that makes it slow?

For a while actually logging with base logging was much slower than Memento. The primary issue was that the CoreLogger needed to let all log message from base through, which made it slow even for debug messages that Memento would ignore. Memento itself should be comparable in speed because it lazily evaluates most of the record attributes. The only slowness would be from when we need to propagate records up the hierarchy... which we can't really avoid.

NOTE: I haven't benchmarked this in over 6 months, so it could be better now.

@iamed2
Copy link
Member

iamed2 commented Apr 4, 2019

One thing we can do is limit logging based on the log level environment variable that Julia uses, which would mean by default we wouldn't generate debug logs. In general, it would be a lower (upper??) bound, so we could limit logs further than that.

I have no idea if I am expressing myself effectively lol

@rofinn
Copy link
Member Author

rofinn commented Apr 6, 2019

Yeah, we can set the base julia global log level to filter out debug messages before it hits the Memento code, but if we want just some debug messages in Memento we need to have base generate logs for everything. This is the part I have issue with because base logs are relatively slow and don't lazily evaluate. If we could have Memento's lazy evaluation of records then this would be less of an issue. Generating a new records should be cheap and fields should only be evaluated when needed.

@rofinn
Copy link
Member Author

rofinn commented Feb 10, 2020

I don't see us coming up with a good solution for this anytime soon, so I'm not going to include this proposal in a 1.0 milestone.

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