Send instance name as x-api-key to the OTLP relay#316
Conversation
Adds an optional in-VM OTLP/HTTP export sink that converts browser telemetry events into OTLP log records and forwards them to a configured endpoint, alongside the existing S2 sink. Gated on OTLP_RELAY_ENDPOINT; no behavior change when unset. - Converter maps each telemetry envelope to an OTLP log record: event type to EventName (also mirrored to kernel.event.type for backends that drop it), structured JSON body, derived severity, and kernel.* attributes. Network and console fields are promoted to semantic-convention attributes. Screenshot and monitor categories are excluded from export. - Sink mirrors S2StorageWriter: an independent ring reader feeds records through the OTel log SDK (otlploghttp + batch processor), running independently of the S2 sink. Export failures surface via a logging exporter wrapper. - Config and startup wiring, plus a dev collector config for local testing.
Rename export env vars from OTLP_RELAY_* to BTEL_OTLP_* so they don't collide with the standard OTEL_* vars that configure the API server's own telemetry, and drop the metro-api-specific "relay" naming (dev points at a collector). Rename the platform-identity config fields to InstanceJWT / InstanceName / MetroName since they aren't OTLP-specific. Default export path is now the standard /v1/logs. Make sink startup non-fatal: an optional best-effort exporter failing to start must not take down the browser, so log and continue instead of os.Exit(1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The OTel log SDK reports records dropped under sustained backpressure via its global logger at logr V(1). That logger is a no-op until wired, and even once wired it renders just below slog Info, so a default Info handler would still swallow it. Route the SDK global logger to a handler that admits that level when export is enabled, so drops surface instead of being silently discarded (which looked identical to a healthy idle sink). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TestLoad's expected structs didn't account for the OTLPPath and OTLPServiceName defaults that Load applies, so it failed once those defaults were in place. Assert the applied defaults. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The batch processor caps records per export by count (200), not bytes. With the 1MB per-envelope publish ceiling, a full batch of large records could reach ~200MB and be rejected by the collector/relay, and the SDK drops rejected records without retry. Split each export in the loggingExporter into sub-requests under a byte budget (4MiB) so a batch of large records can't exceed the target's HTTP body limit; a single record (<=1MB) always fits in one sub-request. Adds a chunk-splitting unit test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The metro-api OTLP relay authenticates a VM by its instance name checked against active sessions (mirroring the capmonster/hcaptcha relays), so send INST_NAME as x-api-key instead of the instance JWT as a bearer token. Drop the now-unused KERNEL_INSTANCE_JWT exporter config (the env still exists for the envoy wrapper; the exporter just no longer reads it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
# Conflicts: # server/cmd/api/main.go # server/cmd/config/config.go
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
# Conflicts: # server/cmd/config/config.go
The exporter round-trip test still asserted an Authorization: Bearer header; the relay authenticates by instance name sent as x-api-key, so the fixture now sends and asserts that header to document the real wire contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
I'd like to go the other way here: keep the exporter on the instance JWT and correct the relay to verify it — the relay isn't deployed yet, so this pair of PRs is the cheapest moment to set the contract we actually want, rather than standardizing further on bare instance names. the JWT already carries instance_name (+ metro) in its claims, so the relay gets everything x-api-key gives it today, plus a place to put more later.
what's blocking JWT auth today — small list:
- relay side (kernel/kernel#2763): verify the token, keep the lookups for authorization. plumb the JWT secret into metro-api, validate signature/expiry, take
instance_namefrom the claims, and then the existing active-session + otlp-binding lookups stay exactly as they are — claims tell the relay who's calling, the DB tells it what they're allowed to do. - this PR: keep sending the JWT (
Authorization: Bearer <KERNEL_INSTANCE_JWT>), i.e. roughly revert to the pre-PR header and keep theInstanceJWTconfig field. - fork-identity-wait must refresh the exporter's credential. the fork identity payload already delivers a new
KERNEL_INSTANCE_JWT, butkernel-images-apistarts before identity applies and is never restarted, and the exporter captures config once at process start — so forked VMs export with a stale/empty credential forever (true for x-api-key too, so this needs fixing regardless of which credential wins). the exporter needs to read the JWT dynamically (from the applied identity payload) rather than from env-at-boot.
future direction, briefly: re-mint the JWT at claim time with session_id / org_id / other useful instance claims, delivered through a generalized identity-update channel (the fork-identity endpoint grown into the every-VM claim-time push), and migrate capmonster/hcaptcha/extensionfeatures onto the same model. none of that blocks v1 — the boot-time JWT with the instance name claim is enough for the relay today.
cc @sjmiller609
Per review, the relay (kernel/kernel#2763) is being changed to verify the instance JWT rather than trust a bare instance name, so the exporter keeps sending Authorization: Bearer <KERNEL_INSTANCE_JWT>. This reverts the x-api-key switch and restores the InstanceJWT config field. The JWT is now resolved per request instead of frozen at process start. In fork-identity-wait mode the platform delivers a fresh KERNEL_INSTANCE_JWT in the applied identity payload after kernel-images-api has already started (and it is never restarted), so a boot-time snapshot would export with a stale or empty credential forever. A new instanceJWTProvider reads the JWT from the applied fork-identity payload (falling back to the boot env), and the exporter installs a RoundTripper that sets the bearer header from it on each request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| URLPath: config.OTLPPath, | ||
| Insecure: config.OTLPInsecure, | ||
| Headers: headers, | ||
| AuthTokenFunc: jwtProvider.Token, |
There was a problem hiding this comment.
Auth header mismatches relay contract
High Severity
The OTLP exporter sends an Authorization: Bearer token, but the relay expects an x-api-key header with the instance name. This authentication mismatch causes OTLP exports to fail, preventing session telemetry from landing.
Reviewed by Cursor Bugbot for commit a6d280d. Configure here.
Two container e2e tests against a mock OTLP collector (the S2 sink had e2e coverage; OTLP had none): - TestOTLPExport: enables export via the telemetry API and asserts records land carrying the instance JWT as a bearer token plus the instance-name resource attribute. - TestOTLPExportForkIdentityRefresh: boots in fork-identity-wait mode with an empty boot JWT, applies a fresh KERNEL_INSTANCE_JWT via the real /internal/fork-identity endpoint, and asserts exports pick up that JWT with no restart. Both skip when docker is unavailable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| forkidentity.PayloadFile = payload | ||
| forkidentity.AppliedFile = applied | ||
| }) | ||
| } |
There was a problem hiding this comment.
Duplicate fork-identity test helper
Low Severity
withForkIdentityFiles duplicates useTempForkIdentityFiles in the same main package. Both retarget forkidentity.PayloadFile / AppliedFile to a temp dir and restore them on cleanup, so future path-handling fixes can easily drift between the two helpers.
Reviewed by Cursor Bugbot for commit 3c9a8e5. Configure here.
Addresses review follow-ups on the fork-identity credential path: - Resource attributes (kernel.instance_name/metro) were still stamped from boot env, so a forked VM authenticated with the fresh JWT but labeled its telemetry with the parent/empty instance. Resolve them from the applied fork-identity payload too, via InstanceNameFunc/MetroFunc read at exporter-build time (export starts per session after identity applies). - Cache only a payload-sourced (or non-fork, stable) identity: a boot fallback during the pre-apply window is returned uncached so the fresh identity is picked up on the next call rather than pinned for the cache TTL. - Surface an invalid KERNEL_FORK_IDENTITY_WAIT instead of silently disabling the payload read. The fork-identity e2e now asserts the refreshed instance_name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1d5acf1. Configure here.
| metro := cfg.Metro | ||
| if cfg.MetroFunc != nil { | ||
| metro = cfg.MetroFunc() | ||
| } |
There was a problem hiding this comment.
Resource identity frozen after export start
Low Severity
InstanceNameFunc and MetroFunc are evaluated once when the exporter is built at Start, while AuthTokenFunc refreshes on every request. If OTLP export is enabled before fork identity applies and left running, the bearer JWT updates after apply but kernel.instance_name and kernel.metro stay at the empty or boot values for that export session. The API is already reachable in fork-wait mode before apply, so that ordering is possible.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1d5acf1. Configure here.
2 participants
Summary
Keeps the in-VM OTLP exporter on the instance JWT (
Authorization: Bearer <KERNEL_INSTANCE_JWT>) and resolves it per request so forked VMs export with a fresh credential. The relay (kernel/kernel#2763) is being changed to verify the JWT signature/expiry and readinstance_name(+metro) from its claims, then keep the existing active-session + otlp-binding lookups for authorization — claims say who's calling, the DB says what they're allowed to do.Companion to the metro-api relay PR (kernel/kernel#2763). Wire contract:
Authorization: Bearer <instance_jwt>+ POST toBTEL_OTLP_PATH(set to/otlp-relay/v1/logsby provisioning when pointing a VM at the relay).Why
The instance JWT already carries
instance_nameandmetroin its claims, so the relay gets everything a bare instance name would give it today, plus room forsession_id/org_id/etc. later. The relay isn't deployed yet, so this is the cheapest moment to set the contract we actually want rather than standardizing further on bare instance names. Long term, capmonster/hcaptcha/extensionfeatures can migrate onto the same verified-JWT model.Fork-identity credential refresh
The exporter previously captured the credential once at process start. In fork-identity-wait mode
kernel-images-apistarts before identity applies and is never restarted, and the platform delivers a freshKERNEL_INSTANCE_JWTin the applied identity payload — so a boot-time snapshot would export with a stale/empty credential forever. This PR resolves the JWT per request instead:instanceJWTProviderreadskernel_instance_jwtfrom the applied fork-identity payload (only once the applied marker is present, to avoid a torn mid-apply read), falling back to the boot env JWT; result cached briefly (3s) to avoid a filesystem read per export. Outside fork mode it returns the boot JWT unchanged.RoundTripper(viaotlploghttp.WithHTTPClient) that sets the bearer header from the provider on each request; an empty token sends no header.Changes
server/cmd/api/main.go: build the exporter with anAuthTokenFuncbacked byinstanceJWTProviderinstead of a static header map.server/cmd/api/otlp_credential.go: the per-request JWT provider (fork-payload read + fallback + TTL cache).server/lib/events/otlpstorage.go:OTLPConfig.AuthTokenFunc+bearerRoundTripper.server/cmd/config/config.go: unchanged frommain(theInstanceJWTfield stays).Test plan
go build ./...,go vet, andgo test ./cmd/config ./cmd/api/... ./lib/eventsgreen (incl.-race).TestInstanceJWTProvider(non-fork boot / fork-before-apply fallback / fork-after-apply fresh),TestOTLPStorageWriter_RefreshesAuthToken(token change mid-run reflected on later exports), revertedTestOTLPStorageWriter_ExportsEventsto assert the bearer header.Bearer <fresh jwt>with no restart.Notes
kernel.instance_name/metro) are also resolved from the applied fork-identity payload (viaInstanceNameFunc/MetroFuncread at exporter-build time), so a forked VM's telemetry is labeled with its own instance, not the parent/empty boot value. Covered by the fork-identity e2e.session_id/org_idclaims via a generalized identity-update channel, and migrate the sibling relays onto the same dynamic-credential model.