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

split k8sattributes/ecs processor: do not add any extra metadata #46

Conversation

tetianakravchenko
Copy link
Contributor

What: create k8sattributes/ecs processor where not added extra metadata.

Why: with the common for otel and ecs k8sattributes processor all docs (in ecs mode) contained those fields:

k8s.daemonset.name
k8s.pod.ip
k8s.pod.start_time
k8s.replicaset.name
k8s.statefulset.name

those fields were not translated into the kubernetes.* field format (in opposite to fields like deployment.name, namespace, node.name, pod.name and pod.uid - that is defined here https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/57caf5f830369d3f4994f7563323c73d6842424a/exporter/elasticsearchexporter/model.go#L55-L59)

this PR is to prevent adding fields in k8s.* format in ecs mode docs and to prevent error related to the k8s.pod.ip field:

failed to index document	{"kind": "exporter", "data_type": "metrics", "name": "elasticsearch/ecs", "index": ".ds-metrics-kubernetes.pod-default-2024.11.04-000001", "error.type": "document_parsing_exception", "error.reason": "[1:699] failed to parse field [k8s.pod.ip] of type [ip] in a time series document at [2024-11-04T15:20:56.719Z]"}

Note: there still will be added k8s.pod.start_time attribute, because it enabled by default

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Copy link
Member

@carsonip carsonip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also possible to fix this by updating es exporter to translate these fields to ecs. Do you prefer one fix over another?

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@@ -697,6 +697,13 @@ collectors:
name: k8s.pod.uid
- sources:
- from: connection
extract:
metadata:
- "k8s.replicaset.name"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note:
for those fields translation happens on the es exporter side:

- "k8s.namespace.name"
- "k8s.deployment.name"
- "k8s.node.name"
- "k8s.pod.name"
- "k8s.pod.uid"

those fields were removed in comparison to k8sattributes for otel mode:

- "k8s.pod.ip"
- "k8s.pod.start_time"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that!
So clarification,
- "k8s.replicaset.name",
- "k8s.statefulset.name"
- "k8s.daemonset.name"
- "k8s.cronjob.name"
- "k8s.job.name"
will remain as is in the kubernetes.pod datastream. We dont have a unique naming convention, just saying !

@carsonip
Copy link
Member

carsonip commented Nov 6, 2024

The upstream es exporter fix is open-telemetry/opentelemetry-collector-contrib#36233 . When that is merged, this PR can be reverted.

@rogercoll
Copy link
Contributor

@tetianakravchenko Do we want to include this in the 8.16 release version?

@tetianakravchenko
Copy link
Contributor Author

tetianakravchenko commented Nov 6, 2024

@tetianakravchenko Do we want to include this in the 8.16 release version?

@rogercoll yes, merging of this PR will not make it included in 8.16?

@tetianakravchenko tetianakravchenko merged commit a79624b into elastic:main Nov 6, 2024
1 check passed
@rogercoll
Copy link
Contributor

@tetianakravchenko Do we want to include this in the 8.16 release version?

@rogercoll yes, merging of this PR will not make it included in 8.16?

No, we need to manually merge main instead :( #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants