feat(otel): initialize OpenTelemetry tracing and metrics in boot()#96
feat(otel): initialize OpenTelemetry tracing and metrics in boot()#96fubezz wants to merge 30 commits into
Conversation
Adds otel_initialize(), following the same enabled-flag + graceful-degradation pattern as sentry_initialize(): reads the standard OTEL_* env vars, sets up a process-wide TracerProvider/MeterProvider exporting via OTLP/gRPC, and is a no-op unless both explicitly enabled and OTEL_EXPORTER_OTLP_ENDPOINT is set. Called from boot() so every Foundry service gets it via a core version bump, with no per-service code changes needed for process-level tracing/metrics. Request-level FastAPI tracing needs one additional call once the app exists (instrument_fastapi(app), exported here) since boot() runs before the app is constructed - this still needs wiring into the foundry-python template's Service.api_serve(). Ref: OP-3108
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Adds LoggerProvider/BatchLogRecordProcessor(OTLPLogExporter()) setup to otel_initialize(), and a loguru sink that converts each record into a stdlib logging.LogRecord and forwards it to OTel's own LoggingHandler. Loguru has no first-party OTel integration (this project uses loguru, not stdlib logging, throughout), so reusing LoggingHandler's own conversion and trace-correlation logic via a thin bridge is simpler than reimplementing OTLP log export from scratch. Closes the logs gap flagged in PR review against OP-3108's acceptance criteria (traces + metrics + logs).
Splits logs export behind a new logs_enabled setting (default False), independent of the main enabled flag. Traces/metrics behave unchanged; the loguru->OTLP logs bridge only activates when logs_enabled is also set, since log volume/cost characteristics differ from traces/metrics and shouldn't be forced on every service that enables tracing. Ref: OP-3108
Records the design decisions behind the otel_initialize()/boot() work in this PR, plus the gitops-side shared-tools gateway/Contour/HTTPProxy/ k8sAttributes decisions from OP-3108 that motivated it. Ref: OP-3108
The collector-push-gateway chart's own architecture (Deployment mode, loadbalancing tier, k8s enrichment, fan-out) is the portfolio's existing standard pattern, already running elsewhere. Reframes the first section to make clear only the instance-to-reach and sidecar-vs-direct-export questions are actually being decided in this ADR.
terraform-gcp-cloud-run-v2 is deprecated for Foundry. Foundry services are built into an image and deployed via the GCP API from a generated Knative manifest (foundry-python template + deploy-cloudrun GitHub Action), not via that Terraform module. Corrects the sidecar-rejection reasoning and the Direct VPC egress follow-up to point at the template's manifest instead of a module that doesn't apply here.
Splits the single 'enabled' flag (which bundled traces+metrics together) into three independent settings: traces_enabled and metrics_enabled (default True once the master 'enabled' switch is on), and logs_enabled (default False, unchanged). A service can now, for example, disable metrics while keeping traces on, without needing all-or-nothing behavior. Ref: OP-3108
GCP-native observability wasn't a live alternative we rejected - OP-3108 explicitly scoped this to the shared stack, and any service remains free to point its SDK at Google's OTLP endpoint instead, since that's just an instrumentation-level config choice. Also updates the logs-default section and Decision text to reflect the independent traces_enabled/ metrics_enabled/logs_enabled toggles instead of one bundled flag.
… stakeholders - Sidecar rejection now explicitly notes this matches the existing Sentry SDK-instrumentation pattern (in-process, not agent/sidecar-based) -- no precedent anywhere in the stack for a sidecar telemetry agent. - Ingress section now states ingress-nginx is deprecated company-wide and Contour is the chosen long-term replacement -- not just a local elegance trade-off for this one gateway. - Cert provisioning section clarified: both options result in the same cert-manager-issued TLS certificate: proper encryption either way, only the issuance mechanism differs. Added GCP-service-account authentication enforcement as a follow-up (not yet designed). - Added Johannes Zorn and Arne Baumann as informed stakeholders.
Widens the scope statement to explicitly cover the ingress addition and the gateway-instance-specific enhancements (k8sattributes disable, optional auth), instead of just 'which instance and how'. Names the concrete mechanism for optional ingress auth: Contour HTTPProxy's built-in spec.virtualhost.jwtProviders (Envoy jwt_authn), which verifies Google-issued identity tokens against Google's public JWKS -- an available feature to turn on, not something to design from scratch.
The public app-collector instance was never a genuine alternative -- its public exposure and VM-onboarding-specific fragility rule it out before any real evaluation, so it's no longer framed as a considered-and-rejected Option A/B pair. Adds the actual hard blocker for a sidecar collector: our metrics path is scrape-only (no Thanos Receive, no real Prometheus Pushgateway), and an ephemeral Cloud Run sidecar has no stable target for Prometheus to scrape and nothing to push to -- its metrics would simply never be collected.
…efit Direct SDK export has no local buffer surviving instance teardown -- telemetry not yet flushed (BatchSpanProcessor queue, pending metric points) can be lost on scale-to-zero/revision replacement/crash. Cloud Run's SIGTERM grace period allows a shutdown-hook flush to mitigate most of this, but not guarantee it. Also captures that pointing every service at our own gateway endpoint (rather than a vendor-specific one) makes the backend swappable later (e.g. Grafana Cloud) by reconfiguring only the gateway's exporters.
… collector' The existing app-collector instance serves mono/Pelago-onboarding-VM traffic, an unrelated purpose -- it was never actually considered or compared against for this decision, so it shouldn't be named as a dismissed alternative. Replaced with generic references to the standard OTel push gateway collector pattern.
Not relevant to how Foundry actually deploys Cloud Run -- describes the real mechanism (Knative manifest + deploy-cloudrun GitHub Action) directly without contrasting against an irrelevant module.
… rejecting it Adds the thesis half of the dialectic that was missing: a sidecar scales alongside each Cloud Run instance automatically, so collector capacity tracks traffic without a separate scaling decision -- unlike a shared gateway sized for combined load across all services. Still rejected on the metrics-scrape blocker, but now the trade-off is stated honestly rather than only listing downsides.
Routing through the collector (vs. any hypothetical direct push to a metrics backend like Thanos Receive) keeps cardinality-dropping and label normalization centralized rather than reinvented per source, giving dashboards/alerts/on-call a common, predictable telemetry shape regardless of where it originated.
Follow-ups was restating the same four points already explained in Harder/risks. Trimmed to terse action items pointing back to the existing explanation instead of repeating it.
…-of-scope Direct VPC egress is not automatic from Shared VPC membership alone -- adds its own section detailing what's actually in place (44 projects' enable_cloudrun_vpc_access IAM grants) vs. what's incomplete (VPC egress annotations only render for Cloud SQL/Memorystore services; subnet values are manual, unverified placeholders). This is the real rollout blocker, previously understated as a one-line risk. Also restores a trimmed authentication mention (Contour jwtProviders / OTel-side mechanism), explicitly scoped out since the entire telemetry chain today trusts network location only, not just this one gateway -- a valid future improvement, not something to solve as a side effect here. Fixes dangling Follow-ups references to a since-removed section.
Previously framed as 'decouple from the condition or otherwise guarantee it' -- now states the resolved direction plainly: every Cloud Run service gets Direct VPC egress regardless of Cloud SQL/Memorystore usage, since reaching shared internal services is a universal need, not a database-specific one. Still not implemented in this PR -- tracked as a follow-up in foundry-python.
Checked whether the manual project.hcl placeholders could just be replaced with Terraform outputs -- they can't, directly, today: gcp-service-projects doesn't output the subnet/network path at all, and even if it did, the subnet isn't derivable from a fixed naming convention fleet-wide (aignx-bridge-dev hardcodes a legacy subnet key that doesn't follow the newer prefix-environment-env pattern). Right first step is adding real module outputs so onboarding copies an authoritative value instead of guessing; full automation needs a cross-repo output-sharing mechanism that doesn't exist yet -- split into two explicit follow-ups instead of one.
Reframed from an active follow-up to a recorded, valid improvement that isn't being solved here -- consistent with how GCP-native observability and ingress authentication are already handled elsewhere in this ADR. The manual project.hcl placeholder mechanism stays as-is.
Add a Pull requests section and link foundry-python#385 and foundry-infrastructure#10 from the Follow-ups that implement them. Update the repo count/list in Context now that the VPC egress fix spans the two Copier templates as well as gitops and this repo.
Fold the standalone Pull requests section into Status, next to the Jira ticket reference, instead of a separate section.
a424026 to
3a3efe0
Compare
|
| integrations=sentry_integrations, | ||
| context=ctx, | ||
| ) | ||
| otel_initialize(context=ctx) |
There was a problem hiding this comment.
| otel_initialize(context=ctx) | |
| _otel_initialize(context=ctx) |
| ] | ||
|
|
||
|
|
||
| def otel_initialize( |
There was a problem hiding this comment.
| def otel_initialize( | |
| def _otel_initialize( |
| """Initialise settings, deriving env_prefix from the active FoundryContext.""" | ||
| super().__init__(_env_prefix=f"{get_context().env_prefix}OTEL_", **kwargs) # pyright: ignore[reportCallIssue] | ||
|
|
||
| enabled: Annotated[ |
|
Notes from testing this against a locally-generated Foundry service (OP-3108, sandbox). A few design points worth considering before merge, none blocking: 1.
|
boot() previously discarded the bool sentry_initialize()/otel_initialize() return, so nothing surfaced at runtime whether either actually initialized vs. silently no-op'd (master switch off, missing DSN/endpoint). Appends "sentry: on/off, otel: on/off" to the existing boot log line using those already-computed return values.
…ride Adds otel_initialize(instrumentors=...) and default_otel_instrumentors(), mirroring how constants.py builds SENTRY_INTEGRATIONS: try/except-import each optional instrumentation package, degrade gracefully if absent. Unlike FastAPIInstrumentor, these instrument globally and don't need a live app instance, so they can run inside otel_initialize() itself rather than needing a separate call site. Defaults to HTTPXClientInstrumentor (new opentelemetry-instrumentation-httpx dependency) — without it, an outbound httpx call to another Foundry service doesn't get the W3C traceparent header injected, so the callee starts a new, disconnected trace instead of continuing the caller's. instrumentors=None uses the default; instrumentors=[] opts out; a longer list opts into more (e.g. SQLAlchemyInstrumentor) via boot(otel_instrumentors=...). Only applied when traces_enabled, since propagation is a tracing concern.
…ernal CA root The OTel push-gateway's HTTPProxy/ingress-facing TLS (Cloud Run's only path to it, per this ADR) is signed by the internal aignx-ca-root-authority CA (GoogleCASClusterIssuer, CA pool aignx-ca-pool in aignx-host-project-kah) — not a publicly trusted root, so grpc's bundled Mozilla roots reject it with CERTIFICATE_VERIFY_FAILED. In-cluster GKE consumers never hit this since they use the collector's plaintext otlp/insecure:4319 receiver directly; Cloud Run only has the TLS-terminated 443 path through the HTTPProxy. Bundles the root cert (fetched via `gcloud privateca roots describe aignx-ca-root-authority --pool aignx-ca-pool --project aignx-host-project-kah --location europe-west1`) as package data and defaults OTEL_EXPORTER_OTLP_CERTIFICATE to it in otel_initialize(), mirroring the existing OTEL_SERVICE_NAME setdefault pattern. An explicit value for that env var (e.g. pointing at a different vendor) always overrides the default.
…ource attribute Every Foundry "service" project gets a SQLAlchemy-backed database layer unconditionally (aignostics_foundry_core.database), so DB query spans are as much a baseline expectation as HTTP ones. Adds SQLAlchemyInstrumentor to default_otel_instrumentors() alongside HTTPXClientInstrumentor — .instrument() with no engine arg hooks SQLAlchemy's event system globally, picking up init_engine()'s engine once it's created later in boot(). Also sets foundry_service=<context name> on the shared Resource, distinct from service.name: on the gateway's Prometheus exporter (resource_to_telemetry_conversion), the scrape target's own job/instance labels (naming the collector pod) shadow the app's same-named resource attributes into exported_job/exported_instance, which is easy to misread. foundry_service sidesteps that collision with an unambiguous label.
…rs on exit Adopts three patterns from pviz's otel bootstrap (github.com/aignostics/pviz): GoogleCloudResourceDetector for standard cloud.*/faas.*/k8s.* attributes on GCP, an idempotency guard against double-initializing the same provider, and an atexit shutdown per provider so buffered spans/metrics/logs aren't lost when a Cloud Run instance is frozen or killed shortly after the last request.
…om resource Better fixed at the source than server-side: opentelemetry-sdk's Resource.create() auto-generates a random service.instance.id, and GoogleCloudResourceDetector sets faas.instance to a per-container-instance hash. Neither is human-readable or stable enough to be useful, and any backend that flattens resource attributes into labels (e.g. the OTel gateway's Prometheus exporter with resource_to_telemetry_conversion) turns them into an unbounded label dimension. Stripped once from the shared Resource before any provider is built, so it holds for traces/metrics/logs and every backend, rather than requiring the same drop to be duplicated in every collector config.



Summary
otel_initialize()in a newsrc/aignostics_foundry_core/otel.py, following the exact same pattern assentry_initialize(): pydantic-settings-drivenenabledflag (env-prefixed perFoundryContext), graceful no-op when the SDK isn't available or isn't enabled.TracerProvider/MeterProviderexporting via OTLP/gRPC, using the standard, unprefixed OpenTelemetry env vars (OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_SERVICE_NAME, etc.) rather than reinventing project-specific ones —OTEL_SERVICE_NAMEdefaults to theFoundryContextproject name if not explicitly set.logging— loguru has no first-party OTel integration. Added a loguru sink (_make_otel_log_sink) that converts each loguru record into a stdliblogging.LogRecordand forwards it to OTel's ownLoggingHandler, reusing the SDK's own conversion/trace-correlation logic instead of reimplementing OTLP log export. Gated behind a separatelogs_enabledsetting, off by default even whenenabledis true — log volume/cost characteristics differ from traces/metrics, so it shouldn't be forced on every service that turns on tracing.otel_initialize()intoboot(), right aftersentry_initialize()— every Foundry service gets this via a single core version bump, no per-service code changes for process-level tracing/metrics(/logs, opt-in).instrument_fastapi(app)for request-level tracing, sinceboot()runs before any FastAPIappinstance exists.Related: OP-3108 (expose an OTel endpoint for Foundry Cloud Run services — this is the service-side counterpart, decided against pushing this logic into the
foundry-pythontemplate since it's not an importable runtime dependency there).Known follow-up (not done in this PR)
instrument_fastapi(app)needs to actually be called somewhere — but the FastAPIappinstance is constructed inService.api_serve(), which lives in thefoundry-pythontemplate (not this repo), not inboot(). This PR ships the capability; wiring the one-line call into the template is a separate, small follow-up infoundry-python. Until that lands, request-level FastAPI auto-tracing (spans per HTTP request) isn't automatic — process-level tracing/metrics/logs (manually-created spans, outbound instrumentation, log records) work regardless.Test plan
mise run lint— ruff, pyright, deptry all pass.mise run test_unit— 13 passed,otel.pyat 100% coverage.mise run test_integration— 100 passed,otel.pyat 100% coverage.mise run pre_commit_run_all— all hooks pass.logs_enabledon/off gating.aignx-otel-gateway.aignostics.aifrom the gitops side of OP-3108 is live) — not done here, this PR is unit/integration-tested only.Opened as draft pending review of the design (in particular: the loguru→OTel logs bridge approach and its opt-in gating, and confirmation on the
instrument_fastapifollow-up plan).🤖 Generated with Claude Code