otlp.ExporterOption WithInsecure should take a boolean parameter.

[seanschade]

opentelemetry-go/exporters/otlp/options.go

Line 104 in bbc9465

func WithInsecure() ExporterOption {

The current design makes it harder to map this option to configuration. You need to check if the value was set, and then add the WithInsecure option. All of the other options take a parameter for the value. This is nice because you can just pass the value into the option.

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/exporter.md#configuration-options

If I define the configuration parameter using the default outlined in specification then I can't easily map this value to an ExporterOption.

// Whether to enable client transport security for the exporter's grpc or http connection.
Insecure bool `env:"OTEL_EXPORTER_OTLP_INSECURE,default=false"`

Current:

func (e *ExporterConfiguration) Insecure() otlp.ExporterOption {
	if e.OTLP.Insecure {
		return otlp.WithInsecure()
	}
	return nil
}

Proposed:

func (e *ExporterConfiguration) Insecure() otlp.ExporterOption {
	return otlp.WithInsecure(e.OTLP.Insecure)
}

The code change is minor, but it would be a breaking change.

// WithInsecure disables client transport security for the exporter's gRPC connection
// just like grpc.WithInsecure() https://pkg.go.dev/google.golang.org/grpc#WithInsecure
// does. Note, by default, client security is required unless WithInsecure is used.
func WithInsecure(insecure bool) ExporterOption {
	return func(cfg *config) {
		cfg.canDialInsecure = insecure
	}
}

[seanschade]

I realize that this option just mirrors the grpc.WithInsecure() option, but it also suffers from the same problem IMO.

https://pkg.go.dev/google.golang.org/grpc#WithInsecure

[Aneurysm9]

I'm definitely on board with ensuring options are easily managed through configuration and niladic options to set flags don't really fit well in that model. I wonder whether the consonance with the grpc.WithInsecure() option is enough to leave that interface as it is and perhaps add an additional interface. I could see something like this working:

func WithTLSVerify(verify bool) ExporterOption {
  return func(cfg *config) {
    cfg.canDialInsecure = !verify
  }
}

func WithInsecure() ExporterOption {
  return WithTLSVerify(false)
}

[pellared]

The OTLP exporters are already stable.

I am against adding more exported options until it is necessary.

This can be solved by something like

var opts []otlp.ExporterOption
if insecure {
  opts = append(opts, grpc.WithInsecure())
}