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

Relabel pod and service dimensions for non-pod metrics #878

Merged
merged 2 commits into from
Sep 3, 2020

Conversation

sumo-drosiek
Copy link
Contributor

Description

Relabel pod and service dimensions for non-pod metrics

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@sumo-drosiek sumo-drosiek requested review from frankreno, a team, vsinghal13, pmalek-sumo and perk-sumo and removed request for a team September 2, 2020 12:04
sourceLabels: [job, __name__]
- action: labelmap
regex: (pod|service)
replacement: sumo_metrics_${1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. sumo_metrics_ as the prefix feels unintuitive. In this case, pod and service are pointers to the pod/service that Prometheus scraped the data from. Let's change it sd_ to indicate it is part of the service discovery mechanism.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should go further and use service_discovery_${1} - no need to use the non-obvious abbreviation.

@sumo-drosiek sumo-drosiek force-pushed the drosiek-relabel-pod-for-unralated-metrics branch from 3205de6 to 740cb66 Compare September 2, 2020 16:21
Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@pmalek-sumo pmalek-sumo left a comment

Choose a reason for hiding this comment

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

🚢

Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

LGTM, but +1 to Perk's suggestion

Co-authored-by: Marcin Stożek <marcin.stozek@gmail.com>
@sumo-drosiek sumo-drosiek force-pushed the drosiek-relabel-pod-for-unralated-metrics branch from 6f1ea87 to 8b43864 Compare September 3, 2020 07:08
@sumo-drosiek sumo-drosiek merged commit bfc5a4d into master Sep 3, 2020
@sumo-drosiek sumo-drosiek deleted the drosiek-relabel-pod-for-unralated-metrics branch September 3, 2020 08:04
@perk-sumo perk-sumo added this to the v1.2 milestone Sep 3, 2020
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.

6 participants