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

fix: set cluster field in metadata pipelines #2284

Merged
merged 1 commit into from May 24, 2022

Conversation

swiatekm-sumo
Copy link
Contributor

@swiatekm-sumo swiatekm-sumo commented May 17, 2022

Description

The cluster metadata field was the only field set on the collector in Sumo, rather than in metadata pipelines. In the metadata pipelines, how it was handled varied a lot:

  • FluentD metrics set it correctly.
  • FluentD logs didn't set it at all.
  • OT pipelines were actively deceptive, pretending to set it in the k8s_tagger processor, which doesn't actually do anything.

The K8s metadata field that allows the k8s_tagger to set the cluster name is being deprecated in K8s upstream due to being actively confusing and not very useful. See: open-telemetry/opentelemetry-collector-contrib#9886 for more context, and also SumoLogic/sumologic-otel-collector#578.

Additionally, setting the field on the collector actually behaves subtly differently from sending it as a metric label, which caused one nasty bug for a client trialing OT.

This change explicitly sets the field based on the configuration value and removes it from the collector.


Checklist
  • Changelog updated
Testing performed
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@swiatekm-sumo swiatekm-sumo requested a review from a team as a code owner May 17, 2022 11:24
@swiatekm-sumo swiatekm-sumo added this to the v2.9 milestone May 17, 2022
@swiatekm-sumo
Copy link
Contributor Author

I noticed I also need to add this for FluentD events, which use the log source. Tagging this don't merge until that's done.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains invalid labels. Please remove all of the following labels: ['do-not-merge/hold']

The `cluster` metadata field was set on the collector in Sumo. In the
metadata pipelines, the behaviour was inconsistent. FluentD metrics set
it correctly, but seemingly no other pipeline did. In particular, OT
pipelines were actively deceptive,
pretending to set it in the k8s_tagger processor, which doesn't actually
do anything, and was recently deprecated upstream for this reason.

Additionally, setting the field on the collector actually behaves
subtly differently from sending it as a metric label, which caused one
nasty bug for a client trialing OT.

This change explicitly sets the field based on the configuration value
and removes it from the collector.
@swiatekm-sumo swiatekm-sumo enabled auto-merge (rebase) May 24, 2022 10:18
@swiatekm-sumo swiatekm-sumo merged commit 6e7809e into main May 24, 2022
@swiatekm-sumo swiatekm-sumo deleted the fix/cluster-metadata branch May 24, 2022 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants