canonical / tempo-coordinator-k8s-operator

This charmed operator is part of automation the operational procedures of running Grafana Tempo, an open-source tracing backend, in microservices mode.
Apache License 2.0
0 stars 0 forks source link

Add a way to configure extra receivers for uncharmed applications #7

Closed michaeldmitry closed 2 weeks ago

michaeldmitry commented 3 weeks ago

Issue

Fixes https://github.com/canonical/tempo-k8s-operator/issues/87

Solution

Expose juju config options to force enable each protocol supported by Tempo.

Testing instructions

Deploy Tempo coordinator

cwd to tempo-coordinator-k8s-operator

charmcraft pack
juju deploy ./tempo-coordinator-k8s_ubuntu-22.04-amd64.charm 

"Unblock" coordinator

juju deploy facade s3-facade --channel=latest/edge
juju integrate s3-facade:provide-s3 tempo-coordinator-k8s:s3
juju run s3-facade/0 update --params ./tests/manual/facades/s3.yaml

cwd to tempo-worker-k8s-operator

git fetch
git checkout coordinator-integration
charmcraft pack
juju deploy ./tempo-worker-k8s_ubuntu-22.04-amd64.charm  tempo-worker --resource tempo-image=docker.io/ubuntu/tempo:2-22.04
 juju integrate tempo-coordinator-k8s tempo-worker

Validation

juju run tempo-coordinator-k8s/0 list-receivers

should see sth like

otlp-http: http://coord-0.coord-endpoints.test.svc.cluster.local:4318

Enable extra receiver

juju config tempo-coordinator-k8s always_enable_zipkin=True
juju run tempo-coordinator-k8s/0 list-receivers

Should now see sth like

otlp-http: http://coord-0.coord-endpoints.test.svc.cluster.local:4318
zipkin: http://coord-0.coord-endpoints.test.svc.cluster.local:9411
michaeldmitry commented 2 weeks ago

looking good! not sure why this PR and the other both share a bunch of apparently overlapping changes to the test code (such as the commented-out modules). +100 for fixing the tests, but still I'd have preferred to have a single pr to fix the tests, then two PRs on top of that doing the receivers and the pydantic port.

This makes it quite hard to review the test code changes. For the next time :) Now let's try to merge these two.

100% agreed. Will take care next time. Comments on the overlapping changes for test files will be addressed in https://github.com/canonical/tempo-coordinator-k8s-operator/pull/5