Yaml configuration support

[re-thc]

Add optional yaml support for config please

[pmwmedia]

I prefer properties files as a primary and official configuration of tinylog. Multiply ways of configuration will make it much more difficult to keep the documentation of tinylog up-to-date and complete for all configuration options.

However, I would accept a third party project and I can imagine to make the configuration of tinylog pluggable to make it easy to develop such configuration extensions as your requested YAML support.

[Git5000]

I agree with pmwmedia that there is little need for non property configuration. However, since pmwmedia suggested a pluggable service for configuration - and I ran out of own issues to implement ... :-) I had a quick look and hack at a service loader for the Configuration. It turns out that with 10-20 LOC of code you can load other file formats when the property loader is turned into a service. As test I could load a JSON property file with another 10 LOC in a different project. I don't know YAML so well but if there is a library which can read it it should be similarly easy.

See the next comment for my suggestion for an implementation. Please have a look at it and see whether this would be useful too have or what other approach would be preferable.

[Git5000]

Introduce a new SERVICE:
org.tinylog.configuration.PropertyLoader

for the new interface

public interface PropertyLoader {
	public Properties load()  throws Exception;
}

Here then an implementation of the standard tiny log propertiy loader as it was before in Configuration:

public class StandardPropertyLoader implements PropertyLoader {
	public Properties load() {
   ... (all exactly copied from Configuration)
  }
}

the main change then in the load method of Configuration so that it calls the new services instead of loading properties directly:

public final class Configuration {
	public static Properties load() {
		ServiceLoader<PropertyLoader>  serviceLoader = new ServiceLoader<PropertyLoader>(PropertyLoader.class);
		Properties result = new Properties();
		Collection<PropertyLoader> loaders = serviceLoader.createAll();
		for (PropertyLoader loader:loaders)	{
			try {
				Properties props = loader.load();
				if (props != null && props.size() != 0) {
					result.putAll(props);
				}
			}
			catch(Exception e) {
			}
		}
		return result;
	}
}

To make it easier for other loaders we should have a bit more public access to some former package elements:
EnvironmentVariableResolver -> goes public
SystemPropertyResolver -> goes public
Configuration:static String resolve(...) -> goes public

That was all needed to get another loader running.

[Git5000]

There is one conceptual issue:

Since one can not really configure the configuration itself (except with system properties or so but you don't really want to enforce that) one cannot really determine which property loader to use.

• So the easiest is to just load all and get the proprties from all and just merge them all together. In practise there should only be one file format present and all non used loaders will not find their files and return nothing anyway
• Give some priority to the property loader services (e.g. public int priority();} and load the highest one which returns values.
• Do introduce some configuration of the configuration after all. By a system property or another static element somewhere

I think the first is sufficient though. All others are complicated with not so much gain.

[pmwmedia]

@Git5000 I really like your concept! It is very flexible and generic.

Personally, I prefer a priority property for determining which PropertyLoader comes first.

[Git5000]

Ok, sounds good. Just a few more questions:

1. With "priority property" you mean an entry inside the read property map e.g. "configuration.priority"="1000"?
2. Lower or higher priorities shall be preferred?
3. Maybe also additionally and optional a system property "tinylog.configurationLoader"="StandardPropertyLoader" (which matched the loader's class name overriding any priorities)

[pmwmedia]

With "priority property" you mean an entry inside the read property map e.g. "configuration.priority"="1000"?

Actually, I thought about a priority getter in the PropertyLoader interface. Firstly, the PropertyLoader implementation with the highest priority tries to load a configuration. If it couldn't find any, the next PropertyLoader can try to load a configuration and so on ...
If the system property tinylog.configuration=*.* is set, the PropertyLoader implementations have to check, if they can load the passed configuration file.

Lower or higher priorities shall be preferred?

Higher numbers first (same as thread priorities in Java)

Maybe also additionally and optional a system property "tinylog.configurationLoader"="StandardPropertyLoader"

Good idea :)

PS: Maybe ConfigurationLoader is a clearer interface name than PropertyLoader?

[Git5000]

Actually, I thought about a priority getter in the PropertyLoader interface.

Yes I thought this first too (in my initial suggestion). I understood your reply then as a property inside the files. This I actually liked better than my first idea. The configuration is then on file level not code level. So you load all files and keep the one with the highest priority. A disadvantage is of course that you load all files (but in practise this is one or two only anyway). Advantage is that you can set this property independently of the code inside the actual files.

Of course, in practise I cannot really see that someone will really load properties from JSON and YAML and Java properties. So you should only have one loader doing any loading anyway.

configuration.priority=1000

writerA=console
writerA.tag=TAG1
writerA.format={date}: [{level}]: PROP {message}
writerA.level=INFO

If you have a priority getter in the code the use case is probably only to switch off the tinylog configuration itself and use your own (single) loader. Then maybe a boolean getter would suffice? But of course in case we miss a use case this takes away flexibility.

So what should we do? :-)

Higher numbers first (same as thread priorities in Java)

ok

Maybe ConfigurationLoader is a clearer interface name than PropertyLoader?

Ok

[pmwmedia]

@Git5000 Sorry for my delayed responses. Currently, I'm on vacations at the Baltic Sea.

Mainly, I'm working on Java EE and JSF projects in my job. In this context, I'm used to call getters (+ setters) as properties. Sorry for the confusion!

My main goal is to keep the tinylog code and configuration files as simple as possible. If the priority has to be configured directly in the configuration files, what will happen if there are multiple configuration files but the user hasn't set any priorities in these files? In the latest tinylog version, it is already possible to define priorities via the configuration file names (tinylog-dev.properies, tinylog-test.properties, and tinylog.properties).

Actually, I see loading of merged configuration as confusing and I can't imagine a real world use case for it. I prefer defining the priority via a getter or always prefer a custom ConfigurationLoader implementation to the in-built ConfigurationLoader implementation for Java properties files.

[Git5000]

Don't worry about a delayed response. I am actually amazed about the speed of your responses. And you certainly should enjoy your holiday :-)

My main goal is to keep the tinylog code and configuration files as simple as possible.

I fullly agree. That the library is simple, fast, well designed but yet capable is the main reason I really like and use it.

Actually, I see loading of merged configuration as confusing and I can't imagine a real world use case for it. I prefer defining the priority via a getter or always prefer a custom ConfigurationLoader implementation to the in-built ConfigurationLoader implementation for Java properties files.

You right that it should not be confusing and kept simple.

Generally, the discussion here in the forum is very useful and I think we always iterate to a good solution.

I played around a bit with the lastest ideas and I think the best solution is indeed to always prefer a custom loader (i.e. no additional property or method in the interface). With having a system property to select the loader, there is still all flexibility to select any loaders you would want, internal or custom. The loader code becomes small and good too.

The code then would looks like:

                String loaderClassName = System.getProperty(CONFIGURATION_LOADER_CLASS_PROPERTY); //  == "tinylog.configurationLoader"
		
		// Select which configuration loader to use 
		ConfigurationLoader activeLoader = new PropertyConfigurationLoader();
		boolean hasCustomLoader = false;
		for (ConfigurationLoader loader : loaders) {
                        // Check system property to override loader
			if (loaderClassName != null && !loader.getClass().getSimpleName().equals(loaderClassName)) {
				continue;
			}

			if (hasCustomLoader) {
				InternalLogger.log(Level.ERROR, "More than one active configuration loaders found. Ignoring it.");
				continue;
			}
			activeLoader = loader;
			hasCustomLoader = true;
		}
		
		// Load the properties from the active configuration loader
		try {
			Properties activeProps = activeLoader.load();
			if (activeProps != null) {
					return activeProps;
			} else {
				return new Properties();
			}
		} catch (Exception ex) {
			InternalLogger.log(Level.ERROR, "Configuration loader error: '" + ex + "'");
			return new Properties();
		}

You think that is ok?

Is the property name "tinylog.configurationLoader" ok?

[Git5000]

The concept works actually very nicely :-)

• Choose a loader (use one of the following lines to get the loader you want, assuming they are all registered as service (otherwise you don't even need the system property):

  	a) System.setProperty("tinylog.configurationLoader", PropertyConfigurationLoader.class.getSimpleName());
	b) System.setProperty("tinylog.configurationLoader", JsonPropertyLoader.class.getSimpleName());
	c) System.setProperty("tinylog.configurationLoader", YamlPropertyLoader.class.getSimpleName());

• The loaders load e.g. one of the following files:

  a) tinylog.properties
  b) tinylog.json
  c) tinylog.yaml

• Having a YAML input file looks actually pretty:

  writerA: 
     type: "console"
     level: "INFO"
     tag: "TAG1"
     format: "{date}: [{level}]: YAML {message}"
     stream: "out"
  writerB: 
     type: "console"
     level: "WARN"
     tag: "TAG1"
     format: "{date}: [{level}]: YAML {message}"
     stream: "out"
  extra: "test"

• YamlPropertyLoader needs only 15 LLOC (real code lines) using the snake YAML reader library

PS: I can later on make a small repository on my Github page with the Tinylog extra stuff where I can upload these loaders as example.

[pmwmedia]

Generally, the discussion here in the forum is very useful and I think we always iterate to a good solution.

I fully agree 👍 Discussing ideas together is a very efficient way to find the best solutions in an iterative way. I'm very glad about your great contribution to tinylog :)

Is the property name "tinylog.configurationLoader" ok?

Yes, I would choose the same property name. I just prefer to use a lower case l for consistency with other properties. Currently, all properties have only lower case letters, even combined words like "autoshutdown" or "writingthread".

Do you use tinylog's service loader implementation org.tinylog.configuration.ServiceLoader? If the class names are PropertiesConfigurationLoader, JsonConfigurationLoader, and YamlConfigurationLoader, even tinylog.configurationloader = properties, tinylog.configurationloader = json, and tinylog.configurationloader = yaml will work.

Your YAML example looks very straight forward and clearer than a properties file. I like it :)

I can later on make a small repository on my Github page with the Tinylog extra stuff where I can upload these loaders as example.

Sounds very great :) I would announce your repository on tinylog's news site and link it in the documentation.

[Git5000]

Yes I use the service loader. Interesting option to load them also the short way. Currently, it does not work but I think I named the classes wrong. I will change that and since you seem ok with the overall changes I will slowly prepare a pull request for review. I suppose there will be a few iterations on the code too then.