Skip to content

Point event sinks at the identity a fork actually takes - #319

Merged
sjmiller609 merged 6 commits into
mainfrom
hypeship/fork-identity-telemetry
Jul 30, 2026
Merged

Point event sinks at the identity a fork actually takes#319
sjmiller609 merged 6 commits into
mainfrom
hypeship/fork-identity-telemetry

Conversation

@sjmiller609

@sjmiller609 sjmiller609 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Problem

A forked instance boots carrying the identity of the instance it was forked from, and only learns its own when the control plane POSTs /internal/fork-identity. The S2 writer captured its stream from env at startup, so events from a fork were appended to the source instance's stream — a stream named for an instance that no longer exists once the source is discarded. Verified on a dev host: after a successful apply, the api process still held the seed's S2_STREAM while the guest's config endpoint and rendered envoy bootstrap had the fork's.

#316 fixed the OTLP half of this by resolving identity per use. S2 can't work that way: the append session binds a stream when it starts.

Change

forkIdentityHandler gained an onApplied callback. An instance holding for a fork identity now defers its S2 writer until the identity arrives, then opens it against that stream — off the request path, since opening the session dials with no deadline.

The stream also resolves from an already-applied identity at startup, so it survives an api restart (autorestart=true), gated on the wrapper's ready file: the wrapper clears the applied marker before writing that file, so a marker without one predates this boot's wait and binding to it would pin the writer to an identity nothing will use.

Payload keys go through forkidentity.Env, the same mapping the wrapper applies.

Validated

Ran both the pre-change and branch binaries standalone in a live browser VM
(spare ports, untouched live api), with a script playing the wrapper's part:
writing the applied-identity marker and ready file where and when the wrapper
does. S2 credentials were fake — under test is which stream gets bound and
when, which is logged before the session opens.

before after
bound at boot, holding for identity SEEDSTREAM nothing
after identity handoff SEEDSTREAM FORKSTREAM

Also confirmed: restart with the applied marker + ready file rebinds
FORKSTREAM rather than deferring forever; a marker without the ready file is
ignored; non-fork instances still bind the env stream at boot.

Notes

  • Behavior change: a sink that fails to open, or a malformed wait flag, now logs instead of os.Exit(1). Under supervisord autorestart, exiting over a bad basin or a provisioning typo crashloops the VM rather than just losing telemetry. This matches how Authenticate OTLP exports with the instance JWT, refreshed on fork #316's provider treats the same flag.
  • No sender includes s2_stream yet; without it the writer falls back to the env stream (today's behavior) rather than emitting nothing.

+120 non-test lines. cmd/... and lib/events suites, go vet, and -race pass.


Note

Medium Risk
Changes when and which S2 stream events land on for forked instances and alters startup/shutdown around async S2 dial; mis-handoff could delay or mis-route telemetry but browser availability is intentionally preserved over hard exits.

Overview
Fixes forked VMs appending telemetry to the source instance's S2 stream by deferring the S2 writer until the guest's identity is applied, then binding the append session to that stream.

forkIdentityHandler now accepts an onApplied callback (non-blocking) invoked after the applied marker is confirmed. The API wires this to start the S2 writer asynchronously using s2_stream from the payload via forkidentity.Env, with env fallback.

At startup, appliedS2Stream picks the stream from an already-applied fork identity (validated against payload + ready file) so an api supervisord restart keeps the fork's stream instead of the seed env. Instances still waiting for handoff skip starting S2 until the callback runs.

Behavior change: S2 start failures and invalid fork-identity wait flags log and continue instead of os.Exit(1), avoiding VM crashloops on bad telemetry config. Shutdown waits on in-flight S2 starts (with timeout) before draining the writer.

Reviewed by Cursor Bugbot for commit e0708cc. Bugbot is set up for automated code reviews on this repo. Configure here.

A forked instance boots carrying the identity of the instance it was
forked from and only learns its own when the control plane posts one.
The api captured S2_STREAM and INSTANCE_NAME from env at startup, so
events from a fork were written to the source instance's S2 stream and
labeled with its name in OTLP — a name that no longer exists once the
source is discarded.

Retarget both sinks when the identity is applied. The S2 append session
binds to one stream at Start, so an instance holding for a fork identity
now defers its writer until the identity that owns the events arrives;
OTLP builds its Resource at Start, which for a fork is after the
handoff, so updating the config is enough.

Verified on a dev host: after a 204 apply, the api process still held
S2_STREAM=SEEDSTREAM and INSTANCE_NAME=<seed> while the guest's own
config endpoint and rendered envoy bootstrap had the fork's identity.

Co-Authored-By: Claude <noreply@anthropic.com>
@sjmiller609
sjmiller609 marked this pull request as ready for review July 28, 2026 15:49
Comment thread server/cmd/api/main.go Outdated
Comment thread server/cmd/api/main.go Outdated
Comment thread server/cmd/api/fork_identity.go
Review follow-ups on the fork identity retarget:

- An applied fork identity now resolves at startup from the payload the
  guest took, not just from the live callback. kernel-images-api runs
  with autorestart=true, so a restart after the handoff would otherwise
  leave S2 off and OTLP labeled with the source instance for the rest of
  the instance's life, with no second handoff coming.
- Opening the S2 append session dials with no deadline, so it no longer
  runs inline in the handoff: the identity POST must not depend on an
  optional sink being reachable.
- A failed writer start releases the slot again so a later identity can
  still open one.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread server/cmd/api/main.go
Comment thread server/cmd/api/main.go Outdated
Two more review follow-ups:

- The deferred S2 start can now overlap shutdown, and Start holds the
  writer's lock across an append-session dial that has no deadline, so
  Stop would block behind it. Shutdown waits briefly for a start to
  settle and skips the drain otherwise, which at worst loses the events
  of a writer that was never serving.
- A sink that cannot open no longer exits the process. The api runs under
  supervisord with autorestart, so exiting over a misconfigured basin or
  token would crashloop the VM instead of just losing telemetry. This
  makes the boot path agree with the fork path, which already logs.

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread server/cmd/api/main.go
sjmiller609 and others added 2 commits July 28, 2026 17:36
This process starts before the wrapper enters the fork identity wait, and
entering it clears the applied marker before writing the ready file. An
applied marker with no ready file therefore predates the wait the wrapper
is about to start, and binding the sinks to it would pin them to an
identity nothing is going to use — the writer is bound once, so the real
handoff would then no-op.

Require the ready file alongside the marker, which is only true once the
wrapper has armed and applied in this boot.

Co-Authored-By: Claude <noreply@anthropic.com>
#316 landed a dynamic OTLP identity provider that resolves the instance
JWT and resource attributes per use from the applied fork-identity
payload. That supersedes the SetIdentity setter this branch added, so the
setter and its test are gone and the OTLP wiring is main's.

What remains here is the S2 half, which #316 does not cover: an append
session binds a stream when it starts, so it cannot be resolved lazily.
The writer is still deferred until an identity arrives and still reads an
already-applied identity at startup — narrowed to just the stream, and
renamed to appliedS2Stream so it no longer shadows the OTLP provider.

Also aligned the wait-flag handling with the merged provider: warn and
treat as disabled rather than exiting, since exiting crashloops the VM.

Co-Authored-By: Claude <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 91e0ee5. Configure here.

Comment thread server/cmd/api/main.go
The previous comment claimed OTLP resolves identity per use, which is only
true of the credential: the resource attributes are read when the exporter
is built. Export is turned on per session, after the handoff, so a fork
stamps its own attributes — but an export started before then keeps the
source's until it is restarted, and the comment should not imply
otherwise.

Co-Authored-By: Claude <noreply@anthropic.com>
@sjmiller609
sjmiller609 requested a review from archandatta July 29, 2026 19:23
@sjmiller609
sjmiller609 merged commit 759635d into main Jul 30, 2026
11 checks passed
@sjmiller609
sjmiller609 deleted the hypeship/fork-identity-telemetry branch July 30, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants