Added OpenTelemetry instrumentation

[jpkrohling]

This PR adds OpenTelemetry instrumentation to the Jaeger Operator. It also allows the operator to auto-provision a Jaeger instance. A new sidecar was added to the deploy/operator.yaml, pointing to the Jaeger instance that will be provisioned by the operator. As the agent is resilient enough, all spans that are generated via the bootstrap are collected:

And this is how the reconciliation loop looks like for the auto-provisioned CR:

Closes #737.

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

[jpkrohling]

I built an image based on this branch: quay.io/jpkroehling/jaeger-operator:737-Instrument-Operator. To test this PR, just replace the image in the operator.yaml and deploy the operator as usual.

[jpkrohling]

I just ran this with OLM and could confirm that it works fine. During my first test, I didn't get the bootstrap trace, but if I kill the pod, the new pod does report a bootstrap trace. Here are the YAML files used to create the operator source/group/subscription:

---
apiVersion: operators.coreos.com/v1
kind: OperatorSource
metadata:
  name: jpkroehling-operators
  namespace: marketplace
spec:
  type: appregistry
  endpoint: https://quay.io/cnr
  registryNamespace: jpkroehling
---
apiVersion: operators.coreos.com/v1alpha2
kind: OperatorGroup
metadata:
  name: jpkroehling-operatorgroup
  namespace: marketplace
spec:
  targetNamespaces:
  - marketplace
---
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
  name: jpkroehling-subsription
  namespace: marketplace
spec:
  channel: stable
  name: jaeger
  source: jpkroehling-operators
  sourceNamespace: marketplace

And here's the CSV: https://paste.centos.org/view/c7b0cfba

[objectiser]

Partial review, will continue later.

[objectiser]

From a quick scan of the otel docs, couldn't find a standard definition for error attribute, so unclear whether should be a boolean type as in opentracing?

[jpkrohling]

Something for @pavolloffay perhaps.

[objectiser]

End of first pass. Looks good - couple of comments:

1. Wondering whether some/all of the operation names may need more scope - to make it clear what part of the code they relate to, e.g. upgrade.ManagedInstances rather than just ManagedInstances.
2. I think only the dependencies processing actually added extra attributes - because it was processing of a loop with multiple dependencies. Possibly not for this initial PR, but it might be useful having additional information about the data being passed to the functions, as attributes.

[jpkrohling]

Wondering whether some/all of the operation names may need more scope - to make it clear what part of the code they relate to, e.g. upgrade.ManagedInstances rather than just ManagedInstances.

Good point. Looking at the screenshots, I have the feeling that the context is indeed there, given that the parent spans do have upgrade in the name. On the other hand, the operation name is already big, and we can probably use slashes / to add more context. Like: upgrade/ManagedInstances, which yields an operation name like operator/bootstrap/upgrade/ManagedInstances.

I think only the dependencies processing actually added extra attributes - because it was processing of a loop with multiple dependencies. Possibly not for this initial PR, but it might be useful having additional information about the data being passed to the functions, as attributes.

+1, but I'd do that on an individual basis

[objectiser]

Only one minor change required on test operator.yaml.

[jpkrohling]

Merging, e2e passed locally.