-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instrumentation support selector #2744
Comments
This approach has one considerable downside over the annotation. The annotation changes pod spec, therefore it triggers re-deployment which is needed by the OTEL operator to inject the auto-instrumentation (the operator uses pod mutating webhook) |
Yeah, you're right. However, we hope that all the configurations associated with OTel can be handed over to Operator, and if the annotation are used, we also need to maintain them ourselves during the |
This is our usage scenario; I'll submit a PR if appropriate. apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
name: my-instrumentation
spec:
selector:
app: appname
propagators:
- tracecontext
java:
image: image |
@crossoverJie we discussed this issue at the SIG (and realized we should have probably done this prior to your PR) and had a few follow up questions. I saw the link you shared for how skywalking handles this, and at first glanced it seemed very similar, however, there's a key difference. Skywalking seems to be a java-only auto-instrumentation solution whereas opentelemetry can inject instrumentation for multiple languages. That results in what we agreed to be a confusing experience for users where a cluster admin needs to set a piece of configuration AND a tenant in the cluster needs to set configuration as well.
I was imagining it would be more beneficial for both personas if the cluster admin could specify a set of rules that specify a selector for applications to target for injection and specify which languages (and optionally container names) to inject. Let me know your thoughts here, and if you'd like to meet more to discuss the above I'd be happy to chat. |
Thank you for following up on this issue.
Can you explain the relationship between cluster admin and tenants here?
apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
name: instrumentation-sit-grpc-consumer
namespace: sit
spec:
env:
- name: OTEL_SERVICE_NAME
value: grpc-consumer
selector:
matchLabels:
app: grpc-consumer
java:
image: autoinstrumentation-java:v1
extensions:
- image: extensions:v1
dir: /extensions
env:
- name: OTEL_RESOURCE_ATTRIBUTES
value: service.name=grpc-consumer
---
apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
name: instrumentation-sit-grpc-provider
namespace: sit
spec:
env:
- name: OTEL_SERVICE_NAME
value: grpc-provider
selector:
matchLabels:
app: grpc-provider
java:
image: autoinstrumentation-java:v1
extensions:
- image: extensions:v2
dir: /extensions
env:
- name: OTEL_RESOURCE_ATTRIBUTES
value: service.name=grpc-provider This is our usage scenario, and there are some application-specific configurations that need to be defined in different There are some other examples:
Therefore we like that each application can maintain its own |
Thanks for adding more context! Given that you want each application to maintain its own I still think the broader use case you describe is valuable, however, and it would be much simpler if the instrumentation selected its applications too. I think that instrumentation selector only works (or works best) when the user doesn't need to modify their application whatsoever to do so. |
Thanks for your reply, the following issues may require further discussion.
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: order
name: order
spec:
template:
metadata:
annotations:
instrumentation.opentelemetry.io/container-names: "order"
instrumentation.opentelemetry.io/inject-java: "true"
# associate instrumentation
instrumentation.opentelemetry.io/instrumentation-name: "instrumentation-prod"
...
----
apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
name: instrumentation-prod
namespace: prod
... Do you mean to associate
I don't understand clearly here. Similar to my last comment, in the current time we will create an Is there any other way to configure these environment variables?
Not sure I understand what you mean. |
the example you linked demonstrates what i was referring to i.e. creating an instrumentation object and then referencing it directly via its name in the application.
There is a need to modify the application with at least the following in the world where we applied a selector as your PR does today. per your example:
What i'm thinking is a world where a cluster admin like yourself would define an instrumentation like so:
This would then inject configuration for the following deployments without requiring any modifications in the deployment:
This to me would be closer to what skywalking can accomplish today and would allow cluster admins to entirely separate injection from application developers. Let me know what you think, thank you! |
@jaronoff97 Sorry for the delayed response.
Using a new CRD: One more question: if we use |
@crossoverJie this is a good question, I think before we can answer it we should have a design in mind for InstrumentationRules to see if it's a superset or distinct (there are tradeoffs for either approach). Would you be able to work on that design? If not, I can see if anyone else is available to do so. (my cmd+enter pressed closed... oops!) |
Hahaha, no worries.😂
I will draft a proposal for |
apiVersion: opentelemetry.io/v1alpha1
kind: InstrumentationRules
metadata:
name: instrumentation-prod
# It will only select the Pods in this namespace.
namespace: prod
spec:
# If the rule is not specified, the exporter/propagators/sampler will be used
exporter:
endpoint: http://global-otel-collector:4317
propagators:
- tracecontext
- baggage
- b3
sampler:
type: parentbased_traceidratio
argument: "0.35"
rules:
- name: order-service
java:
image: ""
extensions:
- image: ""
dir: /extensions
exporter:
endpoint: http://prod-otel-collector:4317
propagators:
- tracecontext
- baggage
- b3
sampler:
type: parentbased_traceidratio
argument: "0.25"
env:
- name: OTEL_EXPORTER_OTLP_PROTOCOL
value: grpc
- name: OTEL_SERVICE_NAME
value: order-service
# Multi-container pods with single instrumentation
container-names: [order] # If there are multiple containers in the pod, specify the container name here, otherwise it will be use the first container in the pod.
selector:
matchLabels:
app: order
- name: user-service
# Multi-container pods with multiple instrumentations
java:
env: []
container-names: [user-java]
python:
env: []
container-names: [user-python]
selector:
matchLabels:
app: user The purpose of this proposal is to flexibly configure Here, a resource named Additionally, there are a few issues that need to be confirmed:
This is just a draft, and I look forward to your reply.@jaronoff97 |
apiVersion: opentelemetry.io/v1alpha1
kind: InstrumentationRules
metadata:
name: instrumentation-rule-for-namespace
# It will only select the Pods in this namespace.
namespace: prod
spec:
# If usedForNamespace is true, the rule will be used for the namespace.
usedForNamespace: true
# If the rule is not specified, the exporter/propagators/sampler will be used
exporter:
endpoint: http://global-otel-collector:4317
propagators:
- tracecontext
- baggage
- b3
sampler:
type: parentbased_traceidratio
argument: "0.35"
rules:
# Only one rule is allowed for given namespace.
- name: used-for-namespace
java:
env: []
python:
env: []
For this issue, I suggest adding a new field called If there are ohter @jaronoff97 Do you have any other suggestions? If not, I'd like to start developing this feature as soon as possible. |
cc @swiatekm @iblancasa #2744 (comment) #2744 (comment) Here is a new proposal regarding select, If you have time, please take a look. |
@crossoverJie thanks for working on this! I think it would be great if you could write an RFC for this given the functionality will be net new and there will be some edge cases and rollout issues I want to be sure we all understand and agree upon. I can reach out via slack and we can work on it. I'm going to be creating an RFC process and this feels like a great first RFC to write. If you wanted to start on an implementation so we can have something as a proof of concept, that would be great too. |
Component(s)
auto-instrumentation
Is your feature request related to a problem? Please describe.
Instrumentation
applies to the specified Pod, similar to this:https://skywalking.apache.org/docs/skywalking-swck/latest/java-agent-injector/#1-label-selector-and-container-matcher
Describe the solution you'd like
Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: