diff --git a/deploy/k8s/tavolo.yaml b/deploy/k8s/tavolo.yaml index e65bcbc..3618df5 100644 --- a/deploy/k8s/tavolo.yaml +++ b/deploy/k8s/tavolo.yaml @@ -71,9 +71,9 @@ data: # CORS_ALLOW_CREDENTIALS: "false" # CORS_EXPOSE_HEADERS: "" # CORS_MAX_AGE: "600" - # OpenTelemetry (kaya-otel + system-metrics): traces, metrics and logs - # via OTLP/HTTP, disabled unless OTEL_ENABLED is set. The image bundles - # the otel extra. + # OpenTelemetry (kaya-otel + contrib instrumentations): traces + # (HTTP/WebSocket, Postgres, Redis), metrics and logs via OTLP/HTTP, + # disabled unless OTEL_ENABLED is set. The image bundles the otel extra. # OTEL_ENABLED: "true" # OTEL_SERVICE_NAME: "tavolo" # OTEL_EXPORTER_OTLP_ENDPOINT: "http://otel-collector.observability:4318" diff --git a/docker-compose.yml b/docker-compose.yml index f88ef2d..c4a55f6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -122,8 +122,9 @@ services: CORS_ALLOW_CREDENTIALS: ${CORS_ALLOW_CREDENTIALS:-} CORS_EXPOSE_HEADERS: ${CORS_EXPOSE_HEADERS:-} CORS_MAX_AGE: ${CORS_MAX_AGE:-} - # OpenTelemetry (kaya-otel + system-metrics): traces, metrics and - # logs via OTLP/HTTP, disabled unless OTEL_ENABLED is set. + # OpenTelemetry (kaya-otel + contrib instrumentations): traces + # (HTTP/WebSocket, Postgres, Redis), metrics and logs via OTLP/HTTP, + # disabled unless OTEL_ENABLED is set. # The image bundles the otel extra (see packages/tavolo-app/pyproject.toml # and server/requirements.txt). OTEL_ENABLED: ${OTEL_ENABLED:-} diff --git a/server/.env.example b/server/.env.example index d80e54a..d23f701 100644 --- a/server/.env.example +++ b/server/.env.example @@ -61,11 +61,12 @@ TURN_TIMEOUT_SECONDS=30 # Seconds browsers may cache the preflight response (default 600): #CORS_MAX_AGE=600 -# OpenTelemetry (via kaya-otel + the contrib system-metrics package). Off -# unless OTEL_ENABLED is truthy; exports traces, HTTP/app/process metrics -# and logs over OTLP/HTTP (requires the tavolo-app "otel" extra, bundled -# in the Docker image). The endpoint defaults to the standard -# OTEL_EXPORTER_OTLP_ENDPOINT (localhost:4318) when unset. +# OpenTelemetry (via kaya-otel + the contrib instrumentation packages). +# Off unless OTEL_ENABLED is truthy; exports traces (HTTP/WebSocket, +# Postgres via asyncpg, Redis), metrics and logs over OTLP/HTTP (requires +# the tavolo-app "otel" extra, bundled in the Docker image). The endpoint +# defaults to the standard OTEL_EXPORTER_OTLP_ENDPOINT (localhost:4318) +# when unset. #OTEL_ENABLED=true #OTEL_SERVICE_NAME=tavolo #OTEL_EXPORTER_OTLP_ENDPOINT=http://otel-collector:4318 diff --git a/server/README.md b/server/README.md index 7d98bd5..fdfdba4 100644 --- a/server/README.md +++ b/server/README.md @@ -130,7 +130,11 @@ loggers: With `OTEL_ENABLED` set, the app exports over OTLP/HTTP (kaya-otel plus `tavolo.telemetry`): -- **Traces**: HTTP server spans and one span per WebSocket connection. +- **Traces**: HTTP server spans, one span per WebSocket connection, plus + database spans for the Postgres (asyncpg, through Tortoise) and Redis + clients. Redis command values are redacted by the instrumentation + (`GET ? ?`) and asyncpg spans carry the parameterized SQL only — no + parameter values. - **Metrics**: the HTTP server metrics from kaya-otel, plus `tavolo.active_users` (sessions in the session store), `tavolo.ongoing_games` (live matches not finished yet, maintained by the diff --git a/server/packages/tavolo-app/README.md b/server/packages/tavolo-app/README.md index 3ec1626..6e7cbf8 100644 --- a/server/packages/tavolo-app/README.md +++ b/server/packages/tavolo-app/README.md @@ -12,9 +12,11 @@ The deployable tavolo application: the composition root wiring `tavolo.app:app` for granian. - `config.py` — environment-driven frozen `Settings`. - `telemetry.py` — the optional OpenTelemetry wiring beyond kaya-otel: - meter/logger providers, the `tavolo.active_users`/`tavolo.ongoing_games` - gauges (polled from the stores), process/CPython metrics via - `opentelemetry-instrumentation-system-metrics` and OTLP log export. + meter/tracer/logger providers, the + `tavolo.active_users`/`tavolo.ongoing_games` gauges (polled from the + stores), process/CPython metrics via + `opentelemetry-instrumentation-system-metrics`, Postgres/Redis query + spans via the asyncpg/redis instrumentations, and OTLP log export. - `aerich_config.py` — Tortoise ORM configuration consumed by the aerich CLI (migrations live in `server/migrations/`; aerich itself is configured via the `[tool.aerich]` section of `server/pyproject.toml`). diff --git a/server/packages/tavolo-app/pyproject.toml b/server/packages/tavolo-app/pyproject.toml index 8b0cdbd..9febbc4 100644 --- a/server/packages/tavolo-app/pyproject.toml +++ b/server/packages/tavolo-app/pyproject.toml @@ -35,6 +35,8 @@ dev = [ ] otel = [ "kaya-otel>=0.0.4", + "opentelemetry-instrumentation-asyncpg>=0.66b0", + "opentelemetry-instrumentation-redis>=0.66b0", "opentelemetry-instrumentation-system-metrics>=0.66b0", ] diff --git a/server/packages/tavolo-app/src/tavolo/app.py b/server/packages/tavolo-app/src/tavolo/app.py index 5e9e631..eb08d1f 100644 --- a/server/packages/tavolo-app/src/tavolo/app.py +++ b/server/packages/tavolo-app/src/tavolo/app.py @@ -80,7 +80,9 @@ def cors_mixin_from_settings(settings: Settings) -> Optional[CorsMixin]: def otel_mixin_from_settings( - settings: Settings, meter_provider: Optional[Any] = None + settings: Settings, + meter_provider: Optional[Any] = None, + tracer_provider: Optional[Any] = None, ) -> Optional[KayaMixin]: """Build a :class:`~kaya.otel.OTelMixin` from the OTEL_* settings. @@ -89,10 +91,11 @@ def otel_mixin_from_settings( is imported lazily here: default installs and the test suite never need the OpenTelemetry packages. - ``meter_provider`` is the provider built by - :func:`tavolo.telemetry.telemetry_from_settings`; injecting it lets the - custom gauges share the same OTLP pipeline (kaya-otel then leaves its - shutdown to the telemetry mixin). + ``meter_provider``/``tracer_provider`` are the providers built by + :func:`tavolo.telemetry.telemetry_from_settings`; injecting them lets + the custom instruments and the database instrumentations share the same + OTLP pipelines (kaya-otel then leaves their shutdown to the telemetry + mixin). """ if not settings.otel_enabled: return None @@ -111,6 +114,8 @@ def otel_mixin_from_settings( } if meter_provider is not None: kwargs["meter_provider"] = meter_provider + if tracer_provider is not None: + kwargs["tracer_provider"] = tracer_provider return OTelMixin(**kwargs) @@ -190,7 +195,9 @@ mixins: list[KayaMixin] = [session_mixin, oidc_mixin, tortoise_mixin, openapi_mi telemetry = telemetry_from_settings(settings, session_store, game_store) if telemetry is not None: otel_mixin = otel_mixin_from_settings( - settings, meter_provider=telemetry.meter_provider + settings, + meter_provider=telemetry.meter_provider, + tracer_provider=telemetry.tracer_provider, ) assert otel_mixin is not None # OTEL_ENABLED was checked above # kaya-otel first: before hooks run in registration order (after hooks diff --git a/server/packages/tavolo-app/src/tavolo/telemetry.py b/server/packages/tavolo-app/src/tavolo/telemetry.py index af11095..09b8ab9 100644 --- a/server/packages/tavolo-app/src/tavolo/telemetry.py +++ b/server/packages/tavolo-app/src/tavolo/telemetry.py @@ -37,6 +37,7 @@ from .config import Settings if TYPE_CHECKING: from opentelemetry.sdk._logs import LoggerProvider from opentelemetry.sdk.metrics import MeterProvider + from opentelemetry.sdk.trace import TracerProvider log = getLogger(__name__) @@ -122,17 +123,19 @@ CountableSessionStore = Union[CountingInMemorySessionStore, CountingRedisSession class TelemetryMixin(KayaMixin): - """Application gauges and log export on top of kaya-otel. + """Application gauges, DB instrumentation and log export. - The meter and logger providers are built by + The meter, tracer and logger providers are built by :func:`telemetry_from_settings` and injected: kaya-otel only shuts - down providers it created itself, so this mixin owns their shutdown. + down providers it created itself, so this mixin owns their shutdown + (and uninstruments the database clients it instrumented). """ def __init__( self, *, meter_provider: "MeterProvider", + tracer_provider: "TracerProvider", logger_provider: Optional["LoggerProvider"], log_handler: Optional[logging.Handler], session_store: CountableSessionStore, @@ -140,6 +143,7 @@ class TelemetryMixin(KayaMixin): poll_interval_seconds: float, ) -> None: self._meter_provider = meter_provider + self._tracer_provider = tracer_provider self._logger_provider = logger_provider self._log_handler = log_handler self._session_store = session_store @@ -149,8 +153,11 @@ class TelemetryMixin(KayaMixin): self._ongoing_games: Optional[int] = None self._poll_task: Optional[asyncio.Task] = None self._instrumentor: Optional[Any] = None + self._db_instrumentors: list = [] def apply(self, app: KayaApp) -> None: + from opentelemetry.instrumentation.asyncpg import AsyncPGInstrumentor + from opentelemetry.instrumentation.redis import RedisInstrumentor from opentelemetry.instrumentation.system_metrics import ( SystemMetricsInstrumentor, ) @@ -160,6 +167,16 @@ class TelemetryMixin(KayaMixin): self._instrumentor = instrumentor instrumentor.instrument(meter_provider=self._meter_provider) + # Query spans for the Postgres (asyncpg, used by Tortoise) and Redis + # clients. Redis command values are redacted by the instrumentation + # (``SET ? ?``); asyncpg spans carry the parameterized SQL only. + self._db_instrumentors = [ + AsyncPGInstrumentor(capture_parameters=False), + RedisInstrumentor(), + ] + for db_instrumentor in self._db_instrumentors: + db_instrumentor.instrument(tracer_provider=self._tracer_provider) + meter = self._meter_provider.get_meter(__name__) def active_users(options: CallbackOptions): @@ -192,12 +209,16 @@ class TelemetryMixin(KayaMixin): if self._poll_task is not None: self._poll_task.cancel() self._poll_task = None + for db_instrumentor in self._db_instrumentors: + db_instrumentor.uninstrument() + self._db_instrumentors = [] if self._log_handler is not None: logging.getLogger().removeHandler(self._log_handler) self._log_handler.close() self._log_handler = None if self._logger_provider is not None: self._logger_provider.shutdown() + self._tracer_provider.shutdown() self._meter_provider.shutdown() async def _poll(self) -> None: @@ -222,6 +243,7 @@ class Telemetry: """The providers and mixin built from the ``OTEL_*`` settings.""" meter_provider: "MeterProvider" + tracer_provider: "TracerProvider" logger_provider: Optional["LoggerProvider"] log_handler: Optional[logging.Handler] mixin: TelemetryMixin @@ -232,7 +254,7 @@ def telemetry_from_settings( session_store: CountableSessionStore, game_store: GameStore, ) -> Optional[Telemetry]: - """Build the metric/log providers, or ``None`` when telemetry is off. + """Build the metric/trace/log providers, or ``None`` when telemetry is off. Only called when ``OTEL_ENABLED`` is set; the OpenTelemetry imports are lazy so the default install does not need the ``otel`` extra. @@ -246,11 +268,16 @@ def telemetry_from_settings( from opentelemetry.exporter.otlp.proto.http.metric_exporter import ( OTLPMetricExporter, ) + from opentelemetry.exporter.otlp.proto.http.trace_exporter import ( + OTLPSpanExporter, + ) from opentelemetry.sdk._logs import LoggerProvider, LoggingHandler from opentelemetry.sdk._logs.export import BatchLogRecordProcessor from opentelemetry.sdk.metrics import MeterProvider from opentelemetry.sdk.metrics.export import PeriodicExportingMetricReader from opentelemetry.sdk.resources import Resource + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import BatchSpanProcessor except ImportError as exc: raise RuntimeError( "OTEL_ENABLED is set but the OpenTelemetry dependencies are not " @@ -260,6 +287,15 @@ def telemetry_from_settings( endpoint = settings.otel_exporter_endpoint headers = otel_headers(settings) resource = Resource.create({"service.name": settings.otel_service_name}) + tracer_provider = TracerProvider(resource=resource) + tracer_provider.add_span_processor( + BatchSpanProcessor( + OTLPSpanExporter( + endpoint=_signal_endpoint(endpoint, "v1/traces"), + headers=headers, + ) + ) + ) meter_provider = MeterProvider( resource=resource, metric_readers=[ @@ -286,6 +322,7 @@ def telemetry_from_settings( logging.getLogger().addHandler(handler) mixin = TelemetryMixin( meter_provider=meter_provider, + tracer_provider=tracer_provider, logger_provider=logger_provider, log_handler=handler, session_store=session_store, @@ -294,6 +331,7 @@ def telemetry_from_settings( ) return Telemetry( meter_provider=meter_provider, + tracer_provider=tracer_provider, logger_provider=logger_provider, log_handler=handler, mixin=mixin, diff --git a/server/requirements.txt b/server/requirements.txt index 397a3d9..4f0de87 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -94,6 +94,8 @@ opentelemetry-api==1.45.0 # opentelemetry-exporter-http-transport # opentelemetry-exporter-otlp-proto-http # opentelemetry-instrumentation + # opentelemetry-instrumentation-asyncpg + # opentelemetry-instrumentation-redis # opentelemetry-instrumentation-system-metrics # opentelemetry-sdk # opentelemetry-semantic-conventions @@ -106,7 +108,14 @@ opentelemetry-exporter-otlp-proto-common==1.45.0 opentelemetry-exporter-otlp-proto-http==1.45.0 # via kaya-otel opentelemetry-instrumentation==0.66b0 - # via opentelemetry-instrumentation-system-metrics + # via + # opentelemetry-instrumentation-asyncpg + # opentelemetry-instrumentation-redis + # opentelemetry-instrumentation-system-metrics +opentelemetry-instrumentation-asyncpg==0.66b0 + # via tavolo-app (pyproject.toml) +opentelemetry-instrumentation-redis==0.66b0 + # via tavolo-app (pyproject.toml) opentelemetry-instrumentation-system-metrics==0.66b0 # via tavolo-app (pyproject.toml) opentelemetry-proto==1.45.0 @@ -121,6 +130,8 @@ opentelemetry-sdk==1.45.0 opentelemetry-semantic-conventions==0.66b0 # via # opentelemetry-instrumentation + # opentelemetry-instrumentation-asyncpg + # opentelemetry-instrumentation-redis # opentelemetry-instrumentation-system-metrics # opentelemetry-sdk packaging==26.3 @@ -165,4 +176,6 @@ typing-extensions==4.16.0 urllib3==2.8.0 # via opentelemetry-exporter-http-transport wrapt==2.5.0 - # via opentelemetry-instrumentation + # via + # opentelemetry-instrumentation + # opentelemetry-instrumentation-redis diff --git a/server/tests/test_otel.py b/server/tests/test_otel.py index e350a91..1037d55 100644 --- a/server/tests/test_otel.py +++ b/server/tests/test_otel.py @@ -78,6 +78,15 @@ class OTelMixinFromSettingsTests(unittest.TestCase): assert isinstance(mixin, _StubOTelMixin) self.assertIs(provider, mixin.kwargs["meter_provider"]) + def test_tracer_provider_is_passed_through(self): + with _stub_kaya_otel(): + provider = object() + mixin = otel_mixin_from_settings( + _settings({"OTEL_ENABLED": "1"}), tracer_provider=provider + ) + assert isinstance(mixin, _StubOTelMixin) + self.assertIs(provider, mixin.kwargs["tracer_provider"]) + def test_excluded_paths_are_passed_through(self): with _stub_kaya_otel(): mixin = otel_mixin_from_settings(_settings({ diff --git a/server/tests/test_telemetry.py b/server/tests/test_telemetry.py index 54b3e90..48f63db 100644 --- a/server/tests/test_telemetry.py +++ b/server/tests/test_telemetry.py @@ -9,6 +9,7 @@ import logging import os import sys import unittest +from types import SimpleNamespace from typing import Optional from unittest.mock import patch @@ -33,6 +34,11 @@ try: from opentelemetry.sdk.metrics import MeterProvider from opentelemetry.sdk.metrics.export import InMemoryMetricReader from opentelemetry.sdk.resources import Resource + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import SimpleSpanProcessor + from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, + ) HAVE_OTEL = True except ImportError: # pragma: no cover - installs without the otel extra @@ -148,19 +154,25 @@ def _metric_points(reader) -> dict: @unittest.skipUnless(HAVE_OTEL, "tavolo-app[otel] not installed") class MetricExportTest(unittest.TestCase): - # SystemMetricsInstrumentor is a process-wide singleton whose callbacks - # cannot be re-registered, so the provider and mixin are built once for - # the whole class (as the application does). + # The instrumentation packages are process-wide singletons whose + # callbacks cannot be re-registered, so the providers and mixin are + # built once for the whole class (as the application does). sessions = _FakeSessions(0) games = _FakeGames(0) @classmethod def setUpClass(cls) -> None: - cls.reader = InMemoryMetricReader() resource = Resource.create({"service.name": "test"}) + cls.reader = InMemoryMetricReader() cls.provider = MeterProvider(resource=resource, metric_readers=[cls.reader]) + cls.span_exporter = InMemorySpanExporter() + cls.tracer_provider = TracerProvider(resource=resource) + cls.tracer_provider.add_span_processor( + SimpleSpanProcessor(cls.span_exporter) + ) cls.mixin = TelemetryMixin( meter_provider=cls.provider, + tracer_provider=cls.tracer_provider, logger_provider=None, log_handler=None, session_store=cls.sessions, @@ -171,7 +183,7 @@ class MetricExportTest(unittest.TestCase): @classmethod def tearDownClass(cls) -> None: - cls.provider.shutdown() + cls.mixin.shutdown(None) # type: ignore[arg-type] @async_test async def test_app_gauges_report_polled_counts(self) -> None: @@ -218,6 +230,36 @@ class MetricExportTest(unittest.TestCase): # Host-level metrics are deliberately not configured. self.assertNotIn("system.cpu.time", points) + @async_test + async def test_database_clients_are_instrumented(self) -> None: + import asyncpg.connection + import redis.asyncio + + self.assertTrue(hasattr(redis.asyncio.Redis.execute_command, "__wrapped__")) + self.assertTrue(hasattr(asyncpg.connection.Connection.fetch, "__wrapped__")) + + # Exercise each wrapper through stubs: the downstream call fails (no + # server), but the span is recorded before it does. + client = object.__new__(redis.asyncio.Redis) + client.connection_pool = SimpleNamespace( + connection_kwargs={"host": "127.0.0.1", "port": 6379, "db": 0} + ) + with self.assertRaises(AttributeError): + await client.execute_command("GET", "key") + + connection = object.__new__(asyncpg.connection.Connection) + connection._aborted = True # keep __del__ quiet on the stub + with self.assertRaises(Exception): + await connection.fetch("SELECT 1") + + spans = { + span.name: span for span in self.span_exporter.get_finished_spans() + } + self.assertEqual("GET ?", spans["GET"].attributes["db.statement"]) + self.assertEqual("redis", spans["GET"].attributes["db.system"]) + self.assertEqual("SELECT 1", spans["SELECT"].attributes["db.statement"]) + self.assertEqual("postgresql", spans["SELECT"].attributes["db.system"]) + @unittest.skipUnless(HAVE_OTEL, "tavolo-app[otel] not installed") class LogExportTest(unittest.TestCase):