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

Move k8s metadata enrichment from fluentd to otel-collector #192

Merged
merged 3 commits into from
Sep 14, 2021

Conversation

dmitryax
Copy link
Contributor

This change is the first step towards moving away from fluentd logs collection.

Fluentd "kubernetes_metadata_filter" plugin is known as a performance bottleneck in k8s logs collection with fluentd. So moving away from it already gives performance benefits. Also it mitigates issues when fluentd ends up hammering k8s API.

This change is compatible with otel gateway, if otelCollector.enabled=true all logs are forwarded through the gateway and logs enrichment is happening there reducing load on k8s API.

Drawbacks:

Additional changes:

  • Extra attribute added by default k8s.pod.labels.app. Value for this attribute is taken from pod's "app" label if it's set. The change is needed to support istio use case. We also want to enable more of these attributes going forward Capture some pod labels by default #189.

This change is the first step towards moving away from fluentd logs collection.

Fluentd "kubernetes_metadata_filter" plugin is known as a performance bottleneck in k8s logs collection with fluentd. So moving away from it already gives performance benefits. Also it mitigates issues when fluentd ends up hammering k8s API. 

This change is compatible with otel gateway, if `otelCollector.enabled=true` all logs are forwarded through the gateway and logs enrichment is happening there reducing load on k8s API.

Drawbacks:

- `container.image.name` logs attribute cannot be provided at the moment until this issue is resolved: open-telemetry/opentelemetry-collector-contrib#5235

Additional changes:

- Extra attribute added by default `k8s.pod.labels.app`. Value for this attribute is taken from pod's "app" label if it's set. The change is needed to support istio use case. We also want to enable more of these attributes going forward #189.
@dmitryax dmitryax requested review from a team as code owners September 14, 2021 17:34
@@ -531,9 +531,9 @@ image:

otelcol:
# The registry and name of the opentelemetry collector image to pull
repository: quay.io/signalfx/splunk-otel-collector
repository: quay.io/signalfx/splunk-otel-collector-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are pointing to dev? wouldn't that be unstable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not going to release the chart yet. I'll bump the version to the next stable one with the chart release. There is functionality here that depends on the latest changes in https://github.com/signalfx/splunk-otel-collector

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@rockb1017 rockb1017 left a comment

Choose a reason for hiding this comment

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

Looks great

Co-authored-by: Ryan Fitzpatrick <[email protected]>
@dmitryax dmitryax merged commit b0acd83 into main Sep 14, 2021
@dmitryax dmitryax deleted the migrate-k8s-metadata-enrichment branch September 18, 2021 01:28
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.

[k8sprocessor] Add an option to fetch container data
4 participants