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

OTEL processor discovery component enrichment issues #5306

Closed
oferziss-armis opened this issue Sep 26, 2023 · 3 comments · Fixed by #5378
Closed

OTEL processor discovery component enrichment issues #5306

oferziss-armis opened this issue Sep 26, 2023 · 3 comments · Fixed by #5378
Labels
bug Something isn't working frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.

Comments

@oferziss-armis
Copy link

What's wrong?

Seems like the otelcol.processor.discovery will not be really usable as it looks now.

I’ll explain.
Otel semantic conventions dictate a standard structure and naming convention for resources attributes (amongst all other attributes). these attributes are becoming the agreed upon standard upon which all backend visualization and correlation engines are relying.
so for K8s, stuff like k8s.pod.name and k8s.namespace.name or service.instance.id are expected and used to correlated stuff automatically.
The way this component works today is, you create a discovery component which can provide labels for targets for example kubernetes targets will be added with labels such as __meta_kubernetes_namespace and __meta_kubernetes_pod_ip.

BUT, the otelcol.processor.discovery component will not add any resource attributes from target labels begining with __. so we need to pass targets through a discovery.relabel component to map the labels from discovery to the attributes we want mapped.

Then we are reminded that prometheus relabeling will not allow . in any label.

The next major release of the agent giving us the otelcol.processor.k8s_attributes component hoping that will get us to a point where we are close to what the otel collector provides today…

BUT, considering other use-cases like i have. For example, i have to enrich resource attributes for signals coming from containers running on instances using docker-compose where we will need the same logic to work and we do not have a k8s api to help us. this makes the component while being a great idea not useable until we get some relabel engine for targets which is not bound but the prometheus spec...

Steps to reproduce

configure the agent with attached config on a k8s pod running in a daemonset deployment

System information

k8s

Software version

Grafana Agent v0.36.2

Configuration

config.river:

   /* Socket Entrypoint */
    otelcol.receiver.otlp "socket_entrypoint" {
      http {
        endpoint         = "0.0.0.0:4318"
        include_metadata = true
      }

      output {
        traces  = [otelcol.processor.discovery.k8s_attributes.input]
      }
    }

    discovery.kubernetes "k8s_pods" {
      role = "pod"

      selectors {
        role = "pod"
        field = format("spec.nodeName=%s", env("HOSTNAME"))
      }
      attach_metadata {
        node = true
      }

    }

    discovery.relabel "k8s_pods_attributes" {
      targets = discovery.kubernetes.k8s_pods.targets

      rule {
        source_labels = ["__meta_kubernetes_pod_uid"]
        target_label   = "service.instance.id"
      }
      rule {
        source_labels = ["__meta_kubernetes_namespace"]
        target_label   = "service.namespace"
      }
      rule {
        source_labels = ["__meta_kubernetes_namespace"]
        target_label   = "k8s.namespace.name"
      }
      rule {
        source_labels = ["__meta_kubernetes_node_name"]
        target_label   = "k8s.node.name"
      }
      rule {
        source_labels = ["__meta_kubernetes_pod_host_ip"]
        target_label   = "k8s.node.ip"
      }
      rule {
        source_labels = ["__meta_kubernetes_pod_name"]
        target_label   = "k8s.pod.name"
      }
      rule {
        source_labels = ["__meta_kubernetes_pod_ip"]
        target_label   = "k8s.pod.ip"
      }
    }

    otelcol.processor.discovery "k8s_attributes" {
      targets = discovery.relabel.k8s_pods_attributes.output
      output {
        traces = [
          otelcol.exporter.otlphttp.traces_exporter_tempo.input,
        ]
      }
    }


### Logs

```text
{"controller_id":"","err":"decoding River: \"service.instance.id\" is invalid 'target_label' for replace action","level":"error","msg":"failed to evaluate config","node":"discovery.relabel.k8s_pods_attributes","trace_id":"XXXXXXXX","ts":"2023-09-26T14:37:48.500267792Z"}
@ptodev
Copy link
Contributor

ptodev commented Sep 26, 2023

Hi Ofer, thank you for your report. The reason for the current behaviour of otelcol.processor.discovery is that it is porting the prom_sd_ functionality from the Agent's static mode. otelcol.processor.discovery is the migration path we offer to users of this Static mode functionality so that they could move to Flow easily.

You are correct that discovery.relabel only relabels in a Prometheus-compatible way, and it's not possible to have dots in the labels. But otelcol.processor.discovery was never meant to be a generic way to add resource attributes to spans - for that we will have the transform processor, which I expect to be released in the next version of the Agent. In the next version there will also be otelcol.processor.k8sattributes for adding resource attributes from kubernetes metadata.

Since otelcol.processor.discovery works in the way it's expected to, I could add extra information to clarify the following in the docs of otelcol.processor.discovery:

  • This is a Flow equivalent to Static mode functionality.
  • The "targets" should ideally be Prometheus-compliant labels. otelcol.processor.discovery will not work well for non-Prometheus labels, because discovery.* generally only work with Prometheus labels.
  • This is not a generic way of adding extra resource atributes to spans. Users should consider otelcol.processor.transform and otelcol.processor.k8sattributes before they consider otelcol.processor.discovery.

I do understand the confusion, since normally otelcol components don't depend on Prometheus conventions. otelcol.processor.discovery is an exception to this, and we do need to make this clear in the docs.

I'm planning to do the following:

  • Add otelcol.processor.transform to main later this week
  • Update the docs as specified above
  • Close this issue

I hope you are ok with this?

Adding resource attributes to spans is something which many users have requested, and I'm happy that v0.37 should finally address those needs. Thank you again for your feedback, it is very helpful and appreciated!

@oferziss-armis
Copy link
Author

oferziss-armis commented Sep 27, 2023

Hi @ptodev, thanks for the response.
while i do get the point you are making, the issue is that otelcol.processor.transform if i understand the name correctly (it's not in the next release docs yet) will allow us to perform actions in the same way that we do in the otelcol.processor.attributes component. this means we need to create pipelines where we use the otelcol.processor.discovery component to add the data we need in temp attributes and then the transform component to adjust them into standard attributes. which makes the pipeline very complicated.

i would suggest maybe renaming the current discovery processor to a new more fitting name and making this one simply not rely on the prom_sd engine. the idea behind this component is truly amazing and provides so many interesting implementation possibilities that i believe it makes sense to preserve while simply reducing the limitations imposed by the prom_sd engine

@ptodev
Copy link
Contributor

ptodev commented Oct 5, 2023

i would suggest maybe renaming the current discovery processor to a new more fitting name and making this one simply not rely on the prom_sd engine.

I tested otelcol.processor.discovery on labels with dots just now, and it worked ok. The limitation on Prometheus naming comes from the fact that most discovery.* components such as discovery.relabel and discovery.kubernetes only work with Prometheus-compatible label names. Since otelcol.processor.discovery is made to interact with those components, it is in practice very difficult to add attributes containing dots - because no discovery component is willing to pass them down to otelcol.processor.discovery.

I hope that the additions to the docs via #5378 as sufficient for closing this issue.

I do agree that we could work on making discovery.* components more Otel-friendly, but if the use case raised by this issue can be solved by using otelcol.processor.k8sattributes, then I don't see a need to introduce such features just now. Maybe this could come up if someone wants to use a discovery.* other than discovery.kubernetes together with otelcol.processor.discovery.

@github-project-automation github-project-automation bot moved this from Todo to Done in Grafana Agent (Public) Oct 11, 2023
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants