Skip to content
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

Open
crossoverJie opened this issue Mar 11, 2024 · 5 comments · May be fixed by #2778
Open

Instrumentation support selector #2744

crossoverJie opened this issue Mar 11, 2024 · 5 comments · May be fixed by #2778
Labels
area:auto-instrumentation Issues for auto-instrumentation enhancement New feature or request

Comments

@crossoverJie
Copy link
Contributor

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

apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
  name: my-instrumentation
spec:
  selector:
    app: appname

Describe alternatives you've considered

No response

Additional context

No response

@crossoverJie crossoverJie added enhancement New feature or request needs triage labels Mar 11, 2024
@pavolloffay pavolloffay added area:auto-instrumentation Issues for auto-instrumentation and removed needs triage labels Mar 11, 2024
@pavolloffay
Copy link
Member

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)

@crossoverJie
Copy link
Contributor Author

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 deployment.

@crossoverJie
Copy link
Contributor Author

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 crossoverJie linked a pull request Mar 22, 2024 that will close this issue
@jaronoff97 jaronoff97 added the discuss-at-sig This issue or PR should be discussed at the next SIG meeting label May 23, 2024
@jaronoff97
Copy link
Contributor

@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.

  1. What are the user stories for this request and how do they differ from today? With the change you've made, it seems to be a very similar end state where both cluster admin and tenant need to do actions which feels against the goal of the issue which is to put the onus of instrumentation solely in the hands of the cluster admin.

  2. What are the reasons for having multiple instrumentation resources for the same language in the same cluster? I'm not sure I understand the exact issue today that the current solution solves.

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.

@jaronoff97 jaronoff97 removed the discuss-at-sig This issue or PR should be discussed at the next SIG meeting label May 23, 2024
@crossoverJie
Copy link
Contributor Author

@jaronoff97

Thank you for following up on this issue.

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.

Can you explain the relationship between cluster admin and tenants here?
In my scenario, I'm the only one maintaining Instrumentation.

2. What are the reasons for having multiple instrumentation resources for the same language in the same cluster? I'm not sure I understand the exact issue today that the current solution solves.

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 Instrumentations.

There are some other examples:

  • New versions of javaagent are only tested on specific applications first.
  • Some specific applications only need to report metrics but do not want to report traces.
  • For resource isolation, the OTEL_EXPORTER_OTLP_ENDPOINT reported by some applications may be different.

Therefore we like that each application can maintain its own Instrumentation file independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:auto-instrumentation Issues for auto-instrumentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants