diff --git a/charmcraft.yaml b/charmcraft.yaml index cd52c7e5..36637d50 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -20,6 +20,7 @@ parts: - ops - wheel==0.37.1 - setuptools==45.2.0 + - pydantic>=2 cos-tool: plugin: dump source: . diff --git a/lib/charms/tempo_k8s/v0/charm_tracing.py b/lib/charms/tempo_k8s/v1/charm_tracing.py similarity index 78% rename from lib/charms/tempo_k8s/v0/charm_tracing.py rename to lib/charms/tempo_k8s/v1/charm_tracing.py index efad1605..64ac0bd8 100644 --- a/lib/charms/tempo_k8s/v0/charm_tracing.py +++ b/lib/charms/tempo_k8s/v1/charm_tracing.py @@ -15,14 +15,17 @@ `@trace_charm(tracing_endpoint="my_tracing_endpoint")` 2) add to your charm a "my_tracing_endpoint" (you can name this attribute whatever you like) **property** -that returns an otlp grpc endpoint url. If you are using the `TracingEndpointProvider` as +that returns an otlp http/https endpoint url. If you are using the `TracingEndpointProvider` as `self.tracing = TracingEndpointProvider(self)`, the implementation could be: ``` @property def my_tracing_endpoint(self) -> Optional[str]: '''Tempo endpoint for charm tracing''' - return self.tracing.otlp_grpc_endpoint + if self.tracing.is_ready(): + return self.tracing.otlp_http_endpoint() + else: + return None ``` At this point your charm will be automatically instrumented so that: @@ -41,6 +44,62 @@ def tracer(self) -> opentelemetry.trace.Tracer: By default, the tracer is named after the charm type. If you wish to override that, you can pass a different `service_name` argument to `trace_charm`. + +*Upgrading from `v0`:* + +If you are upgrading from `charm_tracing` v0, you need to take the following steps (assuming you already +have the newest version of the library in your charm): +1) If you need the dependency for your tests, add the following dependency to your charm project +(or, if your project had a dependency on `opentelemetry-exporter-otlp-proto-grpc` only because +of `charm_tracing` v0, you can replace it with): + +`opentelemetry-exporter-otlp-proto-http>=1.21.0`. + +2) Update the charm method referenced to from `@trace` and `@trace_charm`, +to return from `TracingEndpointRequirer.otlp_http_endpoint()` instead of `grpc_http`. For example: + +``` + from charms.tempo_k8s.v0.charm_tracing import trace_charm + + @trace_charm( + tracing_endpoint="my_tracing_endpoint", + ) + class MyCharm(CharmBase): + + ... + + @property + def my_tracing_endpoint(self) -> Optional[str]: + '''Tempo endpoint for charm tracing''' + if self.tracing.is_ready(): + return self.tracing.otlp_grpc_endpoint() + else: + return None +``` + +needs to be replaced with: + +``` + from charms.tempo_k8s.v1.charm_tracing import trace_charm + + @trace_charm( + tracing_endpoint="my_tracing_endpoint", + ) + class MyCharm(CharmBase): + + ... + + @property + def my_tracing_endpoint(self) -> Optional[str]: + '''Tempo endpoint for charm tracing''' + if self.tracing.is_ready(): + return self.tracing.otlp_http_endpoint() + else: + return None +``` + +3) If you were passing a certificate using `server_cert`, you need to change it to provide an *absolute* path to +the certificate file. """ import functools @@ -48,7 +107,8 @@ def tracer(self) -> opentelemetry.trace.Tracer: import logging import os from contextlib import contextmanager -from contextvars import ContextVar +from contextvars import Context, ContextVar, copy_context +from pathlib import Path from typing import ( Any, Callable, @@ -62,8 +122,7 @@ def tracer(self) -> opentelemetry.trace.Tracer: ) import opentelemetry -from grpc import ChannelCredentials -from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter +from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import Span, TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor @@ -82,13 +141,14 @@ def tracer(self) -> opentelemetry.trace.Tracer: LIBID = "cb1705dcd1a14ca09b2e60187d1215c7" # Increment this major API version when introducing breaking changes -LIBAPI = 0 +LIBAPI = 1 # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 4 -PYDEPS = ["opentelemetry-exporter-otlp-proto-grpc==1.17.0"] +LIBPATCH = 1 + +PYDEPS = ["opentelemetry-exporter-otlp-proto-http>=1.21.0"] logger = logging.getLogger("tracing") @@ -127,11 +187,29 @@ def get_current_span() -> Union[Span, None]: return cast(Span, span) +def _get_tracer_from_context(ctx: Context) -> Optional[ContextVar]: + tracers = [v for v in ctx if v is not None and v.name == "tracer"] + if tracers: + return tracers[0] + return None + + def _get_tracer() -> Optional[Tracer]: + """Find tracer in context variable and as a fallback locate it in the full context.""" try: return tracer.get() except LookupError: - return None + try: + logger.debug("tracer was not found in context variable, looking up in default context") + ctx: Context = copy_context() + if context_tracer := _get_tracer_from_context(ctx): + return context_tracer.get() + else: + logger.debug("Couldn't find context var for tracer: span will be skipped") + return None + except LookupError as err: + logger.debug(f"Couldn't find tracer: span will be skipped, err: {err}") + return None @contextmanager @@ -141,6 +219,7 @@ def _span(name: str) -> Generator[Optional[Span], Any, Any]: with tracer.start_as_current_span(name) as span: yield cast(Span, span) else: + logger.debug("tracer not found") yield None @@ -175,8 +254,8 @@ def _get_tracing_endpoint(tracing_endpoint_getter, self, charm): f"got {tracing_endpoint} instead." ) else: - logger.debug(f"Setting up span exporter to endpoint: {tracing_endpoint}") - return tracing_endpoint + logger.debug(f"Setting up span exporter to endpoint: {tracing_endpoint}/v1/traces") + return f"{tracing_endpoint}/v1/traces" def _get_server_cert(server_cert_getter, self, charm): @@ -190,9 +269,9 @@ def _get_server_cert(server_cert_getter, self, charm): f"{charm}.{server_cert_getter} returned None; continuing with INSECURE connection." ) return - elif not isinstance(server_cert, str): - raise TypeError( - f"{charm}.{server_cert_getter} should return a valid tls cert (string); " + elif not Path(server_cert).is_absolute(): + raise ValueError( + f"{charm}.{server_cert_getter} should return a valid tls cert absolute path (string | Path)); " f"got {server_cert} instead." ) logger.debug("Certificate successfully retrieved.") # todo: some more validation? @@ -241,14 +320,14 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs): if not tracing_endpoint: return - server_cert: Optional[str] = ( + server_cert: Optional[Union[str, Path]] = ( _get_server_cert(server_cert_getter, self, charm) if server_cert_getter else None ) - credentials = ChannelCredentials(server_cert) if server_cert else None - insecure = None if credentials else True exporter = OTLPSpanExporter( - endpoint=tracing_endpoint, credentials=credentials, insecure=insecure, timeout=2 + endpoint=tracing_endpoint, + certificate_file=str(Path(server_cert).absolute()) if server_cert else None, + timeout=2, ) processor = BatchSpanProcessor(exporter) @@ -313,28 +392,31 @@ def trace_charm( method calls on instances of this class. Usage: - >>> from charms.tempo_k8s.v0.charm_tracing import trace_charm + >>> from charms.tempo_k8s.v1.charm_tracing import trace_charm >>> from charms.tempo_k8s.v1.tracing import TracingEndpointProvider >>> from ops import CharmBase >>> >>> @trace_charm( - >>> tracing_endpoint="tempo_otlp_grpc_endpoint", + >>> tracing_endpoint="tempo_otlp_http_endpoint", >>> ) >>> class MyCharm(CharmBase): >>> >>> def __init__(self, framework: Framework): >>> ... - >>> self.tempo = TracingEndpointProvider(self) + >>> self.tracing = TracingEndpointProvider(self) >>> >>> @property - >>> def tempo_otlp_grpc_endpoint(self) -> Optional[str]: - >>> return self.tempo.otlp_grpc_endpoint + >>> def tempo_otlp_http_endpoint(self) -> Optional[str]: + >>> if self.tracing.is_ready(): + >>> return self.tracing.otlp_http_endpoint() + >>> else: + >>> return None >>> :param server_cert: method or property on the charm type that returns an - optional tls certificate to be used when sending traces to a remote server. + optional absolute path to a tls certificate to be used when sending traces to a remote server. If it returns None, an _insecure_ connection will be used. :param tracing_endpoint: name of a property on the charm type that returns an - optional tempo url. If None, tracing will be effectively disabled. Else, traces will be + optional (fully resolvable) tempo url. If None, tracing will be effectively disabled. Else, traces will be pushed to that endpoint. :param service_name: service name tag to attach to all traces generated by this charm. Defaults to the juju application name this charm is deployed under. @@ -370,11 +452,11 @@ def _autoinstrument( Usage: - >>> from charms.tempo_k8s.v0.charm_tracing import _autoinstrument + >>> from charms.tempo_k8s.v1.charm_tracing import _autoinstrument >>> from ops.main import main >>> _autoinstrument( >>> MyCharm, - >>> tracing_endpoint_getter=MyCharm.tempo_otlp_grpc_endpoint, + >>> tracing_endpoint_getter=MyCharm.tempo_otlp_http_endpoint, >>> service_name="MyCharm", >>> extra_types=(Foo, Bar) >>> ) @@ -382,7 +464,8 @@ def _autoinstrument( :param charm_type: the CharmBase subclass to autoinstrument. :param server_cert_getter: method or property on the charm type that returns an - optional tls certificate to be used when sending traces to a remote server. + optional absolute path to a tls certificate to be used when sending traces to a remote server. + This needs to be a valid path to a certificate. :param tracing_endpoint_getter: method or property on the charm type that returns an optional tempo url. If None, tracing will be effectively disabled. Else, traces will be pushed to that endpoint. diff --git a/lib/charms/tempo_k8s/v0/tracing.py b/lib/charms/tempo_k8s/v1/tracing.py similarity index 84% rename from lib/charms/tempo_k8s/v0/tracing.py rename to lib/charms/tempo_k8s/v1/tracing.py index aa07afaa..79ddebf7 100644 --- a/lib/charms/tempo_k8s/v0/tracing.py +++ b/lib/charms/tempo_k8s/v1/tracing.py @@ -16,7 +16,7 @@ `tracing` interface. The `TracingEndpointRequirer` object may be instantiated as follows - from charms.tempo_k8s.v0.tracing import TracingEndpointRequirer + from charms.tempo_k8s.v1.tracing import TracingEndpointRequirer def __init__(self, *args): super().__init__(*args) @@ -48,7 +48,7 @@ def __init__(self, *args): For example a Tempo charm may instantiate the `TracingEndpointProvider` in its constructor as follows - from charms.tempo_k8s.v0.tracing import TracingEndpointProvider + from charms.tempo_k8s.v1.tracing import TracingEndpointProvider def __init__(self, *args): super().__init__(*args) @@ -83,19 +83,19 @@ def __init__(self, *args): ) from ops.framework import EventSource, Object from ops.model import ModelError, Relation -from pydantic import BaseModel +from pydantic import BaseModel, ConfigDict # The unique Charmhub library identifier, never change it LIBID = "12977e9aa0b34367903d8afeb8c3d85d" # Increment this major API version when introducing breaking changes -LIBAPI = 0 +LIBAPI = 1 # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 8 +LIBPATCH = 2 -PYDEPS = ["pydantic<2.0"] +PYDEPS = ["pydantic>=2"] logger = logging.getLogger(__name__) @@ -107,6 +107,7 @@ def __init__(self, *args): ] RawIngester = Tuple[IngesterProtocol, int] +BUILTIN_JUJU_KEYS = {"ingress-address", "private-address", "egress-subnets"} class TracingError(RuntimeError): @@ -121,37 +122,61 @@ class AmbiguousRelationUsageError(TracingError): """Raised when one wrongly assumes that there can only be one relation on an endpoint.""" -# todo: use fully-encoded json fields like Traefik does. MUCH neater class DatabagModel(BaseModel): """Base databag model.""" - _NEST_UNDER = None + model_config = ConfigDict( + # Allow instantiating this class by field name (instead of forcing alias). + populate_by_name=True, + # Custom config key: whether to nest the whole datastructure (as json) + # under a field or spread it out at the toplevel. + _NEST_UNDER=None, + ) # type: ignore + """Pydantic config.""" @classmethod def load(cls, databag: MutableMapping): """Load this model from a Juju databag.""" - if cls._NEST_UNDER: - return cls.parse_obj(json.loads(databag[cls._NEST_UNDER])) + nest_under = cls.model_config.get("_NEST_UNDER") + if nest_under: + return cls.parse_obj(json.loads(databag[nest_under])) - data = {k: json.loads(v) for k, v in databag.items()} + try: + data = {k: json.loads(v) for k, v in databag.items() if k not in BUILTIN_JUJU_KEYS} + except json.JSONDecodeError as e: + msg = f"invalid databag contents: expecting json. {databag}" + logger.error(msg) + raise DataValidationError(msg) from e try: return cls.parse_raw(json.dumps(data)) # type: ignore except pydantic.ValidationError as e: - msg = f"failed to validate remote unit databag: {databag}" + msg = f"failed to validate databag: {databag}" logger.error(msg, exc_info=True) raise DataValidationError(msg) from e - def dump(self, databag: MutableMapping): - """Write the contents of this model to Juju databag.""" - if self._NEST_UNDER: - databag[self._NEST_UNDER] = self.json() + def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True): + """Write the contents of this model to Juju databag. + + :param databag: the databag to write the data to. + :param clear: ensure the databag is cleared before writing it. + """ + if clear and databag: + databag.clear() - dct = self.dict() - for key, field in self.__fields__.items(): # type: ignore + if databag is None: + databag = {} + nest_under = self.model_config.get("_NEST_UNDER") + if nest_under: + databag[nest_under] = self.json() + + dct = self.model_dump() + for key, field in self.model_fields.items(): # type: ignore value = dct[key] databag[field.alias or key] = json.dumps(value) + return databag + # todo use models from charm-relation-interfaces class Ingester(BaseModel): # noqa: D101 @@ -522,13 +547,21 @@ def get_all_endpoints( return return TracingProviderAppData.load(relation.data[relation.app]) # type: ignore - def _get_ingester(self, relation: Optional[Relation], protocol: IngesterProtocol): + def _get_ingester( + self, relation: Optional[Relation], protocol: IngesterProtocol, ssl: bool = False + ): ep = self.get_all_endpoints(relation) if not ep: return None try: ingester: Ingester = next(filter(lambda i: i.protocol == protocol, ep.ingesters)) - return f"{ep.host}:{ingester.port}" + if ingester.protocol in ["otlp_grpc", "jaeger_grpc"]: + if ssl: + logger.warning("unused ssl argument - was the right protocol called?") + return f"{ep.host}:{ingester.port}" + if ssl: + return f"https://{ep.host}:{ingester.port}" + return f"http://{ep.host}:{ingester.port}" except StopIteration: logger.error(f"no ingester found with protocol={protocol!r}") return None @@ -537,21 +570,41 @@ def otlp_grpc_endpoint(self, relation: Optional[Relation] = None) -> Optional[st """Ingester endpoint for the ``otlp_grpc`` protocol.""" return self._get_ingester(relation or self._relation, protocol="otlp_grpc") - def otlp_http_endpoint(self, relation: Optional[Relation] = None) -> Optional[str]: - """Ingester endpoint for the ``otlp_http`` protocol.""" - return self._get_ingester(relation or self._relation, protocol="otlp_http") + def otlp_http_endpoint( + self, relation: Optional[Relation] = None, ssl: bool = False + ) -> Optional[str]: + """Ingester endpoint for the ``otlp_http`` protocol. - def zipkin_endpoint(self, relation: Optional[Relation] = None) -> Optional[str]: - """Ingester endpoint for the ``zipkin`` protocol.""" - return self._get_ingester(relation or self._relation, protocol="zipkin") + The provided endpoint does not contain the endpoint suffix. If the instrumenting library needs the full path, + your endpoint code needs to add the ``/v1/traces`` suffix. + """ + return self._get_ingester(relation or self._relation, protocol="otlp_http", ssl=ssl) + + def zipkin_endpoint( + self, relation: Optional[Relation] = None, ssl: bool = False + ) -> Optional[str]: + """Ingester endpoint for the ``zipkin`` protocol. + + The provided endpoint does not contain the endpoint suffix. If the instrumenting library needs the full path, + your endpoint code needs to add the ``/api/v2/spans`` suffix. + """ + return self._get_ingester(relation or self._relation, protocol="zipkin", ssl=ssl) - def tempo_endpoint(self, relation: Optional[Relation] = None) -> Optional[str]: + def tempo_endpoint( + self, relation: Optional[Relation] = None, ssl: bool = False + ) -> Optional[str]: """Ingester endpoint for the ``tempo`` protocol.""" - return self._get_ingester(relation or self._relation, protocol="tempo") + return self._get_ingester(relation or self._relation, protocol="tempo", ssl=ssl) + + def jaeger_http_thrift_endpoint( + self, relation: Optional[Relation] = None, ssl: bool = False + ) -> Optional[str]: + """Ingester endpoint for the ``jaeger_http_thrift`` protocol. - def jaeger_http_thrift_endpoint(self, relation: Optional[Relation] = None) -> Optional[str]: - """Ingester endpoint for the ``jaeger_http_thrift`` protocol.""" - return self._get_ingester(relation or self._relation, "jaeger_http_thrift") + The provided endpoint does not contain the endpoint suffix. If the instrumenting library needs the full path, + your endpoint code needs to add the ``/api/traces`` suffix. + """ + return self._get_ingester(relation or self._relation, "jaeger_http_thrift", ssl=ssl) def jaeger_grpc_endpoint(self, relation: Optional[Relation] = None) -> Optional[str]: """Ingester endpoint for the ``jaeger_grpc`` protocol.""" diff --git a/pyproject.toml b/pyproject.toml index b90567ed..9613c75f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,8 +6,8 @@ version = "0.1" # not relevant [project.optional-dependencies] lib_pydeps = [ - "opentelemetry-exporter-otlp-proto-grpc", - "pydantic<2" + "opentelemetry-exporter-otlp-proto-http", + "pydantic>=2" ] # Testing tools configuration diff --git a/src/charm.py b/src/charm.py index b0a51de6..4d267feb 100755 --- a/src/charm.py +++ b/src/charm.py @@ -35,8 +35,8 @@ from charms.prometheus_k8s.v1.prometheus_remote_write import ( PrometheusRemoteWriteProvider, ) -from charms.tempo_k8s.v0.charm_tracing import trace_charm -from charms.tempo_k8s.v0.tracing import TracingEndpointRequirer +from charms.tempo_k8s.v1.charm_tracing import trace_charm +from charms.tempo_k8s.v1.tracing import TracingEndpointRequirer from charms.traefik_k8s.v1.ingress_per_unit import ( IngressPerUnitReadyForUnitEvent, IngressPerUnitRequirer, @@ -116,6 +116,7 @@ def to_status(tpl: Tuple[str, str]) -> StatusBase: @trace_charm( tracing_endpoint="tempo", + server_cert="server_cert_path", extra_types=[ KubernetesComputeResourcesPatch, CertHandler, @@ -1061,7 +1062,12 @@ def _push(self, path, contents): @property def tempo(self) -> Optional[str]: """Tempo endpoint for charm tracing.""" - return self.tracing.otlp_grpc_endpoint() + return self.tracing.otlp_http_endpoint() + + @property + def server_cert_path(self) -> Optional[str]: + """Server certificate path for TLS tracing.""" + return CERT_PATH if __name__ == "__main__": diff --git a/tests/unit/test_endpoint_consumer.py b/tests/unit/test_endpoint_consumer.py index a8957a8a..f07c5a34 100644 --- a/tests/unit/test_endpoint_consumer.py +++ b/tests/unit/test_endpoint_consumer.py @@ -493,11 +493,13 @@ def test_consumer_accepts_rules_with_no_identifier(self): with self.assertLogs(level="DEBUG") as logger: _ = self.harness.charm.prometheus_consumer.alerts messages = logger.output - self.assertIn( + + searched_message = ( "No labeled alert rules were found, and no 'scrape_metadata' " - "was available. Using the alert group name as filename.", - messages[1], + "was available. Using the alert group name as filename." ) + any_matches = any(searched_message in log_message for log_message in messages) + self.assertTrue(any_matches) alerts = self.harness.charm.prometheus_consumer.alerts identifier = f"unlabeled_external_cpu_alerts_{RELATION_NAME}_{rel_id}" self.assertIn(identifier, alerts.keys()) diff --git a/tox.ini b/tox.ini index dc8ec5d8..0ca3cdca 100644 --- a/tox.ini +++ b/tox.ini @@ -96,9 +96,9 @@ deps = pytest ops-scenario >=5.1,<6.0 -r{toxinidir}/requirements.txt - opentelemetry-exporter-otlp-proto-grpc==1.17.0 # PYDEPS for tracing + opentelemetry-exporter-otlp-proto-http==1.21.0 # PYDEPS for tracing importlib-metadata==6.0.0 # PYDEPS for tracing - pydantic # PYDEPS for tracing + pydantic>=2 # PYDEPS for tracing commands = pytest -v --tb native {[vars]tst_path}/scenario --log-cli-level=INFO -s {posargs}