Add namespace k8s tagger

[chaitanyaphalak]

Description: Add feature to fetch namespace metadata, labels and annotations

Link to tracking Issue: Feature request - #3430 and use-case - #3031

Testing:

Documentation:

[owais]

I think we should re-think the config changes this introduces. Instead of a new extract_namespaces section, we can extend the existing extract section. Some ideas,

1. Use path like keys under extract to tell the exporter to extract from something other than the pod.

    extract:
      namespace/annotations:
      - key: splunk.com/index
      - key: splunk.com/sourcetype
      - key: splunk.com/exclude

This works well if we always want to support extacting data from objects that are related to a pod. In the example above, w'd extract the annotations from the namespace that the pod is in. In future we could have deployment/*, daemonset/*, service/* or any other type that could be related to the pod which generated the span.

2. Add a from: attribute to every item to specify where to extract the item from.

    extract:
      annotations:
      - key: splunk.com/index
        from: namespace
      - key: splunk.com/sourcetype
        from: namespace
      - key: splunk.com/exclude # gets extracted from pod by default

the new from attribute can default to pod and as a result needs to be mentioned only when extracting from something other than a pod.

I feel this allows for a much nicer config pattern that can be extended in future as we add support for newer types. Open to other ideas as well.

[chaitanyaphalak]

@owais
I think the first option feels easy to use from a config and backward compatability point of view.

Should we follow the same pattern for metadata and labels?

extract:
  namespace/labels:
    - tag_name: l1 # extracts value of label with key `label1` and inserts it as a tag with key `l1`
       key: label1      
namespace/metadata:
    # extract the following well-known metadata fields
    - namespaceName
    - namespaceUID
    - namespaceCluster
    - namespaceStartTime

[owais]

Should we follow the same pattern for metadata and labels?

Yes. I used annotations just as an example. We should support all feature we can/want for every k8s resources we add support for.

I'd wait to see what others think about these config options before going ahead with the implementation. Tagging some people who might be interested in this.

@dmitryax @pmm-sumo @ericmustin @tigrannajaryan @pmatyjasek-sumo @dashpole

[dmitryax]

I like both options that Owais suggested, the option 2 looks probably a bit better to me.

Having a default value pod for the additional from key looks more natural than assuming that annotations == pod/annotations.

[owais]

@dmitryax agree. It also makes it easier to deal with config if we don't have to split and compare strings.

[pmatyjasek-sumo]

I think the option 2 looks better. It is inline with for example pod_association convention

[tigrannajaryan]

Option #2 looks good to me as a way to specify where from to extract annotations. Question: does extracted data get added as a Resource attribute or as a Span/LogRecord attribute?

[tigrannajaryan]

What do we do with the additional well-know fields? Do they go under the existing "metadata" setting? e.g.:

    extract:
      metadata:
        # extract the following well-known metadata fields
        # these are pod fields:
        - podName
        - podUID
        - deployment
        - cluster
        - namespace
        - node
        - startTime
        # these are namespace fields:
        - namespaceName # is this the same as "namespace" above?
        - namespaceUID
        - namespaceCluster
        - namespaceStartTime

Do we believe that this single list of fields is sufficient for future needs? Do we expect field name clashes that make it impossible to disambiguate which entity the metadata should be extracted from? For example is "startTime" the pod start time or node start time (hypothetically speaking)?

[ericmustin]

don't have super strong opinions here, but #2 seems a bit cleaner generally speaking

[pmm-sumo]

No strong opinion here either. Option 2 seems cleaner but option 1 is less verbose.

Does extracted data get added as a Resource attribute or as a Span/LogRecord attribute?

I think that in principle, this processor should impact Resource-level attributes only and it seems it wasn't changed here. If attributes are on a record-level, they can be extracted level up using groupbyattrsprocessor

[chaitanyaphalak]

@tigrannajaryan These are added as resource attributes. "startTime" is distinct for all object types like pods, namespaces etc, so they all need to be distinct in some way. Also "namespace" and "namespaceName" are the same but extracted from different objects, pods, and namespace respectively.

[tigrannajaryan]

Should we use Otel-compliant names for fields that we extract? So for example instead of:

    extract:
      metadata:
        # extract the following well-known metadata fields
        # these are pod fields:
        - podName
        - podUID
        - deployment
        - cluster
        - namespace
        - node
        - startTime

we do:

    extract:
      metadata:
        # extract the following well-known metadata fields
        # these are pod fields:
        - k8s.pod.name # this comes from a pod
        - k8s.pod.uid
        - k8s.deployment.name # where does this come from? from pod or the associated deployment? Or it is the same thing?
        - k8s.namespace.name # this comes from a namespace
        - k8s.pod.start_time # pod's start time.
        - k8s.node.start_time # node's start time.
        - startTime # alias for k8s.pod.start_time
        - etc ...

This way it is more apparent what is it that we are populating. This way we can also disambiguate fields which may be present in more than one entity. Thoughts?

I understand this means changing already existing field names, but we can do it in a backward compatible way by supporting old names as aliases for the fields that we start supporting new canonical Otel names.

[chaitanyaphalak]

@tigrannajaryan that actually makes it really easy for the users to understand what they want and configure appropriately.

[owais]

@tigrannajaryan makes sense for the "metadata" section which is static list of well-known fields. One implication of this is that if we need support for new fields, we'll first need to go and update the semantic conventions (which is probably a good thing anyway).

But I think this should be done in a separate PR.

@chaitanyaphalak this reminds me that metadata section is list of well-known fields and the processor knows exactly how and where to fetch these values from (ref). As such, I don't think it makes sense for users to specify a from: option to metadata fields. It still makes sense for Annotations and Labels though.

I think we should split this into multiple PRs:

1. First PR updates the field names to what Tigran suggested. Let's not add any new fields. Only rename.
2. (if applicable) PR adds new well-known fields that might possibly require us to fetch and watch namespace resources.
3. Add from: support to annotations and labels so they can be extracted from namespaces in addition to pods.

If you only need or want to work on 3. let's create an issue to track what Tigran suggested. We should do it before GA.

WDYT?

[chaitanyaphalak]

@owais I am cool working on all the 3 things along with adding the namespace metadata fields to the semantic conventions here -
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/k8s.md

[owais]

@chaitanyaphalak One more thing. If you add support for new metadata fields, try to restrict them in a way so they can be extracted from the pod spec and won't require watching resources other than pods. If we do add support for some fields that can't be extracted from pods and need additional API calls, let's not turn them on by default.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[tigrannajaryan]

@owais @dmitryax PTAL.

[owais]

Thanks. Overall direction looks good. Took a quick look and left some comments. Will review again in detail in the morning.

[owais]

Looks good but I can't find where we are setting default value for "from" to "pod". If we aren't yet, let's add that.

[chaitanyaphalak]

@owais The PR is ready.

[owais]

Looks good but left 2 minor comments. I trust you to take care of those before getting it merged :)

Thanks for the contribution. great work!

[tigrannajaryan]

Please update the documentation in doc.go to show how the new functionality can be used.