Report the app name with command stats - #5136
Open
clouvet wants to merge 1 commit into
Open
Conversation
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.
Follows #5134, which populated
app_id/org_idoncommand/statsfrom token scope.The gap
config.ScopedIDsonly 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's nearly every human at a business — exactly the population we most want to attribute.Worth being precise: this is a property of the credential, not the person.
fetchOrgTokensearly-returns whent.FromFile() == "", so a token supplied through the environment never fans out. Tightly scoped tokens used for CI and agentic work already report both IDs today and are unaffected by this change.The change
Send the app name rather than trying harder to resolve an ID on the client.
RequireAppNamealready resolves it from--app/FLY_APP/fly.tomlwith 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're holding. The warehouse already resolves names this way for deploys and launches (againstraw_web_pg.appsandbase__flyio__organizations), so this needs no new lookup anywhere.Coverage is the 40 app-scoped command packages — including
status,logs,ssh,scale,machine,volumes,ips,certificatesandconfig, wherecommand/statsis the only event covering the invocation.Why this complements the IDs rather than replacing them
Resolution downstream should prefer the event's own id and fall back to the name — the two-step pattern the existing staging models already use, rather than coalescing output columns.
Alternatives considered
GetAppcall in a shared preparer. Adds a network round trip to the highest-volume event we have (~2.09M command invocations/day vs ~50k deploys/day) and a network failure mode toRequireAppName, which today does no I/O.FlapsClient.GetApp. Free, but only covers commands that already fetch an app — which overlap with deploy and launch, and notably excludestatus/logs/ssh. Doesn't fix the multi-org case at all.Follow-ups (not in this PR)
flyctl-metricsmust restore thesendToRudderstack("flyctl_command_stats", …)block —command/statsis currently accepted but not forwarded, so nothing on this payload reaches Snowflake yet.stg_rudderstack__flyctl_commandsdbt model doing the same deduped, id-first resolution as the deploys/launches models. Name joins againstraw_web_pg.appsmust be deduped (a deleted app frees its name) or they silently fan out.org_id_sourceprovenance column. Attribution mechanism correlates with interactive-vs-automation use, so an unlabelled mix will quietly bias any comparison across those segments.Testing
go build ./internal/...,go vet,go test ./internal/metrics/... ./internal/command/ ./internal/config/...all pass;golangci-lint v2.11.3(the pinned CI version) reports0 issues.🤖 Generated with Claude Code