diff --git a/lib/charms/tempo_k8s/v1/charm_tracing.py b/lib/charms/tempo_k8s/v1/charm_tracing.py index 000e0cb57..dc84e3f4e 100644 --- a/lib/charms/tempo_k8s/v1/charm_tracing.py +++ b/lib/charms/tempo_k8s/v1/charm_tracing.py @@ -126,15 +126,14 @@ def my_tracing_endpoint(self) -> Optional[str]: from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import Span, TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor +from opentelemetry.trace import INVALID_SPAN, Tracer +from opentelemetry.trace import get_current_span as otlp_get_current_span from opentelemetry.trace import ( - INVALID_SPAN, - Tracer, get_tracer, get_tracer_provider, set_span_in_context, set_tracer_provider, ) -from opentelemetry.trace import get_current_span as otlp_get_current_span from ops.charm import CharmBase from ops.framework import Framework @@ -147,7 +146,7 @@ def my_tracing_endpoint(self) -> Optional[str]: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 8 +LIBPATCH = 10 PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"] @@ -295,16 +294,17 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs): # self.handle = Handle(None, self.handle_kind, None) original_event_context = framework._event_context + # default service name isn't just app name because it could conflict with the workload service name + _service_name = service_name or f"{self.app.name}-charm" - _service_name = service_name or self.app.name - + unit_name = self.unit.name resource = Resource.create( attributes={ "service.name": _service_name, "compose_service": _service_name, "charm_type": type(self).__name__, # juju topology - "juju_unit": self.unit.name, + "juju_unit": unit_name, "juju_application": self.app.name, "juju_model": self.model.name, "juju_model_uuid": self.model.uuid, @@ -342,16 +342,18 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs): _tracer = get_tracer(_service_name) # type: ignore _tracer_token = tracer.set(_tracer) - dispatch_path = os.getenv("JUJU_DISPATCH_PATH", "") + dispatch_path = os.getenv("JUJU_DISPATCH_PATH", "") # something like hooks/install + event_name = dispatch_path.split("/")[1] if "/" in dispatch_path else dispatch_path + root_span_name = f"{unit_name}: {event_name} event" + span = _tracer.start_span(root_span_name, attributes={"juju.dispatch_path": dispatch_path}) # all these shenanigans are to work around the fact that the opentelemetry tracing API is built # on the assumption that spans will be used as contextmanagers. # Since we don't (as we need to close the span on framework.commit), # we need to manually set the root span as current. - span = _tracer.start_span("charm exec", attributes={"juju.dispatch_path": dispatch_path}) ctx = set_span_in_context(span) - # log a trace id so we can look it up in tempo. + # log a trace id, so we can pick it up from the logs (and jhack) to look it up in tempo. root_trace_id = hex(span.get_span_context().trace_id)[2:] # strip 0x prefix logger.debug(f"Starting root trace with id={root_trace_id!r}.") diff --git a/src/charm.py b/src/charm.py index b87dfb871..f921ee523 100755 --- a/src/charm.py +++ b/src/charm.py @@ -819,6 +819,7 @@ def render_auth_file(self, auth_file: str, reload_pgbouncer: bool = False): f"{PGB_CONF_DIR}/{self.app.name}/{AUTH_FILE_NAME}", auth_file, perms=0o700 ) self.unit.status = initial_status + self.peers.unit_databag["auth_file_set"] = "true" if reload_pgbouncer: self.reload_pgbouncer() diff --git a/src/relations/peers.py b/src/relations/peers.py index 68a0602c4..7c74e2f2f 100644 --- a/src/relations/peers.py +++ b/src/relations/peers.py @@ -26,6 +26,7 @@ """ # noqa: W505 import logging +from hashlib import shake_128 from typing import List, Optional, Set from ops.charm import CharmBase, HookEvent @@ -60,7 +61,6 @@ def __init__(self, charm: CharmBase): self.framework.observe(charm.on[PEER_RELATION_NAME].relation_joined, self._on_joined) self.framework.observe(charm.on[PEER_RELATION_NAME].relation_changed, self._on_changed) self.framework.observe(charm.on.secret_changed, self._on_changed) - self.framework.observe(charm.on.secret_remove, self._on_changed) self.framework.observe(charm.on[PEER_RELATION_NAME].relation_departed, self._on_departed) @property @@ -133,7 +133,11 @@ def _on_changed(self, event: HookEvent): if self.charm.backend.postgres: self.charm.render_prometheus_service() + pgb_dbs_hash = shake_128(self.app_databag.get("pgb_dbs_config", "{}").encode()).hexdigest( + 16 + ) self.charm.render_pgb_config(reload_pgbouncer=True) + self.unit_databag["pgb_dbs"] = pgb_dbs_hash def _on_departed(self, _): self.update_leader() diff --git a/src/relations/pgbouncer_provider.py b/src/relations/pgbouncer_provider.py index a4dec1276..d9eab5546 100644 --- a/src/relations/pgbouncer_provider.py +++ b/src/relations/pgbouncer_provider.py @@ -36,6 +36,7 @@ """ # noqa: W505 import logging +from hashlib import shake_128 from charms.data_platform_libs.v0.data_interfaces import ( DatabaseProvides, @@ -110,11 +111,36 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: event.defer() return + for key in self.charm.peers.relation.data.keys(): + if key != self.charm.app: + if self.charm.peers.relation.data[key].get("auth_file_set", "false") != "true": + logger.debug("Backend credentials not yet set on all units") + event.defer() + return + # Retrieve the database name and extra user roles using the charm library. database = event.database extra_user_roles = event.extra_user_roles or "" rel_id = event.relation.id + dbs = self.charm.generate_relation_databases() + dbs[str(event.relation.id)] = {"name": database, "legacy": False} + roles = extra_user_roles.lower().split(",") + if "admin" in roles or "superuser" in roles: + dbs["*"] = {"name": "*", "auth_dbname": database} + self.charm.set_relation_databases(dbs) + + pgb_dbs_hash = shake_128( + self.charm.peers.app_databag["pgb_dbs_config"].encode() + ).hexdigest(16) + for key in self.charm.peers.relation.data.keys(): + # We skip the leader so we don't have to wait on the defer + if key != self.charm.app and key != self.charm.unit: + if self.charm.peers.relation.data[key].get("pgb_dbs", "") != pgb_dbs_hash: + logger.debug("Not all units have synced configuration") + event.defer() + return + # Creates the user and the database for this specific relation. user = f"relation_id_{rel_id}" logger.debug("generating relation user") @@ -141,12 +167,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: ) return - dbs = self.charm.generate_relation_databases() - dbs[str(event.relation.id)] = {"name": database, "legacy": False} - roles = extra_user_roles.lower().split(",") - if "admin" in roles or "superuser" in roles: - dbs["*"] = {"name": "*", "auth_dbname": database} - self.charm.set_relation_databases(dbs) + self.charm.render_pgb_config(reload_pgbouncer=True) # Share the credentials and updated connection info with the client application. self.database_provides.set_credentials(rel_id, user, password) diff --git a/tests/unit/relations/test_pgbouncer_provider.py b/tests/unit/relations/test_pgbouncer_provider.py index f930e5693..a4178e13d 100644 --- a/tests/unit/relations/test_pgbouncer_provider.py +++ b/tests/unit/relations/test_pgbouncer_provider.py @@ -100,8 +100,19 @@ def test_on_database_requested( self.client_relation._on_database_requested(event) _pg.create_user.assert_not_called() + # check we exit immediately if not all units are set. _check_backend.return_value = True self.client_relation._on_database_requested(event) + _pg.create_user.assert_not_called() + + with self.harness.hooks_disabled(): + self.harness.update_relation_data( + self.peers_rel_id, self.unit, {"auth_file_set": "true"} + ) + self.harness.update_relation_data( + self.peers_rel_id, self.app, {"pgb_dbs_config": "{}"} + ) + self.client_relation._on_database_requested(event) # Verify we've called everything we should _pg().create_user.assert_called_with(