Skip to content

Add app_id and org_id to metrics payloads - #5130

Merged
clouvet merged 2 commits into
masterfrom
metrics-app-org-ids
Aug 26, 2026
Merged

Add app_id and org_id to metrics payloads#5130
clouvet merged 2 commits into
masterfrom
metrics-app-org-ids

Conversation

@clouvet

@clouvet clouvet commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds app_id and org_id (internal numeric IDs) alongside the existing app name / org slug fields on the deploy, launch, and command-stats telemetry payloads, so they can flow through to Snowflake for analytics.
  • All new fields are omitempty, so older flyctl builds and any code path that doesn't populate them send the same payload as today.
  • Requires a companion PR to flyctl-metrics (the receiver) to persist these fields. Until that lands, the new fields are silently ignored by the receiver since it decodes with json.Decoder and no DisallowUnknownFields.

Deploy payload

DeployStatusPayload.AppID/OrgID are populated in deployToMachines() from the *flaps.App already fetched for the deploy (app.InternalNumericID, app.Organization.InternalNumericID).

Launch payload

LaunchStatusPayload.AppID/OrgID are populated from the app created in launchState.createApp(). Since app creation happens in a different function than where the metrics payload is built in cmd.go, the IDs are stashed on launchState (appID/orgID) at creation time and copied onto status right after state.Launch(ctx) returns. If --no-create is used (no app creation), these stay empty as before.

Command stats payload

Added metrics.WithAppID/WithOrgID context helpers and wired RecordCommandFinish to read AppID/OrgID from context. However, nothing currently calls the setters: command.RequireAppName only resolves the app name (no network call), and metrics.RecordCommandContext captures the context once, early, before app resolution would happen. Populating this properly needs a GetApp call threaded through the preparer chain (a real behavior/perf change beyond the scope here), so it's left for a follow-up PR. The plumbing is in place for that follow-up.

Test plan

  • go build ./...
  • go vet ./...
  • Manual verification against a local flyctl-metrics receiver once the companion PR lands

Sprite and others added 2 commits August 26, 2026 08:21
Send the internal numeric app and org IDs alongside the existing
name/slug fields in the deploy, launch, and command-stats telemetry
payloads, so they can flow through to Snowflake for analytics. Fields
are omitempty so older flyctl behavior is unchanged for the receiver.

- Deploy: populated from the flaps.App returned during deployToMachines.
- Launch: populated from the app created in createApp, threaded through
  launchState since app creation happens in a different function than
  where the metrics payload is built.
- Command stats: added context plumbing (WithAppID/WithOrgID) and wired
  RecordCommandFinish to read from it, but nothing currently populates
  it — RequireAppName only resolves the app name, not the numeric ID,
  and command context is captured once early via RecordCommandContext,
  so populating this needs a GetApp call threaded through the preparer
  chain. Left for a follow-up PR.

Requires a companion flyctl-metrics PR to persist these fields; until
then the receiver silently ignores them (json.Decoder without
DisallowUnknownFields).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@clouvet
clouvet requested a review from dangra August 26, 2026 10:22

@dangra dangra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm!

@clouvet
clouvet merged commit 6fb1094 into master Aug 26, 2026
27 checks passed
@clouvet
clouvet deleted the metrics-app-org-ids branch August 26, 2026 15:31
clouvet added a commit that referenced this pull request Aug 28, 2026
The command stats payload gained app_id/org_id fields in #5130, but
nothing ever populated them. Populate them from the scope our tokens
already carry, which costs no API call and needs no app lookup.

Tokens hold one macaroon per org the user belongs to, so an org is only
reported when every permission macaroon agrees on one. Single-org tokens
-- CI and deploy tokens, and users in a single org -- therefore report an
org, while a token spanning several reports none rather than guessing.
Apps are only reported by tokens narrowed to exactly one app, so app_id
stays empty far more often than org_id. Both fields are omitempty, so an
invocation we can't attribute sends what it sends today.

Deriving this from the tokens rather than from a fetched app is what
makes it worth doing: only 5 of the 39 app-scoped command packages ever
call GetApp, and those overlap with deploy and launch, which already
report both IDs on their own payloads. The remaining 34 -- status, logs,
ssh, scale, machine, volumes and friends -- only ever deal in an app
name, and command stats is the sole event covering them.

Also replaces the WithAppID/WithOrgID context helpers from #5130. They
could never have worked: RecordCommandFinish reads a package-global
context that startMetrics snapshots before any app is resolved, and a
context derived later is a different value the global never sees. A
package-level setter matches how IsUsingGPU is already reported.

Co-authored-by: Sprite <noreply@sprites.dev>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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