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

Update default label behaviour for metadata enhancement plugin #60

Merged
merged 4 commits into from
Jul 11, 2019

Conversation

samjsong
Copy link
Contributor

@samjsong samjsong commented Jul 2, 2019

  1. We remove the kubernetes prefix from all of the enhanced metadata attached by the enhance_k8s_metadata filter plugin, including pod.labels.foo
  2. We rename service metadata to prometheus_service to avoid collision with the future attached service metadata.

Example metric output:

up {cluster="ssong-k8s-stag-9", _collector="kubernetes-1561673833", _collectorId="00000000061841EB", _contentType="Prometheus", _origin="kubernetes", _source="(default-metrics)", _sourceCategory="Http Input", _sourceId="00000000062EEE26", _sourceName="Http Input", deployment.name="kube-dns", endpoint="http-metrics", instance="100.115.226.65:9153", job="coredns", namespace="kube-system", pod.labels.k8s-app="kube-dns", pod.labels.pod-template-hash="2609061007", prometheus="sumologic/prometheus-operator-prometheus", prometheus_replica="prometheus-prometheus-operator-prometheus-0", prometheus_service="prometheus-operator-coredns", replicaset.name="kube-dns-6b4f4b544c", source="Metrics"}

@samjsong
Copy link
Contributor Author

samjsong commented Jul 2, 2019

Found out this breaks things - since we have metadata in the form pod.labels.foo, this is actually represented as record = { pod => labels => { foo = bar } ... } which overrides the original metadata for pod=POD_NAME

Will fix and reopen PR

@samjsong samjsong closed this Jul 2, 2019
@samjsong samjsong reopened this Jul 8, 2019
@samjsong
Copy link
Contributor Author

samjsong commented Jul 8, 2019

To avoid collision with pod, I've renamed pod.labels to pod_labels. I hope this is okay @frankreno
cc @maimaisie @yuting-liu the PR is ready for review now.

Example metric:

kube_pod_container_info {cluster="ssong-k8s-stag-10", container="fluentd", image="234646415255.dkr.ecr.us-west-2.amazonaws.com/sumologic/fluentd-kubernetes-sumologic:latest", _collector="ssong-k8s-stag-10", _collectorId="000000000618C5E6", _contentType="Prometheus", _origin="kubernetes", _source="kube-state-metrics", _sourceCategory="Http Input", _sourceId="00000000062F919B", _sourceName="Http Input", container_id="docker://547dd4241002c952d83d34b33ceff35414d3827cdd8a7b6a19f459f1b51c6208", deployment.name="fluentd", endpoint="http", image_id="docker-pullable://<IMAGE>", instance="100.108.94.31:8080", job="kube-state-metrics", namespace="sumologic", pod="fluentd-59d9c9656d-flbn9", pod_labels.k8s-app="fluentd-sumologic", pod_labels.pod-template-hash="1585752128", prometheus="sumologic/prometheus-operator-prometheus", prometheus_replica="prometheus-prometheus-operator-prometheus-0", prometheus_service="prometheus-operator-kube-state-metrics", replicaset.name="fluentd-59d9c9656d", source="Metrics"}

@frankreno
Copy link
Contributor

frankreno commented Jul 9, 2019

@samjsong : in FluentD for logs we do pod_label_XXX where XXX is the actual label key. Can we do the same thing here? You cannot have 2 labels with the same key and you already addressed the Prometheus service collision.

@samjsong
Copy link
Contributor Author

samjsong commented Jul 9, 2019

That makes sense, thanks for the input @frankreno . In that case, is there any reason we use the dot . to join the nested jsons of the metrics records? Can we use the underscore _ instead? This would make the above request a lot easier.

On a tangentially related note, I noticed we attach deployment => {name => NAME} and replicaset => {name => NAME} which gets "dotifiied" in the prometheus_format filter plugin (which is why we see deployment.name=NAME and replicaset.name=NAME). This is fine for metrics, but messy for logs, so I'll instead change it to deployment => NAME and replicaset => NAME

@samjsong
Copy link
Contributor Author

Synced with @frankreno offline, he believes the underscore _ works better as the SPLITOR rather than the ., made the relevant changes.

New sample metric:

up {cluster="ssong-k8s-stag-10", _collector="ssong-k8s-stag-10", _collectorId="000000000618C5E6", _contentType="Prometheus", _origin="kubernetes", _source="(default-metrics)", _sourceCategory="Http Input", _sourceId="00000000062F9197", _sourceName="Http Input", deployment="kube-dns", endpoint="http-metrics", instance="100.114.10.95:9153", job="coredns", namespace="kube-system", pod="kube-dns-6b4f4b544c-gzl2r", pod_labels_k8s-app="kube-dns", pod_labels_pod-template-hash="2609061007", prometheus="sumologic/prometheus-operator-prometheus", prometheus_replica="prometheus-prometheus-operator-prometheus-0", prometheus_service="prometheus-operator-coredns", replicaset="kube-dns-6b4f4b544c", source="Metrics"}

Notice the underscores in the pod_labels_foo, and that deployment.name and replicaset.name are now deployment and replica respectively

@samjsong samjsong merged commit 58da820 into master Jul 11, 2019
@samjsong samjsong deleted the ssong-default-labels-behaviour branch July 11, 2019 21:04
psaia pushed a commit to psaia/sumologic-kubernetes-collection that referenced this pull request May 25, 2021
* make a true cli

* update messages

* rename env vars

* remove codified defaults

* add add-user pipe

* golang img

* main.go

* non-alpine

* workstash

* update commans

* update files

* ugh python

* Delete associate_groups_to_azure_apps.ps1

* Delete existing-resources.tf

* Delete pyvenv.cfg

* wording fix

Co-authored-by: Mel Cone <mel.cone@nytimes.com>
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.

4 participants