Otel declarative configuration

[tsloughter]

I can't figure out a good way to do this change. This is a mess. The issue is we already have file configuration, and worse, it is used layers deep (as in, things like span processors are passed configuration that is passed to tracer provider).

In this PR I have changed those components like span processor to work on structure similar to those in the otel declarative configuration. This isn't required but I felt that structure was better so wanted to use it. This means anyone calling those directory would have broken code. I could, and may, change it to convert the new config to match the old layout instead.

Previous configs people have are meant to continue working as this change will convert those to the new format. The issue there is we have to make a choice between which config we are using. So what it does is check if a key from the new config, like tracer_provider is found and then assumes new config and ignores the old config values.

As I said, a mess.

I'm opening this less to merge and more to discuss strategy.

[ferd]

Yeah looking at this, the approach seems to make sense. There's some mess around the mapping of old to new rules when things were not read consistently with the new format, but I don't know that you can get around that.

The only thing I can imagine making it make more sense is carefully flagging (via comments) which is stuff that can go away on incompatible deprecation vs. what needs to stay in until final deprecation.

[ferd]

if we don't expect dupes in C, then this can be Existing ++ (C -- Existing) and preserve ordering.

[ferd]

I do like having this more explicit split of internal/external config.

[ferd]

looks like a cool case of

lists:foldl(fun(F, OldC) -> F(OldC, Config) end,
            OldConfig,
            [fun convert_disabled/2,
             fun convert_attribute_limits/2,
             fun convert_resource/2,
             fun convert_propagator/2,
             fun convert_tracer_provider/2]).

[ferd]

This is a good and interesting format. Making it recursive (with fold) is also nice because it lets you do multiple deprecation cycles so long as they're in order (convert from 0.4 to 0.6 to 0.7, etc.) and support all of them in the order they happen.

[ferd]

are these fully gone? I guess they're Erlang-specific?

[tsloughter]

Na, they remain, this just should have been deleted instead of commented I think. Pretty sure the point here is just that those don't have to be renamed...

[tsloughter]

Yea, bsp_max_queue_size from the sys.config is already converted to max_queue_size.

[ferd]

oh yeah okay this bit of inconsistency creates some mess then.

[tsloughter]

Aside from the fact this is our 3rd iteration on file configuration I should also boil down where the inconsistency between our solution and the spec are with some psuedo code:

Here is what it looks like based on the spec and how ever other language (I think) implements this without an issue of backwards compatibility:

init() ->
  SDKModel = sdk_model:parse(File) 
  TracerProvider = tracer_provider_model:create(SDKModel.tracer_provider)
  ...
  return {Resource, TracerProvider}

%% in tracer_provider_model
create(Model) ->
  Processors = processors_model:create(Model.processors)
  Sampler = sampler_model:create(Model.sampler)
  Limits = limits_model:create(Model.limits)

  tracer_provider:create(Limits, Sampler, Processors)

Basically, they already have functions like tracer_provider:create/3 that take a processor and are calling that from the new (to the spec) model modules that are constructed based on the file configuration.

We however have no tracer_provider:create/3 that takes Processors as an argument because we actually start the span processors inside the tracer provider process itself, so instead of passing in the created processor we pass in the configuration. Changing that configuration structure breaks anything (should be only tests) that passes config to the tracer provider start function.

There may be no reason to not pass in the Pids of the span processors instead of the config of the span processors, but either way, it breaks those calls to start that pass config today.

Uh, so yea, a sort of mismatch with how we start everything at the leaves of a supervision tree and everyone else starts things iteratively, passing the created object to the thing that consumes it (new Processor, pass new Processor to new TracerProvider, etc).