-
Notifications
You must be signed in to change notification settings - Fork 197
Remove resource/k8s processor and use k8sattributes processor for service attributes #8599
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
Remove resource/k8s processor and use k8sattributes processor for service attributes #8599
Conversation
This pull request does not have a backport label. Could you fix it @ChrsMark? 🙏
|
f541e93
to
b38637c
Compare
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Do we know if there's been any proposal to remove the original extracted attributes? All matching signals will have two resource attributes with the same value
There is no such proposal so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this doesn't affect any of the content for K8s Otel Observability?
Ah @rogercoll did you mean the ones extracted at https://github.com/elastic/elastic-agent/blob/main/deploy/helm/edot-collector/kube-stack/values.yaml#L120C1-L126C26? I think we can remove them as well since they were deleted anyways in |
0b7a420
to
5e35b85
Compare
I dont see a problem as long as service.version and service.name are populated. Reading the old issues (1, 2) we just want to mimic what Agent had in order to support the inventory page. Only side effect from this change is that we might have diffrent names/versions because we change slightly the logic but is ok I think. To be on safe side, can you please add the https://opentelemetry.io/docs/specs/semconv/non-normative/k8s-attributes/#service-attributes on the changelog in order our users to have access to the logic? Maybe also explain the fields in the description. I agree, we can delete app.label.name and app.label.version |
93760ac
to
6d1f85e
Compare
…vice attributes Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
6d1f85e
to
b8045fb
Compare
Signed-off-by: ChrsMark <[email protected]>
b8045fb
to
9cf6007
Compare
|
💚 Build Succeeded
History
cc @ChrsMark |
…vice attributes (#8599) * Remove resource/k8s processor and use k8sattributes processor for service attributes Signed-off-by: ChrsMark <[email protected]> * remove k8s labels extraction Signed-off-by: ChrsMark <[email protected]> * update changelog Signed-off-by: ChrsMark <[email protected]> --------- Signed-off-by: ChrsMark <[email protected]> (cherry picked from commit b2aa165)
…vice attributes (#8599) (#8653) * Remove resource/k8s processor and use k8sattributes processor for service attributes * remove k8s labels extraction * update changelog --------- (cherry picked from commit b2aa165) Signed-off-by: ChrsMark <[email protected]> Co-authored-by: Christos Markou <[email protected]>
…-hosted * feature/hosted-stack-using-oblt-cli: (26 commits) Use the current official docker image for oblt-cli Mark the elasticinframetrics processor as deprecated and schedule for removal (#8659) [main][Automation] Update versions (#8668) chore: Update create_deployment_csp_configuration.yaml (#8669) Attempt to make test more reliable by querying ES directly (#8422) [test] split up ess and beats serverless integration tests (#8551) Remove resource/k8s processor and use k8sattributes processor for service attributes (#8599) fix: use --force-confold for deb tests in TestUpgradeAgentWithTamperProtectedEndpoint_DEB (#8649) [main][Automation] Bump stack images versions to 9.1.0-ea0b7542 (#8612) chore: Update to elastic/beats@f6594fb72670 (#8640) [deb/rpm] restart endpoint with tamper protection after elastic-agent (#8637) ci: don't preinstall fleet packages on retried CI steps (#8636) chore: Update to elastic/beats@6b6941eed496 (#8619) [main][Automation] Bump VM Image version to 1750467641 (#8617) flaky: skip TestUpgradeAgentWithTamperProtectedEndpoint_RPM (#8626) Add skip-changelog PR label for bump VM PRs (#8627) build(deps): bump github.com/elastic/go-seccomp-bpf from 1.5.0 to 1.6.0 (#8611) [ci] fix k8s integration tests flakiness (#8575) bump apmconfig Otel extension to v0.3.0 (#8600) Enhancement/6394 allow deb rpm to upgrade with endpoint tamper protection (#6907) ...
This PR removes the
resource/k8s
processor in honour of thek8sattributes
processor that provides native support for the Service attributes.This change is aligned with the respective Semantic Conventions' guidance: https://opentelemetry.io/docs/specs/semconv/non-normative/k8s-attributes/#service-attributes
This is also aligned with the logic the OpenTelemetry Operator applies:
Potential breaking change
The
k8sattributes
processor does not override service attributes if those are already set. Hence, this change only affects use-cases that were relying on the enrichment that theresource/k8s
does. This should be mainly logs and metrics since the SDKs usually set those attributes on their own.With this change the
service.version
calculation is not affected at all. Theservice.name
calculation now gives priority tok8s.{deployment, daemonset, ...}.name
before using thek8s.container.name
. That's the only possible "breaking" change but should not be really impactful.This PR requires
k8sattributes
processor to be at leastv0.127.0
.v0.127.0
was introduced by #8511 tomain
and8.19.x
.