avaje / avaje-config

Application configuration / properties loading for JVM applications
https://avaje.io/config
Apache License 2.0
55 stars 8 forks source link

Maven Plugin Feature (and some other features). #41

Closed agentgt closed 1 year ago

agentgt commented 1 year ago

Hey Rob,

Massive fan and like all your libraries.

I have been making a code generator (aka annotation processors) version of Spring's Petclinic to highlight various annotation processor libraries such as yours.

I wanted to use our own in-house config framework but I don't have the time to make it open source and accidentally stumbled on yours which has a similar initialization and design as mine. Mine has a general URI SPI mechanism, more filtering options, and the Config part is separated from the Key Value loading. There is also an annotation processor to map config to objects. It is probably massively over designed :).

Anyway here are some missing features I miss.

Feel free to let me know if anything is out of scope or not worthwhile or you want separate bugs filed for each.

Maven Plugin

One of the major features I'm missing is our config framework has a maven plugin very much analogous to the codehaus maven properties plugin.

<plugin>
 <groupId>com.snaphop</groupId>
 <artifactId>snaphop-config-maven-plugin</artifactId>
 <executions>
   <execution>
     <phase>initialize</phase>
     <goals>
       <goal>read-config</goal>
     </goals>
     <configuration>
       <urls>
         <url>file:///${project.basedir}/src/main/resources/db/database.properties</url>
       </urls>
       <!-- finds all properties with prefix and removes it -->
       <propertiesKeyPrefix>database.</propertiesKeyPrefix>
       <!-- prefix the found properties -->
       <pomKeyPrefix>my_project.database.</pomKeyPrefix>
     </configuration>
   </execution>
 </executions>
</plugin>
<!-- now use the properties for other plugins like jOOQ or Flyway -->

I think I can trivially port our maven plugin and we could add it as a module to avaje-config (well not yet as avaje-config is not multimodule but that would be easy to fix). If you are interested I could do a PR.

More configurable initialization

On the one hand I mostly like the current opinionated behavior for simplicity on the other hand it is not exactly the canonical behavior I want. For example I prefer ~/.config/application instead of ~/.localdev. Besides the config source SPI I think a worthwhile add on would be an SPI to provide your own initialization.

Annotation processor for config objects

Another thing our config framework has is a general purpose flat mapping of key values to objects.

@ConfigBean
public interface MyClientConfig {
  public String username();
  public int port();
}

What happens is an implementation is generated that essentially will map method calls like username to Map<String,String>#get or Config#getString. I take @Nullable into account.

Anyway this is again I think something I could port to use your library.

Able to load env like files AND generate them

One of the annoying things is not everything can read java like config. Particularly shell scripts.

One of the things we have is something that will generate shell environment config from your property files config. That is initialize the framework and export out an ".env" file. This requires deciding how you want to name mangle or convert property names to environment variable names.

Side note

As a tangent I will say on the dynamic properties front which we have as well we never use anymore because reload whole app is fast these days. I wonder do you use that feature?

SentryMan commented 1 year ago

Very nice, love to get new ideas

Maven Plugin

So gimme an example of what you'd use it for, I suppose I'm not immediately seeing how this would be used.

For example I prefer ~/.config/application

Have you tried using the load.properties feature?

load.properties is pretty versatile and can even be chained. For example, in your main application properties, you can have load.properties=application-${profile:local}.properties to load based on the profile environment variable/JVM arg, and in the application-whatever.properties you can add load.properties there and so on.

As a tangent I will say on the dynamic properties front which we have as well we never use anymore because reload whole app is fast these days. I wonder do you use that feature?

I've not used it, but you never know when it could come in handy. It's the closest thing to a dedicated event listener Avaje has. You can call Config.set anywhere and trigger a specific function somewhere else.

agentgt commented 1 year ago

So gimme an example of what you'd use it for, I suppose I'm not immediately seeing how this would be used.

oh my bad I thought I did. To configure other maven plugins like the jOOQ or flyway plugin.

As for load.properties there was some issue that I can’t recall why it was not enough.

I think what I really meant to mention was profiles but I suppose that you can use variables to dynamically pick load properties. Let me flesh that one out better. Mainly it was the maven plugin.

SentryMan commented 1 year ago

The maven plugin sounds pretty good to me. I wonder what Rob thinks.

agentgt commented 1 year ago

I've not used it, but you never know when it could come in handy. It's the closest thing to a dedicated event listener Avaje has. You can call Config.set anywhere and trigger a specific function somewhere else.

TL;DR Another feature would be an event capturing system instead of logging so that you can use the library to configure logging which is ironically the best use case of dynamic properties IMO.


I completely forgot another feature/bug and this one is a blocker for some of my companies code bases.

This library loads logging up before configuration is finished loading.

We use a custom SLF4J and System.Logger implementation in many places that might work with this config library because we queue the events fired off on System.Logger or JUL before slf4j loads and then replay them when slf4j actually loads (or if the system crashes). Also our custom logging implementation uses the config library directly instead of providing its own (like all other logging implementations which I could rant about but I will save that for another day).

However in some other places we use logback. The only way to configure logback with external configuration is to either programatically configure it via the Service Loader (yours truly added that to logback a long time ago) or to use System properties / ENV variables.

So what typically happens with configuration frameworks that use logging (and btw this was one of the main reasons I wrote our own config framework) is at best case the logging library does not see any of the config (assuming you set System properties with loaded config which our library has an option to do) or worse you can occasionally get weird class loading static initializing race conditions particularly if you go the programatic route and access the config framework while configuring logging. I believe with SLF4J this race condition is mostly fixed as it has its own internal queue but I think might break on System.Logger.

What our config framework provides is a simple event system. When logging is done you can drain the queue of events usually replaying them to logging our System.err.

SentryMan commented 1 year ago

This library loads logging up before configuration is finished loading.

ok, that sounds like something we should fix sooner rather than later. Create an issue for it if

might work

turns out to be false

agentgt commented 1 year ago

Yeah let me see if I can make a PR with a unit/integration test that fails.

Apparently it now more or less really depends on the backing implementation of System.Logger and SLF4J. I thought that SLF4J does some magic but I think with 2.0 now using the ServiceLoader it no longer does the whole queuing thing (for reentry of events while initializing) EDIT: it still does but it is kind of nasty. It returns a substitute logger.

I will say I know for sure OOB slf4j-simple does not work. It does not provide a queue and initializes eagerly. So if you naively make your own implementation based on slf4j-simple and call avaje config during initialization it will probably break.

What I will do is make a System.Logger integration test. I will have to make a couple of modules to do this because of the ServiceLoader. Unfortunately this will require restructure of the project to a multimodule maven project.

I'll try later this week and let you know.

JUL I'm not so sure about.

agentgt commented 1 year ago

I was wrong SLF4J still does have protection but System.Logger does not. The only way for catastrophic initialization to happen is if your System.Logger implementation is not reentrant. The OOB System.Logger one happens to be reentrant (also surprisingly I thought it uses JUL but it does not).

That being said if your logging system requires system properties to be set a priori then it will not work if the logging system is accessed first (which it will always be with avaje-config).

The only way to fix that really is to be able to control the initialization (or at least hook into the beginning and end) of avaje-config. This is because avaje-config follows the ServiceLocator pattern like most logging frameworks but unlike most logging frameworks its not open for changing.

Even microprofile config has this option (through SPI and ditto for SLF4J) so I think its worthwhile.

Finally the big reason for me currently is I need the config loading to not produce any output for various tools.

rbygrave commented 1 year ago

What our config framework provides is a simple event system. When logging is done you can drain the queue of events usually replaying them to logging our System.err.

I think we should do this for avaje-config. I've created https://github.com/avaje/avaje-config/issues/47 for this.

PR with a unit/integration test that fails.

For myself, I don't need to see a failing integration test for this.

When logging is done you can drain the queue of events usually replaying them to logging our System.err.

Sounds to me like a "Lifecycle" listener + maybe built in options for ease of use (log to System.err, log to System.Logger, log to <nothing / ignore> )

SentryMan commented 1 year ago

Annotation processor for config objects

You still want this?

agentgt commented 1 year ago

Annotation processor for config objects

You still want this?

Object mapping from config is a broad and complicated topic. I'm still mulling it over what exactly is a minimal solution that eases most of the user pain.

While I made an annotation processor to map config it feels like a better tool would be a generic one like MapStruct but they still haven't really finished Map<String,String> / Function<String,String>. Also mine is pretty simple flat mapping and does not do composition or embedded objects.

What I'll do sometime this week or beginning next is port both maven and annotation processor of our config framework in sort of modules avaje-config project.

As a side note it would be nice if avaje-config was already a multimodule project so that I could just put the modules in a forked version of this project (I suppose I could do it on the fork but I didn't want to muddy the changes of adding modules with project restructuring).

SentryMan commented 1 year ago

What I'll do sometime this week or beginning next is port both maven and annotation processor of our config framework in sort of modules avaje-config project.

Neat.

As a side note it would be nice if avaje-config was already a multimodule project

Wait it's already a multimodule maven project though?

agentgt commented 1 year ago

Wait it's already a multimodule maven project though?

Apparently so. I have no idea how I was mistaken. Maybe an older version I had up when I looked?

SentryMan commented 1 year ago

Anything else we wanna add here? this has been open for a while.

agentgt commented 1 year ago

I can't recall if interpolation still only works with System properties? I remember seeing that in the code it being a deal breaker for me.

For example I often have a database properties file

database.schema=foobar
database.host=localhost
database.portPrefix=${global.portPrefix}
database.port=${global.portPrefix}5432
database.url=jdbc\:postgresql\://${database.host}\:${database.port}/${database.schema}
database.username=${database.schema}
database.password=${database.schema} # lets ignore if that is a good idea or not

The above generally requires recursive lookup of properties or multiple passes. I will test if avaje-config can do that now but IIRC it could not.

Pinging @rbygrave as well.

SentryMan commented 1 year ago

I can't recall if interpolation still only works with System properties? I remember seeing that in the code it being a deal breaker for me.

interpolation works for any kind of property, ENV variables, files loaded with load.properties, custom configuration sources, and yes, system properties.

SentryMan commented 1 year ago
database.schema=foobar
database.host=localhost
database.portPrefix=${global.portPrefix}
database.port=${global.portPrefix}5432
database.url=jdbc\:postgresql\://${database.host}\:${database.port}/${database.schema}
database.username=${database.schema}
database.password=${database.schema} 

oh, I guess this doesn't work. thanks for finding this.

rbygrave commented 1 year ago

What does not work? The global are not defined so I don't understand that part of the example. But interpolation includes other properties yes.

SentryMan commented 1 year ago

suppose you add global.portPrefix it would still not work, I have raised a PR to resolve

SentryMan commented 1 year ago

What does not work?

consider this:

database.schema=foobar
database.host=localhost
global.portPrefix=sus
database.portPrefix=${global.portPrefix}
database.port=${global.portPrefix}5432
database.url=jdbc\:postgresql\://${database.host}\:${database.port}/${database.schema}
database.username=${database.schema}
database.password=${database.schema

Currently Config.get("database.portPrefix") throws property not found.

rbygrave commented 1 year ago

The above generally requires recursive lookup of properties or multiple passes. I will test if avaje-config can do that now but IIRC it could not.

Fixed via https://github.com/avaje/avaje-config/pull/82 ... via the eval function using recursion which is being released in version 3.6

agentgt commented 1 year ago

I can file this as a separate feature but it would be nice if the Yaml parsing was ServiceLoaded up.

Then for the folks that already depend on yaml you can make the minor releases just depend on that module:

SentryMan commented 1 year ago

not sure if we'll get any particular use out of splitting, but I like the serviceloader idea

agentgt commented 1 year ago

Well I don't know for sure what the uptake would be like but I see a differentiator of avaje as being as small as possible.

Of course I'm a little bit biased as I avoid YAML like the plague if I can but I realize it some how mostly won the devops configuration format war so I'm not that against it being for server backend but I might use avaje config for command line utils.

rob-bygrave commented 1 year ago

avaje-config-core (original avaje-config minus yaml)

The simple yaml parser that avaje-config includes is approx 300 lines of code. It's not a LOT of code imo. Pulling that out into a separate module doesn't seem justified to me.

agentgt commented 1 year ago

The simple yaml parser that avaje-config includes is approx 300 lines of code. It's not a LOT of code imo. Pulling that out into a separate module doesn't seem justified to me.

I agree on that front. It was more of the dependency but looking at the POM it is optional and for some reason I thought it was not. I'll have to look how you safely load YAML Snake (the safest btw would be to still use the Service Loader).

I will say there are some in the camp (several from google) that believe there is no such thing as an optional dependency or that it is bad.

However from a practical sense I'm fine with it if you can graalvm native compile as well as jpackage without needing the jar.

... oh wait... after looking at the code it appears you have your own builtin parser? If that is true completely disregard that feature request!

agentgt commented 1 year ago

Damn @rbygrave aka @rob-bygrave that YamlLoaderSimple... I might have to borrow it. I love it.

agentgt commented 1 year ago

@SentryMan and @rbygrave I don't have any more feature ideas :) . I will add my companies custom maven plugin as well as the annotation processor that maps bean interfaces to config as a separate project that builds on top of avaje-config and then you can decide what you guys like from it.

Of course @SentryMan knows by the time I get to that it will be 2030 😄

Anyway we can probably close this issue or move it to discussions. Thank you guys so much for the back and forth!

rbygrave commented 1 year ago

YamlLoaderSimple... I might have to borrow it. I love it

Yup, borrow away. The SnakeYaml parser is quite a lot bigger size wise and at the time of creating YamlLoaderSimple the SnakeYaml parser had some security vulnerabilities that they were sitting on a bit.

YamlLoaderSimple does have the limitation that it does not support collections (because in my mind we are translating it all into name/value properties pairs under the hood and collections don't quite map to that).