Skip to content

Commit a424026

Browse files
committed
refactor: Shorten ADR
1 parent fd40488 commit a424026

1 file changed

Lines changed: 27 additions & 75 deletions

File tree

docs/decisions/0006-cloud-run-otel-telemetry-export.md

Lines changed: 27 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -51,29 +51,25 @@ enrichment.
5151
the fleet authenticates callers — the entire telemetry stack chain today trusts network
5252
location alone (reachable only from inside the VPC/cluster pod network). Adding
5353
authentication — e.g. Contour's `HTTPProxy.spec.virtualhost.jwtProviders` (Envoy's `jwt_authn`
54-
filter, which could verify Google-issued Cloud Run service-account identity tokens) or an
55-
OTel-side mechanism — is a valid improvement worth considering, but it would apply to the
54+
filter, which could verify Google-issued Cloud Run service-account identity tokens)
55+
is a valid improvement worth considering, but it would apply to the
5656
whole chain, not just this one gateway, and is out of scope here rather than solved as a
5757
side effect of this decision.
5858

5959
### Which collector instance Cloud Run talks to
6060

6161
A new, vpc-only instance of the standard OTel push gateway collector: `collector-push-gateway`
62-
(same chart, unchanged architecture) deployed onto `shared-tools`, in loadbalancing mode, as
63-
`aignx-otel-gateway.aignostics.ai`, reachable only over the shared VPC via Direct VPC egress, never
64-
publicly. No existing instance serves this purpose. Every OTel collector running today serves
65-
GKE-internal traffic for a specific cluster/service set, none of them expose an ingress or
66-
accept traffic from outside a cluster's pod network.
62+
(same chart, unchanged architecture) deployed in `shared-tools`, in loadbalancing mode,
63+
reachable only over the shared VPC via Direct VPC egress, never
64+
publicly.
6765

68-
### Network reachability — Direct VPC egress is not automatic
66+
### Reachability — Direct VPC connection
6967

7068
Being in a GCP project attached to the shared VPC does not, by itself, give Cloud Run a path
7169
to reach anything on that VPC. Google's own docs confirm Shared VPC service-project membership
7270
only makes subnets *available* but Cloud Run must be explicitly configured with Direct VPC
7371
egress (a specific network + subnet) to actually route traffic there instead of the public
74-
internet. This matters because it's the actual gating factor for whether any Foundry Cloud Run
75-
service can reach `aignx-otel-gateway.aignostics.ai` at all, regardless of what this ADR
76-
decides about the gateway itself.
72+
internet.
7773

7874
What's already in place: 44 Foundry service-projects have `enable_cloudrun_vpc_access = true`
7975
(`gcp-service-projects` Terraform module), each granting the Cloud Run service agent
@@ -83,20 +79,17 @@ permits Direct VPC egress, it doesn't configure it on any actual Cloud Run resou
8379
What's incomplete on the `foundry-python` template side:
8480
- The generated Cloud Run manifest (`service.template.yaml.jinja`) already carries the Direct
8581
VPC egress annotations (`run.googleapis.com/network-interfaces`,
86-
`vpc-access-egress: private-ranges-only`) — but only rendered
87-
`{% if service_cloudsql_enabled or service_memorystore_enabled %}`, since they were added for
88-
the Cloud SQL proxy's private-IP connectivity, not for general reachability. A service using
89-
neither gets no VPC egress configuration at all.
82+
`vpc-access-egress: private-ranges-only`), but only rendered
83+
if cloud sql or memorystore is enabled, not for general reachability.
9084
- Even where it does render, the subnet values feeding it (`vpc_network`/`vpc_subnetwork` in
9185
the service's own `infrastructure/<env>/project.hcl`) are manual placeholders
9286
(`PLACEHOLDER_VPC_NETWORK`/`PLACEHOLDER_VPC_SUBNETWORK`) filled in by hand after onboarding.
9387

9488
Net effect: reachability isn't a solved problem for any Foundry Cloud Run service today,
9589
Cloud-SQL/Memorystore-enabled or not. This is the largest open risk for this ADR's rollout.
9690

97-
Resolved direction: Direct VPC egress should be unconditional, every Cloud Run service gets
98-
it regardless of whether it uses Cloud SQL/Memorystore, not just the ones that happened to need
99-
it for a database connection.
91+
Resolved direction: Direct VPC egress will be unconditional, every Cloud Run service gets
92+
it regardless of whether it uses Cloud SQL/Memorystore.
10093

10194
### Alternatives considered — Sidecar collector vs. direct SDK export from Cloud Run
10295

@@ -111,40 +104,30 @@ in the `foundry-python` Copier template, applied via the `deploy-cloudrun` GitHu
111104
Cloud Run's multi-container/sidecar support is exposed directly through that manifest —
112105
mechanically adding a second container is straightforward. The real benefit would be scaling.
113106
A sidecar scales automatically alongside each Cloud Run instance, so collector capacity always
114-
tracks the service's own traffic with no separate scaling decision to make. This is unlike a shared
115-
gateway, which has to be sized/scaled to handle the combined load of every Cloud Run service
116-
sending to it.
107+
tracks the service's own traffic with no separate scaling decision to make.
117108
Metrics are the hard blocker: our stack ingests metrics exclusively via
118109
Prometheus **scrape** (a `ServiceMonitor` targeting a stable in-cluster Service). There is no Thanos
119110
Receive enabled anywhere in the fleet, and we don't run a real Prometheus Pushgateway
120111
either. A sidecar bundled into an ephemeral, non-cluster Cloud Run instance has no stable,
121-
discoverable target for Prometheus to scrape, and nothing to push to instead. Its metrics
112+
discoverable target for Prometheus to scrape. Its metrics
122113
would simply never be, or with a static target configuration in Prometheus potentially too late,
123114
collected. This rules the option out.
124115

125116
**Option B (chosen) — Direct SDK export to the shared instance**
126-
Cloud Run services export via the OTel SDK's own OTLP exporter straight to
127-
`aignx-otel-gateway.aignostics.ai`, no local hop. This also matches how this same repo already
128-
handles error tracking: Sentry is integrated as in-process SDK instrumentation
129-
(`sentry_initialize()` in `boot()`), not as a sidecar agent — there's no existing precedent
130-
anywhere in the Foundry stack for a sidecar-based telemetry agent, and introducing one just for
131-
OTel would be a new, one-off operational pattern rather than an extension of how the team
132-
already does this.
117+
Cloud Run services export via the OTel SDK's own OTLP exporter straight to the gateway, no local hop.
118+
This also matches how this same repo already handles error tracking: Sentry is integrated as in-process SDK instrumentation.
133119

134120
Accepted risk: with no local buffer, telemetry the SDK hasn't flushed yet (spans queued in
135121
`BatchSpanProcessor`, metric points accumulated since the last periodic export) is lost if the
136122
Cloud Run instance is torn down first — scale-to-zero, revision replacement, or a crash. Cloud
137123
Run sends `SIGTERM` with a grace period (10s default, extendable) before `SIGKILL`; registering
138124
a shutdown hook that force-flushes the tracer/meter providers catches most of this, but it's a
139125
mitigation, not a guarantee (an abrupt kill or a grace period shorter than the flush can still
140-
drop data). Not different in kind from what a sidecar would risk on its own termination — a
141-
sidecar doesn't survive instance teardown any better than the SDK's in-process buffers do.
126+
drop data).
142127

143128
Because every service points at this one gateway endpoint rather than a vendor-specific one,
144129
the export destination is switchable later (e.g. to Grafana Cloud or another vendor) by
145-
reconfiguring only the gateway's own exporters. No change to any Cloud Run service's
146-
instrumentation. A sidecar or GCP-native export would each hard-code a specific destination
147-
directly into every service instead.
130+
reconfiguring only the gateway's own exporters.
148131

149132
### Alternatives considered — Gateway Ingress
150133

@@ -160,10 +143,6 @@ elegance for this one gateway.
160143
Contour's `HTTPProxy` has a native per-route `services[].protocol: tls` field — no
161144
backend-protocol annotation needed. Required adding `shared-tools` to the `contour-internal`
162145
ApplicationSet (envoy-internal IngressClass), which didn't run there before.
163-
This isn't just a local trade-off for `shared-tools`: Contour is the company's chosen
164-
long-term ingress controller, replacing ingress-nginx across the fleet, so this is consistent
165-
with (and a small step towards) where every cluster is headed, not a one-off choice for this
166-
gateway.
167146

168147

169148
### Alternatives considered — k8s metadata enrichment for a non-k8s source
@@ -186,13 +165,12 @@ Wrap the Cloud Run service's startup command with the `opentelemetry-instrument`
186165
configure entirely through environment variables.
187166
Rejected: the actual Cloud Run startup command is generated per-service by the
188167
`foundry-python` Copier *template* (`deployment/cloudrun/service.template.yaml.jinja`), which
189-
only affects newly generated or explicitly re-templated (`copier update`) services. Unlike a
190-
library dependency, there's no single version bump that retroactively reaches every already
191-
generated service. Additionally this kind of instrumentation only provides us predefined options.
168+
only affects newly generated or explicitly re-templated (`copier update`) services.
169+
Additionally this kind of instrumentation only provides predefined instrumentation and helps in case of lack of codeownership.
192170

193171
**Option B (chosen) — Programmatic init inside this repo's `boot()`**
194172
Add `otel_initialize()` (`aignostics_foundry_core.otel`), following the pattern already
195-
established for Sentry (`sentry_initialize()`): a pydantic-settings-driven `enabled` flag,
173+
established for Sentry: a pydantic-settings-driven `enabled` flag,
196174
called once from `boot()`, which every generated Foundry service already calls before
197175
instantiating FastAPI/uvicorn. One `aignostics-foundry-core` version bump reaches every service
198176
that upgrades its dependency — no per-service code or template changes for process-level
@@ -207,11 +185,7 @@ template wiring is a separate, small follow-up in that repo.
207185
**Option A — One `enabled` flag governing traces, metrics, and logs together**
208186
Simplest mental model, matches the original `sentry_initialize()`-style single switch.
209187
Rejected: log volume and cost characteristics differ meaningfully from traces/metrics (far
210-
higher per-request volume in most services), and there's no first-party loguru↔OTel
211-
integration — the logs bridge (a loguru sink converting into OTel's `LoggingHandler`) is
212-
custom code, not a vendor-supported path. Bundling all three risks surprising volume/cost for
213-
a service that only wanted tracing, and gives no way to turn off just one signal without
214-
losing the others.
188+
higher per-request volume in most services).
215189

216190
**Option B (chosen) — Independent per-signal toggles under a master switch**
217191
`enabled` remains the overall kill switch; `traces_enabled` and `metrics_enabled` default to
@@ -221,23 +195,15 @@ affecting the others.
221195

222196
### GCP-native observability — not a considered alternative
223197

224-
Whether to use GCP-native observability (Cloud Trace / Cloud Monitoring / Cloud Logging, which
225-
Cloud Run integrates with automatically) instead of the shared Tempo/Loki/Prometheus stack was
226-
never a live fork in this decision: [OP-3108](https://aignx.atlassian.net/browse/OP-3108)
227-
explicitly scoped this to sending Cloud Run telemetry into the existing shared stack, matching
228-
every other service in the portfolio.
229-
230-
It's also worth noting this ADR doesn't actually preclude it either way: since instrumentation
231-
is configured entirely through the standard `OTEL_EXPORTER_OTLP_ENDPOINT` environment variable
232-
(see below), any individual service remains free to point its SDK at Google's own OTLP-compatible
233-
endpoint instead of `aignx-otel-gateway.aignostics.ai` — that's an instrumentation-level
234-
configuration choice, not something requiring different infrastructure or a different decision
235-
here.
198+
It's also worth noting this ADR doesn't exclude it, since instrumentation
199+
is configured entirely through the standard `OTEL_EXPORTER_OTLP_ENDPOINT` environment variable.
200+
any individual service remains free to point its SDK at Google's own OTLP-compatible
201+
endpoint instead of our solution.
236202

237203
## Decision
238204

239205
Deploy `collector-push-gateway` (loadbalancing mode) on `shared-tools` as `aignx-otel-gateway`,
240-
exposed only internally via a Contour `HTTPProxy` at `aignx-otel-gateway.aignostics.ai`
206+
exposed only internally via a Contour `HTTPProxy`
241207
(reachable from Cloud Run over the shared VPC via Direct VPC egress, never publicly), backed by
242208
an explicit `cert-manager.io/v1 Certificate` and with `k8sAttributes.enabled: false` for this
243209
instance only.
@@ -279,23 +245,9 @@ individually. No sidecar collector, no CLI-wrapper instrumentation, no changes t
279245
automatic, only process-level tracing/metrics/manually-created spans are.
280246
- Reachability isn't guaranteed for any Foundry Cloud Run service today (see Network
281247
reachability above) — closing that gap is out of scope for this ADR, tracked as follow-up.
282-
- The gateway starts with conservative replica counts (1 push-gateway / 1 loadbalancer-backend)
248+
- The gateway starts with conservative replica counts (2 push-gateway / 2 loadbalancer-backend)
283249
since this is a new, currently-zero-traffic path — will need revisiting once real traffic
284250
lands.
285251
- Logs being opt-in means a service that only sets `{PROJECT}_OTEL_ENABLED=true` will not see
286252
its logs in Loki via this path — a support/documentation risk if not made clear in onboarding
287253
material (the companion runbook covers this).
288-
289-
## Follow-ups
290-
291-
- Make Direct VPC egress unconditional in the `foundry-python` template's Cloud Run manifest
292-
(remove the Cloud SQL/Memorystore gating) — see Network reachability above. In review as
293-
[foundry-python#385](https://github.com/aignostics/foundry-python/pull/385), with a companion
294-
fix in [foundry-infrastructure#10](https://github.com/aignostics/foundry-infrastructure/pull/10)
295-
(the `project.hcl` placeholders that workflow now unconditionally reads were under the same
296-
gate).
297-
- Wire `instrument_fastapi(app)` into `Service.api()` in the `foundry-python` template — see
298-
Harder/risks. Also in [foundry-python#385](https://github.com/aignostics/foundry-python/pull/385).
299-
- Revisit collector replica counts once real Cloud Run traffic is flowing — see Harder/risks.
300-
- Evaluate enabling `jwtProviders` on the gateway's HTTPProxy (or an OTel-side mechanism) — see
301-
the authentication note in Context above.

0 commit comments

Comments
 (0)