Skip to content

Commit

Permalink
charm tracing fails over tls (#331)
Browse files Browse the repository at this point in the history
* fix charm tracing cert

* fix static check

* pytest version fix

* latest lib
  • Loading branch information
michaeldmitry committed May 21, 2024
1 parent 23c37dc commit ec74910
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 24 deletions.
7 changes: 4 additions & 3 deletions lib/charms/tempo_k8s/v1/charm_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,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

Expand All @@ -146,7 +147,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 = 5
LIBPATCH = 6

PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"]

Expand Down
34 changes: 20 additions & 14 deletions lib/charms/tempo_k8s/v2/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
object from this charm library. For the simplest use cases, using the `TracingEndpointRequirer`
object only requires instantiating it, typically in the constructor of your charm. The
`TracingEndpointRequirer` constructor requires the name of the relation over which a tracing endpoint
is exposed by the Tempo charm, and a list of protocols it intends to send traces with.
is exposed by the Tempo charm, and a list of protocols it intends to send traces with.
This relation must use the `tracing` interface.
The `TracingEndpointRequirer` object may be instantiated as follows
Expand All @@ -21,28 +21,28 @@
def __init__(self, *args):
super().__init__(*args)
# ...
self.tracing = TracingEndpointRequirer(self,
self.tracing = TracingEndpointRequirer(self,
protocols=['otlp_grpc', 'otlp_http', 'jaeger_http_thrift']
)
# ...
Note that the first argument (`self`) to `TracingEndpointRequirer` is always a reference to the
parent charm.
Alternatively to providing the list of requested protocols at init time, the charm can do it at
any point in time by calling the
`TracingEndpointRequirer.request_protocols(*protocol:str, relation:Optional[Relation])` method.
Alternatively to providing the list of requested protocols at init time, the charm can do it at
any point in time by calling the
`TracingEndpointRequirer.request_protocols(*protocol:str, relation:Optional[Relation])` method.
Using this method also allows you to use per-relation protocols.
Units of provider charms obtain the tempo endpoint to which they will push their traces by calling
Units of provider charms obtain the tempo endpoint to which they will push their traces by calling
`TracingEndpointRequirer.get_endpoint(protocol: str)`, where `protocol` is, for example:
- `otlp_grpc`
- `otlp_http`
- `zipkin`
- `tempo`
If the `protocol` is not in the list of protocols that the charm requested at endpoint set-up time,
the library will raise an error.
If the `protocol` is not in the list of protocols that the charm requested at endpoint set-up time,
the library will raise an error.
## Requirer Library Usage
Expand Down Expand Up @@ -104,7 +104,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 = 3
LIBPATCH = 5

PYDEPS = ["pydantic"]

Expand Down Expand Up @@ -308,6 +308,9 @@ class TracingProviderAppData(DatabagModel): # noqa: D101
external_url: Optional[str] = None
"""Server url. If an ingress is present, it will be the ingress address."""

internal_scheme: Optional[str] = None
"""Scheme for internal communication. If it is present, it will be protocol accepted by the provider."""


class TracingRequirerAppData(DatabagModel): # noqa: D101
"""Application databag model for the tracing requirer."""
Expand Down Expand Up @@ -495,6 +498,7 @@ def __init__(
host: str,
external_url: Optional[str] = None,
relation_name: str = DEFAULT_RELATION_NAME,
internal_scheme: Optional[Literal["http", "https"]] = "http",
):
"""Initialize.
Expand All @@ -505,6 +509,7 @@ def __init__(
if an ingress is present.
relation_name: an optional string name of the relation between `charm`
and the Tempo charmed service. The default is "tracing".
internal_scheme: scheme to use with internal urls.
Raises:
RelationNotFoundError: If there is no relation in the charm's metadata.yaml
Expand All @@ -525,6 +530,7 @@ def __init__(
self._host = host
self._external_url = external_url
self._relation_name = relation_name
self._internal_scheme = internal_scheme
self.framework.observe(
self._charm.on[relation_name].relation_joined, self._on_relation_event
)
Expand Down Expand Up @@ -590,10 +596,11 @@ def publish_receivers(self, receivers: Sequence[RawReceiver]):
try:
TracingProviderAppData(
host=self._host,
external_url=self._external_url,
external_url=self._external_url or None,
receivers=[
Receiver(port=port, protocol=protocol) for protocol, port in receivers
],
internal_scheme=self._internal_scheme,
).dump(relation.data[self._charm.app])

except ModelError as e:
Expand Down Expand Up @@ -822,11 +829,10 @@ def _get_endpoint(
receiver = receivers[0]
# if there's an external_url argument (v2.5+), use that. Otherwise, we use the tempo local fqdn
if app_data.external_url:
url = app_data.external_url
url = f"{app_data.external_url}:{receiver.port}"
else:
# FIXME: if we don't get an external url but only a
# hostname, we don't know what scheme we need to be using. ASSUME HTTP
url = f"http://{app_data.host}:{receiver.port}"
# if we didn't receive a scheme (old provider), we assume HTTP is used
url = f"{app_data.internal_scheme or 'http'}://{app_data.host}:{receiver.port}"

if receiver.protocol.endswith("grpc"):
# TCP protocols don't want an http/https scheme prefix
Expand Down
8 changes: 4 additions & 4 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@

@trace_charm(
tracing_endpoint="tracing_endpoint",
server_cert="server_cert_path",
server_cert="server_ca_cert_path",
extra_types=[
AuthRequirer,
CertHandler,
Expand Down Expand Up @@ -1533,9 +1533,9 @@ def tracing_endpoint(self) -> Optional[str]:
return None

@property
def server_cert_path(self) -> Optional[str]:
"""Server certificate path for TLS tracing."""
return GRAFANA_CRT_PATH
def server_ca_cert_path(self) -> Optional[str]:
"""Server CA certificate path for TLS tracing."""
return str(CA_CERT_PATH) if self.cert_handler.enabled else None


if __name__ == "__main__":
Expand Down
6 changes: 3 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ allowlist_externals = /usr/bin/env
[testenv:unit]
description = Run unit tests
deps =
pytest
pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263
coverage[toml]
responses
cosl
Expand All @@ -85,7 +85,7 @@ allowlist_externals =
[testenv:scenario]
description = Run scenario tests
deps =
pytest
pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263
responses
cosl
ops-scenario==6
Expand All @@ -106,7 +106,7 @@ deps =
asyncstdlib
# Libjuju needs to track the juju version
juju<=3.3.0,>=3.0
pytest
pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263
pytest-operator
pytest-playwright
lightkube
Expand Down

0 comments on commit ec74910

Please sign in to comment.