Repository navigation
feat(telemetry): OTLP traces for cc-catalog-svc - #197
Merged
Merged
Conversation
Wire cc-catalog-svc up as an OTLP *producer* for the platform's built-in observability (the collector + OpenObserve already ship with the chart). Off by default and driven entirely by the standard OpenTelemetry env vars: nothing is exported, constructed or dialed until OTEL_* asks for it, so an unconfigured deployment behaves exactly as before. - app/telemetry.py: shared OTel Resource (service.name/version/environment), one resolve_service_name() used by every signal, a TracerProvider + OTLP (gRPC) span exporter with FastAPI instrumentation, and opt-in logs (root handler) and metrics (MeterProvider). Every exporter is built inside try/except so telemetry can never break startup. - app/main.py: configure telemetry in the lifespan and shut it down on exit; instrument FastAPI before the app starts. - pyproject.toml/poetry.lock: add opentelemetry-api/sdk, exporter-otlp-proto-grpc and instrumentation-fastapi. - tests/test_telemetry.py: off-by-default no-op, service-name precedence, traces-enabled provider, per-signal endpoint precedence, and exporter failures degrading instead of raising.
The platform chart's standard surface is HTTP/protobuf on :4318 with per-signal endpoints that already carry /v1/<signal>, and it deliberately sets no generic OTEL_EXPORTER_OTLP_ENDPOINT. A gRPC-only exporter aimed gRPC at the HTTP port and had no fallback, so the service could not talk to the collector as deployed. - Read OTEL_EXPORTER_OTLP_PROTOCOL (default http/protobuf; grpc supported) and build the matching exporter for traces, logs and metrics. - Reproduce the SDK endpoint rule for both protocols: a per-signal endpoint is used verbatim, a generic endpoint gets /v1/<signal> appended, so the chart's endpoints are never double-pathed. - Add opentelemetry-exporter-otlp-proto-http and refresh the lock. - Tests: protocol selection (default http/protobuf, grpc), endpoint precedence and path-append rules for traces/logs/metrics, alongside the existing off-by-default, service-name, shared-resource and failure-degradation cases.
Contributor
cc-catalog-svc imageTag:
|
Contributor
cc-catalog-svc imageTag:
|
Contributor
cc-catalog-svc imageTag:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds opt-in OTLP telemetry to
cc-catalog-svc— the producer half of the platform's built-in observability (the collector + OpenObserve ship with the platform chart; the collector side already exists).The service now consumes the platform chart's documented standard surface as-is:
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobufwith the per-signalhttp://<release>-otel-collector:4318/v1/...endpoints. No bespoke variables, no extra config.Off-by-default contract
Nothing changes unless the standard OTel env vars are set: no exporter is constructed, no provider installed, no network call made, no new required config.
app/telemetry.pyshort-circuits to a no-op path (returnsNone) when a signal's switch is nototlpor no endpoint is present, and FastAPI is only instrumented when tracing is actually active. A deployment that sets nothing behaves exactly as before.Exact env vars (standard names only — the chart needs no bespoke vars)
Protocol — picks the exporter:
OTEL_EXPORTER_OTLP_PROTOCOLhttp/protobuf(default, the chart's value):4318)grpc:4317)Signal switches (value must be
otlp; anything else leaves the signal off):OTEL_TRACES_EXPORTER=otlpOTEL_LOGS_EXPORTER=otlpOTEL_METRICS_EXPORTER=otlpEndpoints — the per-signal variable wins over the generic one, for both protocols:
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT/OTEL_EXPORTER_OTLP_ENDPOINTOTEL_EXPORTER_OTLP_LOGS_ENDPOINT/OTEL_EXPORTER_OTLP_ENDPOINTOTEL_EXPORTER_OTLP_METRICS_ENDPOINT/OTEL_EXPORTER_OTLP_ENDPOINTFor HTTP the SDK's endpoint rule is reproduced exactly: a per-signal endpoint is used verbatim (the chart's already end in
/v1/<signal>), while a generic endpoint gets/v1/<signal>appended. This is asserted in tests — getting it wrong double-paths the URL and breaks the chart's endpoints.Service / resource / other:
OTEL_SERVICE_NAME— per-component name; wins over the in-code default (cc-catalog-svc).OTEL_RESOURCE_ATTRIBUTES— merged into the shared Resource;deployment.environmentis read from it.OTEL_TRACES_SAMPLER(+OTEL_TRACES_SAMPLER_ARG) andOTEL_SDK_DISABLED— honored where the SDK reads them itself.What is instrumented
TracerProvider+BatchSpanProcessor(<protocol> span exporter), andFastAPIInstrumentoron the app (a server span per request).setup_telemetry()/instrument_fastapi().OTEL_LOGS_EXPORTER=otlp) — aLoggerProvider+ OTLP log exporter attached to the root stdlib logger, so every record is exported with the active span's trace/span id (that is what lets OpenObserve correlate logs to traces).setup_logging_export().OTEL_METRICS_EXPORTER=otlp) — aMeterProvider+ periodic OTLP metric reader, installed as the global meter provider.setup_metrics().One
resolve_service_name()resolves the name for all three signals, and one_build_resource()builds the shared OTelResource(service.name,service.version,deployment.environment), so traces/logs/metrics can never land under different service names.Safety
Every exporter/provider is built inside
try/exceptthat logs and degrades to "no telemetry" instead of raising, because this runs on the FastAPI startup path. Provider shutdown is per-provider guarded too.Wiring
app/main.py— telemetry is set up at the top of the lifespan (after logging is configured) andshutdown_telemetry()flushes on exit;instrument_fastapi(app)runs once before the app starts.How I tested it
poetry run pytest tests/test_telemetry.py→ 26 passed. Covers: nothing-configured no-op (no exporter constructed, no crash);OTEL_SERVICE_NAMEprecedence; protocol selection (http/protobufdefault → HTTP exporter;grpc→ gRPC exporter; unknown/http/json→ http/protobuf); endpoint rules — per-signal verbatim for traces/logs/metrics, generic endpoint gets/v1/<signal>appended (incl. trailing-slash handling), gRPC endpoints never get a path; https →insecure=False; logs (root handler, idempotent) and metrics (global install) opt-in; and broken trace/log/metric exporters degrade toNoneinstead of raising. All exporters are patched — no test performs network I/O.poetry run pytest→ 180 passed, 2 failed. The two failures are pre-existing and unrelated (tests/test_git_http.py::test_info_refs_serves_repo_owned_by_another_userand::test_clone_succeeds_through_symlinked_data_dir_when_foreign_owned); both fail identically on an unmodifiedmaincheckout here, because this machine's git reports "dubious ownership" for the foreign-owned test repositories. I did not touch them./v1/...endpoints, no generic var) and patched exporters:GET /healthz→ 200, HTTP span/log exporters constructed with the endpoints used verbatim, app instrumented, clean shutdown. Also smoke-tested with nothing configured (no-op).ruff checkon changed files: passes.Deliberately not done
k8s/manifests or the chart — the platform's OTel env ConfigMap supplies the variables; this PR is code-only.http/jsonsupport:OTEL_EXPORTER_OTLP_PROTOCOL=http/jsonfalls back tohttp/protobuf(theopentelemetry-exporter-otlp-proto-httppackage only speaks protobuf). gRPC remains fully supported viaOTEL_EXPORTER_OTLP_PROTOCOL=grpc.