[DPE-4266] Strip passwords from command execute output and tracebacks (#473)

* Strip passwords from command execute output and tracebacks

* Avoid password leak upon ChangeError + do not forward context in _run_mysqlsh_script errors

* Remove unnecessary from None for backup exceptions

* Update outdated charm libs

* Update outdated mysql charm lib

Author: shayancanonical

lib/charms/data_platform_libs/v0/data_interfaces.py

@@ -54,7 +54,7 @@
 # The unique Charmhub library identifier, never change it
 LIBID = "4de21f1a022c4e2c87ac8e672ec16f6a"
 LIBAPI = 0
-LIBPATCH = 5
+LIBPATCH = 6
 
 RELATION_OFFER = "replication-offer"
 RELATION_CONSUMER = "replication"
@@ -778,9 +778,10 @@ def _on_consumer_changed(self, event):  # noqa: C901
                 logger.debug("Recreating cluster prior to sync credentials")
                 self._charm.create_cluster()
                 # (re)set flags
-                self._charm.app_peer_data.update(
-                    {"removed-from-cluster-set": "", "rejoin-secondaries": "true"}
-                )
+                self._charm.app_peer_data.update({
+                    "removed-from-cluster-set": "",
+                    "rejoin-secondaries": "true",
+                })
                 event.defer()
                 return
             if not self._charm.cluster_fully_initialized:

lib/charms/mysql/v0/async_replication.py

@@ -99,7 +99,7 @@ def is_unit_blocked(self) -> bool:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 10
+LIBPATCH = 11
 
 
 if typing.TYPE_CHECKING:
@@ -314,11 +314,9 @@ def _on_create_backup(self, event: ActionEvent) -> None:
             return
 
         logger.info(f"Backup succeeded: with backup-id {datetime_backup_requested}")
-        event.set_results(
-            {
-                "backup-id": datetime_backup_requested,
-            }
-        )
+        event.set_results({
+            "backup-id": datetime_backup_requested,
+        })
         self.charm._on_update_status(None)
 
     def _can_unit_perform_backup(self) -> Tuple[bool, Optional[str]]:
@@ -538,11 +536,9 @@ def _on_restore(self, event: ActionEvent) -> None:
             return
 
         logger.info("Restore succeeded")
-        event.set_results(
-            {
-                "completed": "ok",
-            }
-        )
+        event.set_results({
+            "completed": "ok",
+        })
         # update status as soon as possible
         self.charm._on_update_status(None)
 

lib/charms/mysql/v0/backups.py

@@ -134,7 +134,7 @@ def wait_until_mysql_connection(self) -> None:
 # Increment this major API version when introducing breaking changes
 LIBAPI = 0
 
-LIBPATCH = 66
+LIBPATCH = 67
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -874,6 +874,13 @@ def __init__(
         self.monitoring_password = monitoring_password
         self.backups_user = backups_user
         self.backups_password = backups_password
+        self.passwords = [
+            self.root_password,
+            self.server_config_password,
+            self.cluster_admin_password,
+            self.monitoring_password,
+            self.backups_password,
+        ]
 
     def render_mysqld_configuration(  # noqa: C901
         self,
@@ -2532,7 +2539,7 @@ def execute_backup_commands(
         except Exception:
             # Catch all other exceptions to prevent the database being stuck in
             # a bad state due to pre-backup operations
-            logger.exception("Failed to execute commands prior to running backup")
+            logger.exception("Failed unexpectedly to execute commands prior to running backup")
             raise MySQLExecuteBackupCommandsError
 
         # TODO: remove flags --no-server-version-check
@@ -2582,13 +2589,13 @@ def execute_backup_commands(
                 },
                 stream_output="stderr",
             )
-        except MySQLExecError as e:
+        except MySQLExecError:
             logger.exception("Failed to execute backup commands")
-            raise MySQLExecuteBackupCommandsError(e.message)
+            raise MySQLExecuteBackupCommandsError
         except Exception:
             # Catch all other exceptions to prevent the database being stuck in
             # a bad state due to pre-backup operations
-            logger.exception("Failed to execute backup commands")
+            logger.exception("Failed unexpectedly to execute backup commands")
             raise MySQLExecuteBackupCommandsError
 
     def delete_temp_backup_directory(
@@ -2978,6 +2985,13 @@ def get_non_system_databases(self) -> set[str]:
             "sys",
         }
 
+    def strip_off_passwords(self, input_string: str) -> str:
+        """Strips off passwords from the input string."""
+        stripped_input = input_string
+        for password in self.passwords:
+            stripped_input = stripped_input.replace(password, "xxxxxxxxxxxx")
+        return stripped_input
+
     @abstractmethod
     def is_mysqld_running(self) -> bool:
         """Returns whether mysqld is running."""

lib/charms/mysql/v0/mysql.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 """S3 helper functions for the MySQL charms."""
+
 import base64
 import logging
 import tempfile
@@ -31,7 +32,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 8
+LIBPATCH = 9
 
 # botocore/urllib3 clutter the logs when on debug
 logging.getLogger("botocore").setLevel(logging.WARNING)

lib/charms/mysql/v0/s3_helpers.py

@@ -19,7 +19,6 @@
 
 """
 
-
 import base64
 import logging
 import re
@@ -52,7 +51,7 @@
 
 LIBID = "eb73947deedd4380a3a90d527e0878eb"
 LIBAPI = 0
-LIBPATCH = 6
+LIBPATCH = 7
 
 SCOPE = "unit"
 

lib/charms/mysql/v0/tls.py

@@ -172,6 +172,59 @@ def my_tracing_endpoint(self) -> Optional[str]:
 provide an *absolute* path to the certificate file instead.
 """
 
+
+def _remove_stale_otel_sdk_packages():
+    """Hack to remove stale opentelemetry sdk packages from the charm's python venv.
+
+    See https://github.com/canonical/grafana-agent-operator/issues/146 and
+    https://bugs.launchpad.net/juju/+bug/2058335 for more context. This patch can be removed after
+    this juju issue is resolved and sufficient time has passed to expect most users of this library
+    have migrated to the patched version of juju.  When this patch is removed, un-ignore rule E402 for this file in the pyproject.toml (see setting
+    [tool.ruff.lint.per-file-ignores] in pyproject.toml).
+
+    This only has an effect if executed on an upgrade-charm event.
+    """
+    # all imports are local to keep this function standalone, side-effect-free, and easy to revert later
+    import os
+
+    if os.getenv("JUJU_DISPATCH_PATH") != "hooks/upgrade-charm":
+        return
+
+    import logging
+    import shutil
+    from collections import defaultdict
+
+    from importlib_metadata import distributions
+
+    otel_logger = logging.getLogger("charm_tracing_otel_patcher")
+    otel_logger.debug("Applying _remove_stale_otel_sdk_packages patch on charm upgrade")
+    # group by name all distributions starting with "opentelemetry_"
+    otel_distributions = defaultdict(list)
+    for distribution in distributions():
+        name = distribution._normalized_name  # type: ignore
+        if name.startswith("opentelemetry_"):
+            otel_distributions[name].append(distribution)
+
+    otel_logger.debug(f"Found {len(otel_distributions)} opentelemetry distributions")
+
+    # If we have multiple distributions with the same name, remove any that have 0 associated files
+    for name, distributions_ in otel_distributions.items():
+        if len(distributions_) <= 1:
+            continue
+
+        otel_logger.debug(f"Package {name} has multiple ({len(distributions_)}) distributions.")
+        for distribution in distributions_:
+            if not distribution.files:  # Not None or empty list
+                path = distribution._path  # type: ignore
+                otel_logger.info(f"Removing empty distribution of {name} at {path}.")
+                shutil.rmtree(path)
+
+    otel_logger.debug("Successfully applied _remove_stale_otel_sdk_packages patch. ")
+
+
+_remove_stale_otel_sdk_packages()
+
+
 import functools
 import inspect
 import logging
@@ -197,14 +250,15 @@ 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
 
@@ -217,7 +271,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 = 11
+LIBPATCH = 14
 
 PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"]
 
@@ -391,6 +445,9 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs):
         _service_name = service_name or f"{self.app.name}-charm"
 
         unit_name = self.unit.name
+        # apply hacky patch to remove stale opentelemetry sdk packages on upgrade-charm.
+        # it could be trouble if someone ever decides to implement their own tracer parallel to
+        # ours and before the charm has inited. We assume they won't.
         resource = Resource.create(
             attributes={
                 "service.name": _service_name,
@@ -612,38 +669,58 @@ def trace_type(cls: _T) -> _T:
             dev_logger.info(f"skipping {method} (dunder)")
             continue
 
-        new_method = trace_method(method)
-        if isinstance(inspect.getattr_static(cls, method.__name__), staticmethod):
+        # the span title in the general case should be:
+        #   method call: MyCharmWrappedMethods.b
+        # if the method has a name (functools.wrapped or regular method), let
+        # _trace_callable use its default algorithm to determine what name to give the span.
+        trace_method_name = None
+        try:
+            qualname_c0 = method.__qualname__.split(".")[0]
+            if not hasattr(cls, method.__name__):
+                # if the callable doesn't have a __name__ (probably a decorated method),
+                # it probably has a bad qualname too (such as my_decorator.<locals>.wrapper) which is not
+                # great for finding out what the trace is about. So we use the method name instead and
+                # add a reference to the decorator name. Result:
+                #   method call: @my_decorator(MyCharmWrappedMethods.b)
+                trace_method_name = f"@{qualname_c0}({cls.__name__}.{name})"
+        except Exception:  # noqa: failsafe
+            pass
+
+        new_method = trace_method(method, name=trace_method_name)
+
+        if isinstance(inspect.getattr_static(cls, name), staticmethod):
             new_method = staticmethod(new_method)
         setattr(cls, name, new_method)
 
     return cls
 
 
-def trace_method(method: _F) -> _F:
+def trace_method(method: _F, name: Optional[str] = None) -> _F:
     """Trace this method.
 
     A span will be opened when this method is called and closed when it returns.
     """
-    return _trace_callable(method, "method")
+    return _trace_callable(method, "method", name=name)
 
 
-def trace_function(function: _F) -> _F:
+def trace_function(function: _F, name: Optional[str] = None) -> _F:
     """Trace this function.
 
     A span will be opened when this function is called and closed when it returns.
     """
-    return _trace_callable(function, "function")
+    return _trace_callable(function, "function", name=name)
 
 
-def _trace_callable(callable: _F, qualifier: str) -> _F:
+def _trace_callable(callable: _F, qualifier: str, name: Optional[str] = None) -> _F:
     dev_logger.info(f"instrumenting {callable}")
 
     # sig = inspect.signature(callable)
     @functools.wraps(callable)
     def wrapped_function(*args, **kwargs):  # type: ignore
-        name = getattr(callable, "__qualname__", getattr(callable, "__name__", str(callable)))
-        with _span(f"{qualifier} call: {name}"):  # type: ignore
+        name_ = name or getattr(
+            callable, "__qualname__", getattr(callable, "__name__", str(callable))
+        )
+        with _span(f"{qualifier} call: {name_}"):  # type: ignore
             return callable(*args, **kwargs)  # type: ignore
 
     # wrapped_function.__signature__ = sig

lib/charms/tempo_k8s/v1/charm_tracing.py

@@ -107,7 +107,7 @@ def __init__(self, *args):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 7
+LIBPATCH = 8
 
 PYDEPS = ["pydantic"]
 
@@ -116,14 +116,13 @@ def __init__(self, *args):
 DEFAULT_RELATION_NAME = "tracing"
 RELATION_INTERFACE_NAME = "tracing"
 
+# Supported list rationale https://github.com/canonical/tempo-coordinator-k8s-operator/issues/8
 ReceiverProtocol = Literal[
     "zipkin",
-    "kafka",
-    "opencensus",
-    "tempo_http",
-    "tempo_grpc",
     "otlp_grpc",
     "otlp_http",
+    "jaeger_grpc",
+    "jaeger_thrift_http",
 ]
 
 RawReceiver = Tuple[ReceiverProtocol, str]
@@ -141,14 +140,12 @@ class TransportProtocolType(str, enum.Enum):
     grpc = "grpc"
 
 
-receiver_protocol_to_transport_protocol = {
+receiver_protocol_to_transport_protocol: Dict[ReceiverProtocol, TransportProtocolType] = {
     "zipkin": TransportProtocolType.http,
-    "kafka": TransportProtocolType.http,
-    "opencensus": TransportProtocolType.http,
-    "tempo_http": TransportProtocolType.http,
-    "tempo_grpc": TransportProtocolType.grpc,
     "otlp_grpc": TransportProtocolType.grpc,
     "otlp_http": TransportProtocolType.http,
+    "jaeger_thrift_http": TransportProtocolType.http,
+    "jaeger_grpc": TransportProtocolType.grpc,
 }
 """A mapping between telemetry protocols and their corresponding transport protocol.
 """