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

Update LoggerConfig from parent packages #6

Open
pacien opened this issue Apr 3, 2019 · 5 comments
Open

Update LoggerConfig from parent packages #6

pacien opened this issue Apr 3, 2019 · 5 comments

Comments

@pacien
Copy link

pacien commented Apr 3, 2019

Currently, using LoggerConfig.fromPackage(String/Package).update(...) only affects Loggers for classes directly under the referred package.

It would be handy to have a way to configure all loggers for all nested sub-packages as well.

For instance LoggerConfig.fromPackage("ba", recursive=true).update(opt -> opt.enable(false)); should disable the Logger of the ba.zoo.Ka class.

@forax
Copy link
Owner

forax commented Apr 3, 2019

Hi Pacien, the short answer is no, the long answer is
the configuration is independent of the Logger, you can set it before or after the logger creation, it will always work so users do not have to care with the initialization sequence, it also means that you can change the configuration at runtime.

To avoid to test the configuration each time you log something (for perf reason), each logger is using the class java.lang.invoke.SwitchPoint [1] to detect when the configuration change, if you want to test recursive package, it means that you have to create as many switchpoints has the number of subpackages even if the configuration doesn't use recursive packages, so the amount of metadata created for each Logger will depends of the number of '.' inside the package name. This seems a waste of resources given that everybody will have to pay the cost even if it doesn't use a configuration that uses sub-packages.

Growing the number of SwitchPoints has a nasty side effect, it means you need to record the dependencies in the JITed code because for each logger calls (technically it's more than each calls because calls can be inlined) the JIT will have to store a pointer from the JITed code to the SwitchPoint object, these pointers are not stored in the heap but in the metaspace (the native memory associated with the generated assembly codes) so it will bloat the metaspace. I don't want beautiful_logger to be that library that makes the application to run out of metaspaces given that people are not used to set the size of the metaspaces but rely on ergonomics (default values) instead.

One way to solve your issue is to use modules, because you can set a configuration module wise but it means that you are using Java 11 and that you are using modules, something you may not be able to do.

[1] see https://youtu.be/z5UkoLaW6ME?list=PLTbQvx84FrARa9pUtZYK7t_UfyGMCPOBn for more info

@pacien
Copy link
Author

pacien commented Apr 3, 2019

Sadly, modules are still a PITA to work with, using common amateur build tools and IDEs.

Alternatively, would it be possible to define some kind of LoggerGroup allowing Loggers to share a common configuration in an arbitrary manner, independently of packages or modules?

var group = new LoggerGroup();
var loggerA = Logger.getLogger(some.class, group);
group.update(opt -> opt.enable(false));

This would only require one additional SwitchPoint if I'm not mistaken.
The per-class configuration would still take precedence over such group configuration.

@forax
Copy link
Owner

forax commented Apr 3, 2019

Sadly, modules are still a PITA to work with, using common amateur build tools and IDEs.

It's not that bad, if you have one module per maven/gradle module, the main issue is that the dependencies has to be configured twice (in the build tool and in the module-info).

Your LoggerGroup is like a Consumer<ConfigOption> if your config is read-only

  Consumer<ConfigOption> group = opt -> opt.enable(false);
  var loggerA = Logger.getLogger(some.class, group);

Now, having another configuration stage like your LoggerGroup doesn't work that well because a Logger is stateless, Logger.getLogger(aClass, configConsumer is just a convenient method that calls Logger.getLogger(aClass) + LoggerConfig.fromClass(aClass).update(configConsumer) and each logger only lazily reads the different configurations (class, package; module) the first time it needs to log something.

That's said, you can write your own LoggerGroup that maintain a list of LoggerConfig and will propagate any modifications to all those logger configs.

  class LoggerGroup {
    private final ArrayList<LoggerConfig> configs = new ArrayList<>();

    public Logger createLogger(Class<?> type) {
      var logger = Logger.getLogger(type);
      configs.add(LoggerConfig.fromClass(type));
      return logger,
    }

   public void update(Consumer<? super ConfigOption> consumer) {
     configs.forEach(config -> config.update(consumer));
   }
  }

or add the logger config of the package in a set if you prefer.

@Maaartinus
Copy link

To avoid to test the configuration each time you log something (for perf reason), each logger is using the class java.lang.invoke.SwitchPoint [1] to detect when the configuration change, if you want to test recursive package, it means that you have to create as many switchpoints has the number of subpackages

Can't you use the trick Logback uses? IIRC, there's a single check per logger there, but what gets checked is propagated from parent packages. This means that you need to track all subpackages and propagate each LoggerConfig.fromPackage("ba", recursive=true).update(opt -> ...) to them. Configurations added after a recursive superpackage configuration update have to be handled appropriately.

@forax
Copy link
Owner

forax commented Apr 3, 2019

Currently there is no check at all that why beautifuf logger is faster than Logback, but it's may be possible to create more SwitchPoints but to only use the closest one when logging and the do the propagation from the parents when invalidating, this may require more synchronization but not when logging.

I need to think a little bit more about that :)

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

No branches or pull requests

3 participants