[DPE-4595] Immediate connection on database created (#257)

* Immediate connection on database created

* Sync configs between units (#258)

* Sync configs between units

* Remove readonly db hash

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}.")
 

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()

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()

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)

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(