Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

instance attribute support for charm_tracing forward-declared definitions #136

Merged
merged 9 commits into from
Jun 20, 2024

Conversation

PietroPasotti
Copy link
Contributor

@PietroPasotti PietroPasotti commented Jun 14, 2024

This PR adds support for charm_tracing to grab the endpoint and cert it needs from instance attributes instead of properties and methods.

This is a nicety which in combination with #135 means any charm can enable/disable charm tracing and correctly configure it with/without TLS with a single line of code (plus the decorator).

before:

from lib....charm_tracing import charm_tracing

@charm_tracing(tracing_endpoint="foo", server_cert="bar")
class MyCharm(...):
    [...]
    def foo(self):
        # complicated logic to return None if we don't have tracing or we have tracing and tls but no cert yet
        return ... 
    def bar(self):
        return ...

after:

from lib....charm_tracing import charm_tracing
from lib....tracing import charm_tracing_config

@charm_tracing(tracing_endpoint="foo", server_cert="bar")
class MyCharm(...):
    def __init__(...):
        self.tracing = TracingEndpointRequirer(...)
        self.foo, self.bar = charm_tracing_config(self.tracing, "path/to/cert/")

Additionally:

  • tuned down logs
  • be less forgiving and raise if tracing_endpoint returns rubbish or raises, like we're doing in charm_logging

lib/charms/tempo_k8s/v1/charm_tracing.py Dismissed Show dismissed Hide dismissed
Copy link
Contributor

@mmkay mmkay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muuuuch nicer, thanks @ca-scribner!

@PietroPasotti PietroPasotti merged commit deb4d2f into main Jun 20, 2024
13 checks passed
@PietroPasotti PietroPasotti deleted the charm_tracing-instance-attribute-support branch June 20, 2024 13:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants