Extract ConfigProperties to a separate module so that it can be used in instrumentation-api

[mateuszrzeszutek]

Is your feature request related to a problem? Please describe.
We'd like to use ConfigProperties in the instrumentation code too, and get rid of the duplicate Config functionality. As it is now, ConfigProperties lives in the autoconfigure-spi module, which we cannot use in instrumentation-api because it has SDK dependencies.

Describe the solution you'd like
We could extract ConfigProperties to a separate opentelemetry-config module that does not depend on anything else; and use it both in the SDK autoconfigure and instrumentation-api (and all instrumentations too).

Describe alternatives you've considered
Moving ConfigProperties to opentelemetry-api is a no-go; we don't really want to have a duplicate Config either.

Additional context
Related to open-telemetry/opentelemetry-java-instrumentation#6202

[jkwatson]

If we remove ConfigProperties from the autoconfigure-spi module, is that a breaking change?

[mateuszrzeszutek]

Hmm, if we change the package name (which now contains words autoconfigure, sdk which might not suit the instrumentation use case) then, it would be a breaking change, unless we preserve the old interface and make a bridge implementation to the new one.

[anuraaga]

Yeah if we go ahead with the extraction we'd definitely need the bridging

[jack-berg]

Couple of options I see:

1. Split it to opentelemetry-config, keep the package name as is.
2. Copy it to opentelemetry-config, changing its package name to better reflect its intended use. Deprecate autoconfigure-spi version and have it extend the opentelemetry-config version.
3. Do nothing, agent uses autoconfigure-spi version.
4. Do nothing, agent defines duplicate Config.

I think option 2 is probably best long term. In addition to the agent being able to benefit from it, there are various places in the SDK that would benefit from reading from ConfigProperties, but don't because they can't take a dependency on autoconfigure-spi (e.g. DebugConfig). Downsides are maintaining a deprecated interface in autoconfigure-spi for a while, and having another artifact.

[jkwatson]

option 1 is not viable, since the Java Module System precludes 2 jars from containing classes in the same package (in the same classloader).

[mateuszrzeszutek]

Option 3 is also not viable -- Config lives in the bootstrap CL, and we cannot put references to the SDK classes there (and autoconfigure-spi has lots of these).

Downsides are maintaining a deprecated interface in autoconfigure-spi for a while, and having another artifact.

One more downside is that most stable SPIs (like AutoConfigurationCustomizer) will have to continue relying on the deprecated interface, and because of type erasure we can't really provide new non-deprecated methods unless we rename all the old one (so that their signatures change).

[trask]

we discussed in office hours and @jkwatson had a good thought that in the future we'll probably want a richer configuration data structure (e.g. for yaml-based config) that would cover both sdk and instrumentation configuration, and that would be a good time to introduce a new config API.

and so in the meantime, the current thought is not to touch the autoconfigure ConfigProperties.

I'll open a new issue in the instrumentation repo to discuss next steps there, but feel free to post here if you think we should still consider changes in autoconfigure ConfigProperties.

[mateuszrzeszutek]

we discussed in office hours and @jkwatson had a good thought that in the future we'll probably want a richer configuration data structure (e.g. for yaml-based config) that would cover both sdk and instrumentation configuration, and that would be a good time to introduce a new config API.

I had also thought of this, my idea was to rename getList/getMap to getStringList/getStringMap, and in the future copy some ideas from the typesafe config API and add a possibility to retrieve sub-configs and sub-config lists (getConfigList, getConfig). Meaning, we don't really need a completely new config API, I think that ConfigProperties can evolve into the new use case.

[mateuszrzeszutek]

I think this can be closed now; we're going to have a separate, internal (for now) agent-only config that simply forwards all calls to ConfigProperties.