tempo-operator

tempo-operator copied to clipboard

For example:

jaegerQuery:
  enabled: true
  ingress:
    enabled: true # new field
    type: ingress

Pros:

• consistency with other options, which have an enabled field for enabling/disabling the component

Cons:

• technically redundant, if the ingress type is set --> enable ingress, if it is not set --> disable ingress

Not sure about this, I mean, you have a point, enabled field is used across all other features

If we are going to introduce a field like this for ingress, then we should not allow empty ingress type IMHO. because ingress enabled=true and with type="" or None, means nothing.

The otel operator just depends on the type field too: https://github.com/open-telemetry/opentelemetry-operator/blob/ec0568fe1bc62217f12168864e039b4d05f27372/tests/e2e-openshift/route/00-install.yaml#L9

Not sure about this, I mean, you have a point, enabled field is used across all other features

If we are going to introduce a field like this for ingress, then we should not allow empty ingress type IMHO. because ingress enabled=true and with type="" or None, means nothing.

Yep, enabled=true and type="" could be prevented in the validating webhook.

The otel operator just depends on the type field too: https://github.com/open-telemetry/opentelemetry-operator/blob/ec0568fe1bc62217f12168864e039b4d05f27372/tests/e2e-openshift/route/00-install.yaml#L9

Yep, that's where I looked for best practices when implementing this feature. Thinking more about it, imho it's also inconsistent there, because it's not following the enabled: ... pattern.