Report org and app IDs on command stats from token scope - #5134
Merged
Conversation
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: Claude Opus 5 <noreply@anthropic.com>
dangra
approved these changes
Aug 27, 2026
clouvet
added a commit
that referenced
this pull request
Sep 4, 2026
The app_id and org_id added to command stats in #5134 come from token scope, which only names an org when every permission macaroon we hold agrees on one. Tokens loaded from the config file fan out to one macaroon per org the user belongs to (fetchOrgTokens), so an interactive user in more than one org reports nothing rather than guessing. Since everyone has a personal org alongside any org they work in, that is nearly every human at a business -- exactly the population we most want to attribute. Send the app name instead of trying harder to resolve an ID on the client. RequireAppName already resolves it from the --app flag, FLY_APP or fly.toml with no I/O at all, and app names are unique, so a name identifies an app and its org regardless of how many macaroons we are holding. The warehouse already resolves names this way for deploys and launches, against raw_web_pg.apps and base__flyio__organizations, so this needs no new lookup anywhere. This complements the token-scoped IDs rather than replacing them. The IDs stay first-hand and are accurate as of the moment the command ran, where a name resolves to whichever org owns the app today; they also cover commands that have no app name at all. Roughly half our command packages are not app-scoped, and for those an org-scoped token remains the only attribution available. Resolution should prefer the event's own id and fall back to the name, which is the pattern the existing staging models already use. Note that this is a property of the credential, not the person: a token supplied through the environment skips the org fan-out entirely, so tightly scoped tokens used for CI and agentic work already report both IDs today and are unaffected by this. The field is omitempty, so a command with no app sends what it sends today. Co-authored-by: Sprite <noreply@sprites.dev> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #5130, which added
app_id/org_idto thecommand/statspayload but never populated them.Approach: derive the IDs from token scope, not from an app lookup
Our tokens are already scoped to an org, and sometimes to an app.
flyio.OrganizationScope/flyio.AppScoperead that straight out of the macaroon caveats, so this costs no API call and works for commands that never fetch an app. Measured at ~5.8µs per invocation.The alternative — calling
GetAppduring command setup — looks appealing but inverts the value:RequireAppName/LoadAppNameIfPresent*)GetAppapps,deploy,mcp,registry,secrets)Those 5 overlap heavily with
deploy/launch, which already report both IDs on their own payloads. The other 34 —status,logs,ssh,scale,machine,volumes,ips,certificates,config,console, … — only ever deal in an app name, andcommand/statsis the only event covering them. So aGetApp-based approach would mostly duplicate IDs we already have, add a network round trip to every invocation of the highest-volume event we emit, and introduce a network failure mode intoRequireAppName, which today does no I/O at all.Expected coverage
org_idis reported when every permission macaroon agrees on one org:fetchOrgTokensdeliberately holds one macaroon per org, so there is no correct single answer without knowing the app; we emit nothing rather than guess and mis-attribute.app_idis only reported by tokens narrowed to exactly one app (fly tokens create deploy -a …), so expect it to be empty far more often thanorg_id.AppScopealready returnsnilfor a wildcard token, so a non-empty result always names specific apps.Both fields are
omitempty, so an invocation we can't attribute sends exactly what it sends today.Also: removes the context helpers from #5130
WithAppID/WithOrgID/AppIDFromContext/OrgIDFromContextcould never have worked.RecordCommandFinishreads the package-globalcommandContext, whichstartMetricssnapshots once — 11th of 12 incommonPreparers, before auth and before any command-specific preparer runs. Contexts are immutable, soctx = metrics.WithAppID(ctx, …)later produces a different context the global never sees. Replaced with a mutex-guarded package-level setter, matching howmetrics.IsUsingGPUis already reported fromcommand.go.flyctl-metrics PR #70 accepts these fields on
command/statsbut deliberately does not forward that endpoint to Rudderstack, because the IDs weren't populated yet. Landing this without restoring thego sendToRudderstack("flyctl_command_stats", …)block in flyctl-metricsmain.go(it's in commitc500ecc) means the IDs still never reach Snowflake. I have not touched that repo.Test plan
go build ./...go vet ./...go test ./internal/config/... ./internal/command/TestScopedIDscovering single-org, multi-org, OAuth-only, nil, single-app and multi-app tokensFlyV1 fm2_…token: org ID extracted, zero network calls