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

Provide a way to add annotations to Pods #426

Closed
indrekj opened this issue Sep 9, 2021 · 8 comments · Fixed by #451
Closed

Provide a way to add annotations to Pods #426

indrekj opened this issue Sep 9, 2021 · 8 comments · Fixed by #451
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@indrekj
Copy link
Contributor

indrekj commented Sep 9, 2021

Hi,

We're using DataDog to collect metrics. I've noticed that prometheus.io/scrape is added to Deployment. DD supports prometheus.io/scrape but it has to be in the Pod spec, not in the Deployment spec. Their current implementation is quite limited though and I'd prefer to use their OpenMetrics autodiscovery hook.

For that I'd need to add annotations to Pod, e.g.:

  annotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"}
          ]
        }
      ]

I noticed that it is possible to add annotations to OpenTelemetryCollector CRD, but then this annotation is propagated to everything (deployment, services, pods, etc). This causes error noise and I'd like to avoid it.

It would be good to be able to control Pod annotations.

@jpkrohling
Copy link
Member

Is this about the sidecar that is injected? Or is it about the PodSpec of a collector instance as Deployment/Daemonset/Statefulset?

@indrekj
Copy link
Contributor Author

indrekj commented Sep 14, 2021

The latter: PodSpec of a collector instance as deployment.

@jpkrohling
Copy link
Member

The only problem I have with this is that it leaks the detail about the pod(s) that is being created by the operator. For instance, right now we create only one pod for most CRs. In the future, we might create more (see the TargetAllocator feature that was just merged). In that case, should the annotations specified here be applied to all pods created as a result of a CR?

@indrekj
Copy link
Contributor Author

indrekj commented Sep 27, 2021

In our case, we would like to annotate all the pods. Then in DataDog we can aggregate them and see how things are performing. It also allows showing how one particular pod is performing.

@jpkrohling
Copy link
Member

Just to double-check: if you are using the target allocator, you'd want the annotations to be present at both the pods for the target allocator and for the collector, right?

@indrekj
Copy link
Contributor Author

indrekj commented Oct 5, 2021

I'm not that familiar with the target allocator yet so pardon my ignorance. I assume you mean collector pods that target allocator creates (agents I guess?) and collector (as in gateway?), then yes. I would like to annotate all of them. If I could provide different annotations for them, then that would be even better but both should work for us.

EDIT: Just to clarify. We're currently using just "Gateway" with deployment type. Soon we're also planning to add agents likely in a form of sidecars that proxy everything to the gateway pods.

@jpkrohling
Copy link
Member

The target allocator is a different deployment that is created when Prometheus is used. Sounds good to me, I think we can start by adding the annotation to all pods and see how it goes.

Would you like to open a PR with that?

@jpkrohling jpkrohling added enhancement New feature or request good first issue Good for newcomers labels Oct 5, 2021
indrekj added a commit to indrekj/opentelemetry-operator that referenced this issue Oct 5, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
@indrekj
Copy link
Contributor Author

indrekj commented Oct 5, 2021

Tried to put something together: #451

indrekj added a commit to indrekj/opentelemetry-operator that referenced this issue Oct 5, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
indrekj added a commit to indrekj/opentelemetry-operator that referenced this issue Oct 5, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
indrekj added a commit to indrekj/opentelemetry-operator that referenced this issue Oct 5, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
jpkrohling pushed a commit that referenced this issue Oct 6, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes #426
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this issue Dec 12, 2021
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this issue May 1, 2024
This allows setting podAnnotations, e.g:
```
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
  name: opentelemetry
  annotations:
    foo: "this won't be applied to pods"
spec:
  mode: deployment
  podAnnotations:
    ad.datadoghq.com/otc-container.check_names: '["openmetrics"]'
    ad.datadoghq.com/otc-container.init_configs: '[{}]'
    ad.datadoghq.com/otc-container.instances: |-
      [
        {
          "prometheus_url": "http://%%host%%:8888/metrics",
          "namespace": "opentelemetry.collector",
          "metrics": [
            {"otelcol_exporter_queue_size": "exporter.queue_size"},
            {"otelcol_exporter_send_failed_spans": "exporter.send_failed_spans"},
            {"otelcol_exporter_sent_spans": "exporter.sent_spans"},
          ]
        }
      ]
  # ...
```

Fixes open-telemetry#426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants