search

canonical / loki-k8s-operator

https://charmhub.io/loki-k8s
Apache License 2.0
10 stars 16 forks source link

Incorrectly assuming that `job` label is always there when replacing `%%juju_topology%%` #406

Open sed-i opened 5 months ago

sed-i commented 5 months ago

Bug Description

When using LogForwarder, it is incorrect to replace the topology stub with a job matcher.

Following up on #332, we can no longer assume that the job label would always be there, because pebble does not create it. https://github.com/canonical/loki-k8s-operator/blob/0f6b87015c151f3f716a4ad54e0a3b1334f82b2f/lib/charms/loki_k8s/v0/loki_push_api.py#L768-L771

Originally reported by @saltiyazan.

Related:

To Reproduce

NTA

Environment

NTA

Relevant log output

NTA

Additional context

No response

saltiyazan commented 5 months ago

Thanks for opening this @sed-i I wasn't sure if it was a bug or a misconfiguration on my side :)

PietroPasotti commented 5 months ago

this happened because in the pebble log forwarder never added the 'job' and 'instance' labels. --> @barrettj12

hopefully it will be fixed and the fix backported to the pebbles in juju 3.4 and up

sed-i commented 5 months ago

Just realized that our lib should probably be the one creating it, just like we do all other labels:

https://github.com/canonical/loki-k8s-operator/blob/3ddd54ea8b1108efff323c274b0e178deed010a8/lib/charms/loki_k8s/v1/loki_push_api.py#L2426-L2435

barrettj12 commented 5 months ago

Personally I supported adding job and instance labels by default to Pebble, but I know there was some opposition to this and we ultimately decided against it.

Is it possible for the charm just to manually set these labels in the Pebble plan?