-
Notifications
You must be signed in to change notification settings - Fork 3
When related to a certificates operator, charm_tracing provides no way to wait for client charm to settle and form relations before TLS errors thrown #121
Comments
I added last week functionality to only pop that error once per event. But you're still going to get the error if you give charm_tracing a path to a cert but the path isn't valid, or if you're not passing a cert but tempo is expecting https communication. Indeed, charm_tracing has no knowledge of the relations your charm has. All it knows is whether you're passing a cert_path to it or not. If yes, it attempts to use it to talk to an https endpoint and fails if either the file isn't there, or the server rejects the connection. We're aware of a race due to the timing of the root span creation. Are you seeing this event after the relation is healthy and the cert has been written to disk? @mmkay we could consider making that error less verbose and just saying 'failed to send trace over https', and put the whole traceback in debug. WDYT? |
Can confirm that we are seeing this error trace when the relation with the certificates operator has not yet been formed and the relation is not healthy if it has been formed Consideration: would it be too convoluted for the |
we can silence the warnings, that's not the issue, the question is how to do that without hiding too many actual errors. It feels hard to generalize what that healthcheck should look like, and you can already disable charm tracing by returning None: from charms.tempo_k8s.v1.charm_tracing import trace_charm
@trace_charm(
tracing_endpoint="my_tracing_endpoint",
)
class MyCharm(CharmBase):
...
@property
def charm_tracing_ready():
# custom healthcheck
return tracing.is_ready() and self.certificates.is_ready() and self.has_written_cert_to_disk() and ... # custom checks
@property
def my_tracing_endpoint(self) -> Optional[str]:
if self.charm_tracing_ready:
return self.tracing.otlp_http_endpoint()
else:
return None # charm tracing disabled for the run does this work for you? |
@PietroPasotti you're right, i hadnt thought of returning |
I think we have a neat solution cooking, stay tuned #135 |
Bug Description
When tempo-k8s is related to a certificates operator, the charm_tracing lib produces a flood of errors when the client application (mysql) is related to tempo-k8s but not the certificates operator. My understanding is that the
trace_charm
decorator does not check if the relation with the certificates operator exists before trying to export spans to tempo-k8s (it always calls and utilizes theserver_cert
kwarg in thetrace_charm
decorator)To Reproduce
In a microk8s model:
1.git clone [email protected]:canonical/cos-lite-bundle.git
2. tox -e render-edge
3. juju deploy ./bundle.yaml --edge
4. juju deploy -n 1 tempo-k8s --channel edge
5. juju deploy -n 1 self-signed-certificates
6. jhack imatrix fill
7. juju offer tempo-k8s:tracing
8. juju offer self-signed-certificates:certificates
In an lxd model:
Environment
juju: 3.4.2
lxd: 5.21.1 LTS
MicroK8s v1.27.13 revision 6744
cert_handler.py: v1.8
Relevant log output
When
None
is passed in asserver_cert
to thetrace_charm
decoratorWhen related with tempo-k8s, but no relation with
self-signed-certificates
while providing aserver_cert
kwarg:Additional context
No response
The text was updated successfully, but these errors were encountered: