-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[metricbeat] support deployment/daemonset specific metrics #820
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💚 CLA has been signed |
After posting the comment, the check was re-evaluated automatically 🤷 |
jenkins test this please |
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, just a nit on test naming
metricbeat/tests/metricbeat_test.py
Outdated
@@ -968,24 +968,101 @@ def test_cluster_role_rules(): | |||
assert rules["resources"][0] == "something" | |||
|
|||
|
|||
def test_adding_pod_labels(): | |||
def test_adding_legacy_labels(): |
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.
I would prefer naming this test test_adding_deprecated_labels
to be more consistent with the other related test names.
jenkins test this please |
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⛴
* [metricbeat] support deployment/daemonset specific metrics * chore: rename test_adding_legacy_labels to test_adding_deprecated_labels
* [metricbeat] support deployment/daemonset specific metrics * chore: rename test_adding_legacy_labels to test_adding_deprecated_labels
* [metricbeat] support deployment/daemonset specific metrics * chore: rename test_adding_legacy_labels to test_adding_deprecated_labels
backported to |
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml
Closes #812
Turned out, initially labeling was completely implemented for DaemonSet (both daemon-set and pod labels), but only partially for Deployment (only pod-level labels). I decided to make these properties to be consistent for both object types, while still keeping to the design we agreed in the linked issue