Merge pull request #29 from canonical/retention-period

Retention period

charmcraft.yaml

@@ -129,10 +129,10 @@ parts:
 
 config:
   options:
-    retention_period_hours:
+    retention-period:
       description: |
         Maximum trace retention period, in hours. This will be used to configure the compactor to clean up trace data after this time. 
-        Defaults to 720 hours, which is equivalent to 30 days.
+        Defaults to 720 hours, which is equivalent to 30 days. Per-stream retention limits are currently not supported.
       type: int
       default: 720
     always_enable_zipkin:

lib/charms/tempo_k8s/v1/charm_tracing.py

@@ -172,14 +172,64 @@ 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
 import os
-import shutil
 from contextlib import contextmanager
 from contextvars import Context, ContextVar, copy_context
-from importlib.metadata import distributions
 from pathlib import Path
 from typing import (
     Any,
@@ -219,7 +269,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 = 13
+LIBPATCH = 15
 
 PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"]
 
@@ -229,7 +279,6 @@ def my_tracing_endpoint(self) -> Optional[str]:
 # set this to 0 if you are debugging/developing this library source
 dev_logger.setLevel(logging.CRITICAL)
 
-
 _CharmType = Type[CharmBase]  # the type CharmBase and any subclass thereof
 _C = TypeVar("_C", bound=_CharmType)
 _T = TypeVar("_T", bound=type)
@@ -281,9 +330,22 @@ def _get_tracer() -> Optional[Tracer]:
     try:
         return tracer.get()
     except LookupError:
+        # fallback: this course-corrects for a user error where charm_tracing symbols are imported
+        # from different paths (typically charms.tempo_k8s... and lib.charms.tempo_k8s...)
         try:
             ctx: Context = copy_context()
             if context_tracer := _get_tracer_from_context(ctx):
+                logger.warning(
+                    "Tracer not found in `tracer` context var. "
+                    "Verify that you're importing all `charm_tracing` symbols from the same module path. \n"
+                    "For example, DO"
+                    ": `from charms.lib...charm_tracing import foo, bar`. \n"
+                    "DONT: \n"
+                    " \t - `from charms.lib...charm_tracing import foo` \n"
+                    " \t - `from lib...charm_tracing import bar` \n"
+                    "For more info: https://python-notes.curiousefficiency.org/en/latest/python"
+                    "_concepts/import_traps.html#the-double-import-trap"
+                )
                 return context_tracer.get()
             else:
                 return None
@@ -361,30 +423,6 @@ def _get_server_cert(
     return server_cert
 
 
-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.
-
-    This only does something if executed on an upgrade-charm event.
-    """
-    if os.getenv("JUJU_DISPATCH_PATH") == "hooks/upgrade-charm":
-        logger.debug("Executing _remove_stale_otel_sdk_packages patch on charm upgrade")
-        # Find any opentelemetry_sdk distributions
-        otel_sdk_distributions = list(distributions(name="opentelemetry_sdk"))
-        # If there is more than 1, inspect each and if it has 0 entrypoints, infer that it is stale
-        if len(otel_sdk_distributions) > 1:
-            for distribution in otel_sdk_distributions:
-                if len(distribution.entry_points) == 0:
-                    # Distribution appears to be empty. Remove it
-                    path = distribution._path  # type: ignore
-                    logger.debug(f"Removing empty opentelemetry_sdk distribution at: {path}")
-                    shutil.rmtree(path)
-
-
 def _setup_root_span_initializer(
     charm_type: _CharmType,
     tracing_endpoint_attr: str,
@@ -420,7 +458,6 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs):
         # 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.
-        _remove_stale_otel_sdk_packages()
         resource = Resource.create(
             attributes={
                 "service.name": _service_name,

lib/charms/traefik_k8s/v2/ingress.py

@@ -56,13 +56,14 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent):
 import socket
 import typing
 from dataclasses import dataclass
+from functools import partial
 from typing import Any, Callable, Dict, List, MutableMapping, Optional, Sequence, Tuple, Union
 
 import pydantic
 from ops.charm import CharmBase, RelationBrokenEvent, RelationEvent
 from ops.framework import EventSource, Object, ObjectEvents, StoredState
 from ops.model import ModelError, Relation, Unit
-from pydantic import AnyHttpUrl, BaseModel, Field, validator
+from pydantic import AnyHttpUrl, BaseModel, Field
 
 # The unique Charmhub library identifier, never change it
 LIBID = "e6de2a5cd5b34422a204668f3b8f90d2"
@@ -72,7 +73,7 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 13
+LIBPATCH = 14
 
 PYDEPS = ["pydantic"]
 
@@ -84,6 +85,9 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent):
 
 PYDANTIC_IS_V1 = int(pydantic.version.VERSION.split(".")[0]) < 2
 if PYDANTIC_IS_V1:
+    from pydantic import validator
+
+    input_validator = partial(validator, pre=True)
 
     class DatabagModel(BaseModel):  # type: ignore
         """Base databag model."""
@@ -143,7 +147,9 @@ def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True):
             return databag
 
 else:
-    from pydantic import ConfigDict
+    from pydantic import ConfigDict, field_validator
+
+    input_validator = partial(field_validator, mode="before")
 
     class DatabagModel(BaseModel):
         """Base databag model."""
@@ -171,7 +177,7 @@ def load(cls, databag: MutableMapping):
                     k: json.loads(v)
                     for k, v in databag.items()
                     # Don't attempt to parse model-external values
-                    if k in {(f.alias or n) for n, f in cls.__fields__.items()}  # type: ignore
+                    if k in {(f.alias or n) for n, f in cls.model_fields.items()}  # type: ignore
                 }
             except json.JSONDecodeError as e:
                 msg = f"invalid databag contents: expecting json. {databag}"
@@ -252,14 +258,14 @@ class IngressRequirerAppData(DatabagModel):
         default="http", description="What scheme to use in the generated ingress url"
     )
 
-    @validator("scheme", pre=True)
+    @input_validator("scheme")
     def validate_scheme(cls, scheme):  # noqa: N805  # pydantic wants 'cls' as first arg
         """Validate scheme arg."""
         if scheme not in {"http", "https", "h2c"}:
             raise ValueError("invalid scheme: should be one of `http|https|h2c`")
         return scheme
 
-    @validator("port", pre=True)
+    @input_validator("port")
     def validate_port(cls, port):  # noqa: N805  # pydantic wants 'cls' as first arg
         """Validate port."""
         assert isinstance(port, int), type(port)
@@ -277,13 +283,13 @@ class IngressRequirerUnitData(DatabagModel):
         "IP can only be None if the IP information can't be retrieved from juju.",
     )
 
-    @validator("host", pre=True)
+    @input_validator("host")
     def validate_host(cls, host):  # noqa: N805  # pydantic wants 'cls' as first arg
         """Validate host."""
         assert isinstance(host, str), type(host)
         return host
 
-    @validator("ip", pre=True)
+    @input_validator("ip")
     def validate_ip(cls, ip):  # noqa: N805  # pydantic wants 'cls' as first arg
         """Validate ip."""
         if ip is None:
@@ -462,7 +468,10 @@ def _handle_relation(self, event):
                 event.relation,
                 data.app.name,
                 data.app.model,
-                [unit.dict() for unit in data.units],
+                [
+                    unit.dict() if PYDANTIC_IS_V1 else unit.model_dump(mode="json")
+                    for unit in data.units
+                ],
                 data.app.strip_prefix or False,
                 data.app.redirect_https or False,
             )

pyproject.toml

@@ -54,7 +54,12 @@ extend-ignore = [
 ]
 ignore = ["E501", "D107"]
 extend-exclude = ["__pycache__", "*.egg_info", "*integration/tester*"]
-per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104"]}
+
+[tool.ruff.lint.per-file-ignores]
+"tests/*" = ["D100","D101","D102","D103","D104"]
+# Remove charm_tracing.py E402 when _remove_stale_otel_sdk_packages() is removed
+# from the library
+"lib/charms/tempo_k8s/v1/charm_tracing.py" = ["E402"]
 
 [lint.mccabe]
 max-complexity = 10

requirements.txt

@@ -3,8 +3,8 @@ importlib-metadata~=6.0.0
 ops
 crossplane
 jsonschema==4.17.0
-lightkube==0.15.4
-lightkube-models==1.24.1.4
+lightkube>=0.15.4
+lightkube-models>=1.24.1.4
 tenacity==8.2.3
 # crossplane is a package from nginxinc to interact with the Nginx config
 crossplane