Clarify namespace requirements for Instrumentation objects when auto-instrumenting #2147
Conversation
… same namespace as the auto-instrumented applications
|
|
|
hey ross, we actually have some documentation for this already: https://github.com/open-telemetry/opentelemetry-operator/blob/main/README.md?plain=1#L299-L303 |
| @@ -191,7 +191,7 @@ When using sidecar mode the OpenTelemetry collector container will have the envi | |||
|
|
|||
| The operator can inject and configure OpenTelemetry auto-instrumentation libraries. Currently Apache HTTPD, DotNet, Go, Java, NodeJS and Python are supported. | |||
|
|
|||
| To use auto-instrumentation, configure an `Instrumentation` resource with the configuration for the SDK and instrumentation. | |||
| To use auto-instrumentation, configure an `Instrumentation` resource with the configuration for the SDK and instrumentation. This resource must be created within the **same namespace** as the applications being auto-instrumented. | |||
There was a problem hiding this comment.
This is technically not true, since the application being instrumented can specify an Instrumentation object in another namespace. Later in this doc is this line:
`"my-other-namespace/my-instrumentation"` - name and namespace of `Instrumentation` CR instance in another namespace.
This is not the first time I've seen this topic come up tho. Instead of adding this line, can we make it clearer and easier to discover that applications can reference Instrumentation resources in other namespaces?
There was a problem hiding this comment.
I think the confusion comes from a combination of: the referenced line being hard to understand, there being no examples of it being used, and no mentions of the default behavior.
I see now that it would be used like this:
annotations:
instrumentation.opentelemetry.io/inject-java: "my-other-namespace/my-instrumentation"I've never seen an annotation used like this before (where it can be either a boolean or a string reference) which is why I didn't understand it at first, but that may be ignorance on my part.
Instead of adding this line, can we make it clearer and easier to discover that applications can reference Instrumentation resources in other namespaces?
I'm happy to make the changes if there are strong opinions on where/how this should be articulated. I think a combination of including different examples as well as a reference of the default behavior would make sense.
Let me know if that sounds good and I can make the change.
There was a problem hiding this comment.
@rossmcdonald yes please make the change.
There was a problem hiding this comment.
I'd like to contribute to this effort. I've conducted related automation tests on my fork, which indicate potential issues with this feature in certain scenarios. We're keen to address this at my company. I'm currently investigating the root cause and will determine if it's a bug or if improved documentation would suffice.
|
Closing this for now. I'll try to get back to this later. |
This PR adds a small clarification to the readme that
InstrumentationCRDs must be created in the same namespace as the auto-instrumented applications (as opposed to the namespace the operator or collector are running in).If they are not in the same namespace, users will see no changes made to their annotated deployments and the operator will generate the error:
Which makes sense, but is not immediately obvious how to fix if the users are not deeply familiar with Kubernetes and operators.