Allow vendor distribution to provide default configuration

[mateuszrzeszutek]

Resolves #1213

As an additional benefit, now all SDK ConfigBuilders can use properties from both SPI and the otel.trace.config file (which they did not use before).

Needs to be merged after #1254

[trask]

Hey @mateuszrzeszutek!

What do you think of turning Config into just a POJO (e.g. @AutoValue)? (e.g. still with static get() method to get the singleton instance)

And moving the construction of the Config object into javaagent-tooling?

Then wouldn't need to reference agent class loader from bootstrap class loader, which is where the complexity enters.

One other thought, it looks like the ExporterConfig arg to SpanExporterFactory.fromConfig(ExporterConfig) is basically unused, and if we replaced that with a Properties object (we can combine propertiesFromConfigFile and propertiesFromSpi before passing in), then the exporter modules wouldn't need to depend on instrumentation-api and javaagent-tooling which would be nice.

[iNikem]

One other thought, it looks like the ExporterConfig arg to SpanExporterFactory.fromConfig(ExporterConfig) is basically unused, and if we replaced that with a Properties object (we can combine propertiesFromConfigFile and propertiesFromSpi before passing in), then the exporter modules wouldn't need to depend on instrumentation-api and javaagent-tooling which would be nice.

If we pass just Properties into SpanExporterFactory, then we have to normalise keys there, right?

[mateuszrzeszutek]

What do you think of turning Config into just a POJO (e.g. @AutoValue)? (e.g. still with static get() method to get the singleton instance)
And moving the construction of the Config object into javaagent-tooling?

That's a great idea!

One other thought, it looks like the ExporterConfig arg to SpanExporterFactory.fromConfig(ExporterConfig) is basically unused, and if we replaced that with a Properties object (we can combine propertiesFromConfigFile and propertiesFromSpi before passing in), then the exporter modules wouldn't need to depend on instrumentation-api and javaagent-tooling which would be nice.

Hmm, one downside of this approach would be duplicating the configuration loading order in each exporter: merged-spi-and-file-props -> env variables -> system properties.
Alternatively we could probably convert env var names (as @iNikem mentioned) to system property names and merge them all together into a single Properties instance.

Anyway, that seems like a lot of changes -- do you think that we should do it in this PR?

[iNikem]

A, io.opentelemetry.sdk.common.export.ConfigBuilder already does that... Yes, I think it is a very good idea, to move actual compilation of configuration from many difference sources into a separate class in tooling module and to expose simple Config/Properties to other components.

[iNikem]

Hmm, one downside of this approach would be duplicating the configuration loading order in each exporter: merged-spi-and-file-props -> env variables -> system properties.
Alternatively we could probably convert env var names (as @iNikem mentioned) to system property names and merge them all together into a single Properties instance.

You read configuration once, normalize it (or more precise, you let ConfigBuilder do it for you) and then pass single instance of already normalised Properties to all other components.

[mateuszrzeszutek]

Nice, reusing SDK ConfigBuilder for this is a neat idea.

[mateuszrzeszutek]

I've pushed a PR that contains all those refactorings: #1254
I'll rebase this one with it.

[mateuszrzeszutek]

Rebased after merging the refactoring PR.

@trask @iNikem I'm just letting you know that this one is ready for review.

[anuraaga]

Sweet!