Config Lifecycle Events

[radcortez]

As a user, I'm interested in being able to:

• Discover which Source was used to load my Config value
• Expand variables in my Config values
• Apply validations to my Config values

[radcortez]

Related issues:

• Test Deployed Configurations  #524
• Support variable expansion #405
• Enable users to determine 'winning' source for a value - where is it coming from? #312
• Null must be injectable #365
• CDI @ConfigProperty default value consistency should be enforced #475

[radcortez]

While all of these features could be implemented in one way or another, looking closing a lot of these operations match Config Lifecycle Events:

• Before Lookup in ConfigSource
• After Lookup in ConfigSource
• Before Value Conversion
• After Value Conversion

Maybe we can implement this API Lifecycle support, which will make the API more flexible to implement these and maybe other use cases.

[dmlloyd]

Since this issue is really about a possible way to solve the use cases, I've tagged it with Implementation.

[radcortez]

A very rough idea just to start the discussion :)

public interface ConfigLifecycle {
    void beforeLookup(ConfigSource configSource, String propertyName);

    void afterLookup(ConfigSource configSource, String propertyName, String value);

    <T> void beforeConversion(Class<T> type, Converter<T> converter, String value);

    <T> void afterConversion(T value);
}

[dmlloyd]

For property expansion, you'd need some means to replace the value. An interceptor-style approach works well in this case.

I'm not really sold that conversion and config source should be bundled up in this way (particularly if conversion gets hoisted off to its own API), given that they're different mechanisms. The only reason to have this grouping is because there is a general correlation to the "shape" of the API, but it's not really clear that there would be any actual practical benefit to putting them together.

[radcortez]

In that particular case, the Converter lifecycle events would be extracted to their own API (if that moves forward somehow).

Or, we could keep things more simple, and just have the beforeLookup and afterLookup. The problem with the Converters is that for property replacement, you do need to perform it before Conversion, no?

Probably we can have afterLookup to return a String and then you can do the property replacement over there.

[dmlloyd]

Right, property replacement is really best done at the config source level. In Quarkus we wrap the config source so it is essentially "interceptor style"; this works well.

Validation is best done at the converter level. In SmallRye Config, we wrap converters and this is a very effective solution.

A challenge with before/after callbacks is that the Config implementation has to coordinate them, which is fairly complex. What if the before callback wants to return an immediate value without calling subsequent steps, for example? Wrapping or interception is just far superior in this regard: simpler to set up, simpler to coordinate, and simpler for users to implement. The Config implementation calls the outermost one, and the user's implementation can return a value immediately, or it can delegate to the next phase, subsequently determining whether the value should be used as is or modified in some way (possibly by way of multiple subsequent delegations, as may be the case for property expansion).

This principle applies just as easily to conversion as it applies to config sources. Wrapping just works better.

[dmlloyd]

So for the config source case, I'd recommend an API like this:

public interface ConfigSourceInterceptorContext {
    String proceed(String key);
}

public interface ConfigSourceInterceptor {
    String getConfigValue(ConfigSourceInterceptorContext ctxt, String key);
}

The context is implemented by the MP Config implementation, and the interceptor is implemented by the user. The interceptor can apply any policy it likes and use the context to retrieve any number of additional values. It can also transform the key arbitrarily.

[dmlloyd]

If we determined to add things like information about the config source, that would go on the context API - the challenge though is that it may be to say what config source a property came from if it is composed of expressions whose expansions come from many sources!

[radcortez]

I guess it will depend on how you implement the lookup. If you go from outer to inner, composed expressions are going to be in the same context of the main one you are looking for, so you should get the main ConfigSource, plus all other ConfigSources that composed the original value.

[dmlloyd]

Yeah ultimately it seems like a solvable problem. The crux of it would be to return information about the source from that proceed method, so I guess it would look more like:

public interface ConfigSourceInterceptorContext {
    ConfigValue proceed(String key);
}

...where ConfigValue is a type that contains the key, value, and identifying information about the config source (maybe or maybe not the actual ConfigSource instance, since that might be an abstraction leak - especially if the ConfigSource is Closeable).

[radcortez]

Well, do we see any use case here for having the actual instance of ConfigSource? What are you going to do with it? So far, the only use case is for logging / info purposes I believe.

[radcortez]

You already got the key and value. We could just give you the Class of the ConfigSource that sourced the value. If you really want the instance, you could get it from Config.getConfigSources.

Just trying to figure out if there is any value to expose it there.

[dmlloyd]

Right it's mainly going to be for logging, and as I said I hope we don't have to expose the ConfigSource itself. But whatever the value type is, it should contain some useful semantic info, especially when it comes to error reporting.

For example I should be able to tell a user something like The value for configuration key "foo.bar" is not within the valid range of [1..10] at line 132 of file:///etc/your-config.properties or something along those lines - but only if that information is available. If the config property came from a database, maybe the error message would only be The value for configuration key "foo.bar" is not within the valid range of [1..10] in configuration database "baz".

[radcortez]

Hum... this is quite interesting.

In your last comment we are now mixing validation, right?

So, in the case a config exist, but is not valid (by whatever means we perform validation), should we just error it out, or search another ConfigSource that may have a valid value?

[dmlloyd]

Hum... this is quite interesting.

In your last comment we are now mixing validation, right?

I'm just thinking ahead: the main use case of knowing the original ConfigSource is essentially validation AFAICT.

So, in the case a config exist, but is not valid (by whatever means we perform validation), should we just error it out, or search another ConfigSource that may have a valid value?

For Quarkus we determined that if a user has made a configuration mistake, it's possible (maybe even likely) that starting the container anyway could result in problems - maybe catastrophic problems. Imagine if the system defaulted to accessing the production database, and the database name for the development database was misspelled in the user's configuration, causing the production database to be overwritten...

For this reason we decided that in this case the application should stop with a descriptive error message. The user should always have exact control over what they're doing, and we should prevent them from shooting themselves in the foot to the maximum practical extent.

[radcortez]

Understood.

Still on that particular sample, due to semantics, it may be impossible to determine if there is an issue or not, unless you add profiles into the mix (like Quarkus does) and compare them with each other.

Anyway, using this interceptor approach, I guess we to support it via ServiceLoader, and then allow developers to implement it as a regular CDI Interceptor?

[dmlloyd]

Anyway, using this interceptor approach, I guess we to support it via ServiceLoader...

Yeah seems doable, though maybe we want to do it in a "provider" style like ConfigSourceProvider?

...and then allow developers to implement it as a regular CDI Interceptor?

This I don't know about. What would it look like?

[radcortez]

Related to #125

Sorry, I probably used the wrong term there with CDI Interceptor. It couldn't be a CDI interceptor, since the API would be different.

But have a CDI way to create the Config Interceptor without using the Service Loader. This, if you rely only on CDI.

[dmlloyd]

Related to #125

Sorry, I probably used the wrong term there with CDI Interceptor. It couldn't be a CDI interceptor, since the API would be different.

But have a CDI way to create the Config Interceptor without using the Service Loader. This, if you rely only on CDI.

Ah! Yes, I think this it is a good/useful idea for CDI to be able to produce instances of this interface.

[svenhaag]

Right, property replacement is really best done at the config source level. In Quarkus we wrap the config source so it is essentially "interceptor style"; this works well.

Hi dmlloyd, I disagree. In our case we have multiple config sources each can contain variables or their actual values. I implemented a solution that collects all key/values whereas higher order config sources have precedence of course and than search for placeholders and replace them.
So for instance we have 2 YAML files application.yml and application-production.yml, the later usually contains the actual placeholder values whereas the former contains the placeholders aka variables to be expanded.
Cheers.

[svenhaag]

Hum... this is quite interesting.

In your last comment we are now mixing validation, right?

So, in the case a config exist, but is not valid (by whatever means we perform validation), should we just error it out, or search another ConfigSource that may have a valid value?

Hi radcortez, imho, if there is an invalid config (with higher prio) and a valid config with lower prio, it should fail.

[radcortez]

I implemented a solution that collects all key/values whereas higher order config sources have precedence of course and than search for placeholders and replace them.
So for instance we have 2 YAML files application.yml and application-production.yml, the later usually contains the actual placeholder values whereas the former contains the placeholders aka variables to be expanded.

I guess that you are looking to have something like this supported:
#470

[svenhaag]

I implemented a solution that collects all key/values whereas higher order config sources have precedence of course and than search for placeholders and replace them.
So for instance we have 2 YAML files application.yml and application-production.yml, the later usually contains the actual placeholder values whereas the former contains the placeholders aka variables to be expanded.

I guess that you are looking to have something like this supported:
#470

Indeed that's very interesting. However, I simply created my own ConfigSource with high order, read all other ConfigSource's (ConfigProvider.getConfig().getConfigSources()) and then expand variables on all the properties. So basically this ConfigSource contains the final truth (all replaced keys).

[radcortez]

Yes, the ideia here it to come up with a solution that supports that out of the box, so users don't have to implement it :)

[dmlloyd]

Right, property replacement is really best done at the config source level. In Quarkus we wrap the config source so it is essentially "interceptor style"; this works well.

Hi dmlloyd, I disagree. In our case we have multiple config sources each can contain variables or their actual values. I implemented a solution that collects all key/values whereas higher order config sources have precedence of course and than search for placeholders and replace them.

What I mean is "better done at the config source level than at the converter level", which was the context of the discussion (I probably could have been more clear). But what you're describing is a third option. There's a couple of problems with this idea that would make me conclude it's not the best general approach for the specification:

• All the configuration must be loaded into memory, even if it resides in a database or similar, and it could be large (MP Config places no upper bound on the maximum size of a configuration).
• Some configuration sources return values for keys that they don't enumerate in their names set (for example, Quarkus implements default values that way, and security vault sources sometimes do this as well to avoid enumeration of sensitive keys). The specification allows this.
• The specification as written today allows config sources whose values change over time (i.e. are mutable), which would be patently incompatible with this approach.
• It can cause substantial up-front (startup time) work to recursively do the resolution in a "big bang", especially for a large configuration.

The approach you describe can be useful for certain situations, but I believe it's not the best general approach for the specification for these reasons.

[Emily-Jiang]

After having processed all of the conversations, I think there can be treated as imlementation details (ConfigSourceInterceptorContext). I don't feel we need to expose such an spi as they are not bootstrapping and end users don't need to do anything with it. Anyway, I think based on the conversation on PR, this should be moved to smallRye to experiment and then come back.

[radcortez]

Yes, we are working on something in SmallRye.

Maybe we don't need to expose anything, but the idea was to have some way to allows users to implement / intercept the config lifecycle, so they can do logging, property replacement, composition etc...

[Emily-Jiang]

I think it is much better to provide property replacement, composition out of box.

[radcortez]

We can. These can just be a couple of interceptors that the API provides.

[radcortez]

For now, we will leave this for the implementation to handle.