[tempo-distributed] Add distributor OTLP receiver advanced config

[ben-natan]

This adds the possibility to configure the distributor's OTLP receiver with advanced options. My initial need for this comes from this issue: grafana/tempo#860.

The advanced configuration can be found here

Signed-off-by: Adrien Bennatan adrien.ben-natan@ensta-paris.fr

[joe-elliott]

I like this addition, thanks! However, it feels like the config belongs under the traces: block where the other receiver config is. Can we move this additional config there?

traces:
  otlp:
    http: false
    grpc: false

I don't know a ton about helm, but it would be awesome if we could take either true or an object {} under traces.otlp.http. If true then just enable with the default endpoint. If an object then use the object directly as the receiver settings.

[ben-natan]

I don't know a ton about helm, but it would be awesome if we could take either true or an object {} under traces.otlp.http. If true then just enable with the default endpoint. If an object then use the object directly as the receiver settings.

I see 3 possibilities:

1. Take either true or an object under traces.otlp.http but I haven't found a way to do it without type checks and I don't think this is good practice
2. Add the config under distributor.config.receivers.otlp.protocols.http like I did in the PR at the time of opening it.
3. Introduce a breaking change and add the traces.otlp.http.enabled and traces.otlp.http.receiverConfig keys

Which do you think is best?

[joe-elliott]

I like option 3. Is it a breaking change? A user can specify:

traces:
  otlp:
    http:
      enabled: true

and we could default the receiverConfig to the existing one which is just the endpoint. They could also specify both if they wanted more detailed config:

traces:
  otlp:
    http:
      enabled: true
      receiverConfig: { custom config }

[ben-natan]

Is it a breaking change?

Users that currently have

traces:
  otlp:
    http: true

would need to adapt to the new way of enabling http.

[joe-elliott]

@ben-natan yikes, of course. Unsure how i missed that. I'm ok with the breaking change but let's up the minor rev to correctly signal it.

Thank you!

[joe-elliott]

This is looking really good @ben-natan. I'd like to request one more change if you have the time. Since this is a breaking change to move to this style of receiver configuration can we go ahead and do the opencensus, zipkin and jaeger recevier types. Otherwise we'll just find ourselves back in this position in a few months.

[joe-elliott]

Excellent work! Thanks @ben-natan