mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: report user secrets adoption summary in telemetry (#24854)
Add a deployment-wide user secrets summary to the telemetry snapshot so we can track adoption of user secrets The summary reports: - A breakdown of secrets by which injection fields are populated: EnvNameOnly, FilePathOnly, Both, Neither - The distribution of secrets per user (max, p25, p50, p75, p90) All metrics are scoped to active non-system users. Soft-deleted users are excluded. The percentile distribution is computed across the entire active non-system user base, including users with zero secrets, so the percentiles reflect deployment-wide adoption. Assisted by Coder Agents.
This commit is contained in:
Zach
@@ -822,6 +822,18 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) {
|
||||
}
|
||||
return nil
|
||||
})
|
||||
eg.Go(func() error {
|
||||
summary, err := r.collectUserSecretsSummary(ctx)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("collect user secrets summary: %w", err)
|
||||
}
|
||||
// summary is nil when another replica already claimed the
|
||||
// telemetry lock for this period.
|
||||
if summary != nil {
|
||||
snapshot.UserSecretsSummary = summary
|
||||
}
|
||||
return nil
|
||||
})
|
||||
|
||||
err := eg.Wait()
|
||||
if err != nil {
|
||||
@@ -952,6 +964,49 @@ func (r *remoteReporter) collectBoundaryUsageSummary(ctx context.Context) (*Boun
|
||||
}, nil
|
||||
}
|
||||
|
||||
// collectUserSecretsSummary returns a deployment-wide aggregate of user
|
||||
// secrets configuration. Returns nil if another replica has already
|
||||
// collected for this period.
|
||||
//
|
||||
// The summary has no natural per-row UUID for the telemetry server to
|
||||
// de-duplicate on, so we elect a single replica per snapshot period
|
||||
// via the telemetry_locks table.
|
||||
func (r *remoteReporter) collectUserSecretsSummary(ctx context.Context) (*UserSecretsSummary, error) {
|
||||
// Claim the telemetry lock for this period. Use snapshot frequency so
|
||||
// each telemetry snapshot period gets exactly one collection across
|
||||
// replicas.
|
||||
periodEndingAt := dbtime.Time(r.options.Clock.Now()).UTC().Truncate(r.options.SnapshotFrequency)
|
||||
err := r.options.Database.InsertTelemetryLock(ctx, database.InsertTelemetryLockParams{
|
||||
EventType: "user_secrets_summary",
|
||||
PeriodEndingAt: periodEndingAt,
|
||||
})
|
||||
if database.IsUniqueViolation(err, database.UniqueTelemetryLocksPkey) {
|
||||
r.options.Logger.Debug(ctx, "user secrets telemetry lock already claimed by another replica, skipping", slog.F("period_ending_at", periodEndingAt))
|
||||
return nil, nil //nolint:nilnil // This is simple to handle when dealing with telemetry.
|
||||
}
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("insert user secrets telemetry lock (period_ending_at=%q): %w", periodEndingAt, err)
|
||||
}
|
||||
|
||||
row, err := r.options.Database.GetUserSecretsTelemetrySummary(ctx)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get user secrets telemetry summary: %w", err)
|
||||
}
|
||||
return &UserSecretsSummary{
|
||||
UsersWithSecrets: row.UsersWithSecrets,
|
||||
TotalSecrets: row.TotalSecrets,
|
||||
EnvNameOnly: row.EnvNameOnly,
|
||||
FilePathOnly: row.FilePathOnly,
|
||||
Both: row.Both,
|
||||
Neither: row.Neither,
|
||||
SecretsPerUserMax: row.SecretsPerUserMax,
|
||||
SecretsPerUserP25: row.SecretsPerUserP25,
|
||||
SecretsPerUserP50: row.SecretsPerUserP50,
|
||||
SecretsPerUserP75: row.SecretsPerUserP75,
|
||||
SecretsPerUserP90: row.SecretsPerUserP90,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func CollectTasks(ctx context.Context, db database.Store) ([]Task, error) {
|
||||
dbTasks, err := db.ListTasks(ctx, database.ListTasksParams{
|
||||
OwnerID: uuid.Nil,
|
||||
@@ -1554,6 +1609,7 @@ type Snapshot struct {
|
||||
ChatMessageSummaries []ChatMessageSummary `json:"chat_message_summaries"`
|
||||
ChatModelConfigs []ChatModelConfig `json:"chat_model_configs"`
|
||||
ChatDiffStatusSummary *ChatDiffStatusSummary `json:"chat_diff_status_summary"`
|
||||
UserSecretsSummary *UserSecretsSummary `json:"user_secrets_summary"`
|
||||
}
|
||||
|
||||
// Deployment contains information about the host running Coder.
|
||||
@@ -2409,6 +2465,38 @@ type ChatDiffStatusSummary struct {
|
||||
Closed int64 `json:"closed"`
|
||||
}
|
||||
|
||||
// UserSecretsSummary contains deployment-wide aggregates about user
|
||||
// secrets. All counts are scoped to active non-system users so that
|
||||
// soft-deleted accounts, dormant or suspended users, and internal
|
||||
// subjects (e.g. the prebuilds user) do not skew the results. Status
|
||||
// transitions move users in and out of this denominator, so a
|
||||
// snapshot's UsersWithSecrets can drop without any secret being
|
||||
// deleted.
|
||||
//
|
||||
// UsersWithSecrets is the count of active non-system users that have
|
||||
// at least one secret. TotalSecrets is the count of secrets owned by
|
||||
// those users. EnvNameOnly, FilePathOnly, Both, and Neither break
|
||||
// TotalSecrets down by which injection fields are populated.
|
||||
//
|
||||
// The SecretsPerUser* fields describe the distribution of secrets per
|
||||
// user across the entire active non-system user base, including users
|
||||
// with zero secrets, so the percentiles reflect deployment-wide
|
||||
// adoption rather than only the power-user subset. Max and Px are the
|
||||
// maximum and the 25th, 50th, 75th, and 90th percentiles.
|
||||
type UserSecretsSummary struct {
|
||||
UsersWithSecrets int64 `json:"users_with_secrets"`
|
||||
TotalSecrets int64 `json:"total_secrets"`
|
||||
EnvNameOnly int64 `json:"env_name_only"`
|
||||
FilePathOnly int64 `json:"file_path_only"`
|
||||
Both int64 `json:"both"`
|
||||
Neither int64 `json:"neither"`
|
||||
SecretsPerUserMax int64 `json:"secrets_per_user_max"`
|
||||
SecretsPerUserP25 int64 `json:"secrets_per_user_p25"`
|
||||
SecretsPerUserP50 int64 `json:"secrets_per_user_p50"`
|
||||
SecretsPerUserP75 int64 `json:"secrets_per_user_p75"`
|
||||
SecretsPerUserP90 int64 `json:"secrets_per_user_p90"`
|
||||
}
|
||||
|
||||
func ConvertAIBridgeInterceptionsSummary(endTime time.Time, provider, model, client string, summary database.CalculateAIBridgeInterceptionsTelemetrySummaryRow) AIBridgeInterceptionsSummary {
|
||||
return AIBridgeInterceptionsSummary{
|
||||
ID: uuid.New(),
|
||||
|
||||
@@ -1998,3 +1998,260 @@ func TestChatDiffStatusSummaryTelemetry(t *testing.T) {
|
||||
assert.Equal(t, int64(2), snapshot2.ChatDiffStatusSummary.Merged)
|
||||
assert.Equal(t, int64(1), snapshot2.ChatDiffStatusSummary.Closed)
|
||||
}
|
||||
|
||||
func TestUserSecretsTelemetry(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("Empty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
|
||||
// Empty deployment should report a non-nil summary with zeros.
|
||||
_, snap := collectSnapshot(ctx, t, db, nil)
|
||||
require.Equal(t, &telemetry.UserSecretsSummary{}, snap.UserSecretsSummary)
|
||||
})
|
||||
|
||||
t.Run("ConfigurationBreakdown", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
|
||||
userA := dbgen.User(t, db, database.User{})
|
||||
userB := dbgen.User(t, db, database.User{})
|
||||
|
||||
// userA: env-only and file-only. dbgen.UserSecret defaults
|
||||
// EnvName and FilePath to non-empty, so use mutators to clear
|
||||
// them where the test wants empty values.
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: userA.ID,
|
||||
Name: "a-env",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = "A_ENV"
|
||||
p.FilePath = ""
|
||||
})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: userA.ID,
|
||||
Name: "a-file",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = "/home/coder/a.file"
|
||||
})
|
||||
// userB: both and neither.
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: userB.ID,
|
||||
Name: "b-both",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = "B_BOTH"
|
||||
p.FilePath = "/home/coder/b.both"
|
||||
})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: userB.ID,
|
||||
Name: "b-neither",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = ""
|
||||
})
|
||||
|
||||
_, snap := collectSnapshot(ctx, t, db, nil)
|
||||
// Each user has exactly two secrets, so every percentile and
|
||||
// the max collapse to 2.
|
||||
require.Equal(t, &telemetry.UserSecretsSummary{
|
||||
UsersWithSecrets: 2,
|
||||
TotalSecrets: 4,
|
||||
EnvNameOnly: 1,
|
||||
FilePathOnly: 1,
|
||||
Both: 1,
|
||||
Neither: 1,
|
||||
SecretsPerUserMax: 2,
|
||||
SecretsPerUserP25: 2,
|
||||
SecretsPerUserP50: 2,
|
||||
SecretsPerUserP75: 2,
|
||||
SecretsPerUserP90: 2,
|
||||
}, snap.UserSecretsSummary)
|
||||
})
|
||||
|
||||
t.Run("PercentileDistribution", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
|
||||
// Five users have secret counts 1, 2, 4, 8, 16 and five other
|
||||
// users have zero secrets. Including the zero-secret users in
|
||||
// the distribution gives a sorted vector of length 10:
|
||||
// [0, 0, 0, 0, 0, 1, 2, 4, 8, 16]
|
||||
// percentile_disc(p) returns the value at the smallest
|
||||
// 1-indexed position i where i/n >= p, so the buckets land at:
|
||||
// p25 -> position 3 -> 0
|
||||
// p50 -> position 5 -> 0
|
||||
// p75 -> position 8 -> 4
|
||||
// p90 -> position 9 -> 8
|
||||
adopters := []int{1, 2, 4, 8, 16}
|
||||
for _, n := range adopters {
|
||||
u := dbgen.User(t, db, database.User{})
|
||||
for i := 0; i < n; i++ {
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: u.ID,
|
||||
Name: fmt.Sprintf("secret-%d", i),
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
// Clear EnvName and FilePath so the unique
|
||||
// (user_id, env_name) and (user_id, file_path)
|
||||
// indexes don't collide across multiple secrets
|
||||
// for the same user.
|
||||
p.EnvName = ""
|
||||
p.FilePath = ""
|
||||
})
|
||||
}
|
||||
}
|
||||
for i := 0; i < 5; i++ {
|
||||
_ = dbgen.User(t, db, database.User{})
|
||||
}
|
||||
|
||||
_, snap := collectSnapshot(ctx, t, db, nil)
|
||||
require.Equal(t, &telemetry.UserSecretsSummary{
|
||||
UsersWithSecrets: 5,
|
||||
TotalSecrets: 31,
|
||||
EnvNameOnly: 0,
|
||||
FilePathOnly: 0,
|
||||
Both: 0,
|
||||
Neither: 31,
|
||||
SecretsPerUserMax: 16,
|
||||
SecretsPerUserP25: 0,
|
||||
SecretsPerUserP50: 0,
|
||||
SecretsPerUserP75: 4,
|
||||
SecretsPerUserP90: 8,
|
||||
}, snap.UserSecretsSummary)
|
||||
})
|
||||
|
||||
t.Run("FilterSkipsInactiveUsers", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
|
||||
// Active user with two secrets contributes the only entries
|
||||
// to UsersWithSecrets, TotalSecrets, and the percentile
|
||||
// distribution.
|
||||
active := dbgen.User(t, db, database.User{})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: active.ID,
|
||||
Name: "active-env",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = "ACTIVE_ENV"
|
||||
p.FilePath = ""
|
||||
})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: active.ID,
|
||||
Name: "active-file",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = "/home/coder/active.file"
|
||||
})
|
||||
|
||||
// Soft-deleted user. user_secrets has ON DELETE CASCADE on
|
||||
// users, but Coder soft-deletes by setting users.deleted, so
|
||||
// the secret row persists. The summary should ignore it.
|
||||
deleted := dbgen.User(t, db, database.User{Deleted: true})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: deleted.ID,
|
||||
Name: "deleted-secret",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = "DELETED_ENV"
|
||||
p.FilePath = ""
|
||||
})
|
||||
|
||||
// User secret owned by a dormant user should be excluded.
|
||||
dormant := dbgen.User(t, db, database.User{Status: database.UserStatusDormant})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: dormant.ID,
|
||||
Name: "dormant-secret",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = "DORMANT_ENV"
|
||||
p.FilePath = ""
|
||||
})
|
||||
|
||||
// User secret owned by a suspended user should be excluded.
|
||||
suspended := dbgen.User(t, db, database.User{Status: database.UserStatusSuspended})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: suspended.ID,
|
||||
Name: "suspended-secret",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = "/home/coder/suspended.file"
|
||||
})
|
||||
|
||||
// System user. Only its UUID is needed. Tying a secret to it
|
||||
// proves the is_system filter excludes it.
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: database.PrebuildsSystemUserID,
|
||||
Name: "prebuilds-secret",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = "/home/coder/prebuilds.file"
|
||||
})
|
||||
|
||||
_, snap := collectSnapshot(ctx, t, db, nil)
|
||||
require.Equal(t, &telemetry.UserSecretsSummary{
|
||||
UsersWithSecrets: 1,
|
||||
TotalSecrets: 2,
|
||||
EnvNameOnly: 1,
|
||||
FilePathOnly: 1,
|
||||
Both: 0,
|
||||
Neither: 0,
|
||||
SecretsPerUserMax: 2,
|
||||
SecretsPerUserP25: 2,
|
||||
SecretsPerUserP50: 2,
|
||||
SecretsPerUserP75: 2,
|
||||
SecretsPerUserP90: 2,
|
||||
}, snap.UserSecretsSummary)
|
||||
})
|
||||
|
||||
t.Run("OnlyOneReplicaCollects", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
|
||||
// Seed one user with one secret so the summary would normally
|
||||
// be populated. The user_secrets_summary aggregate has no
|
||||
// natural per-row UUID for the telemetry server to dedupe on,
|
||||
// so a telemetry lock elects a single replica per period.
|
||||
u := dbgen.User(t, db, database.User{})
|
||||
_ = dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: u.ID,
|
||||
Name: "only-secret",
|
||||
}, func(p *database.CreateUserSecretParams) {
|
||||
p.EnvName = ""
|
||||
p.FilePath = ""
|
||||
})
|
||||
|
||||
clock := quartz.NewMock(t)
|
||||
clock.Set(dbtime.Now())
|
||||
|
||||
// First snapshot claims the lock and reports the summary.
|
||||
_, snap1 := collectSnapshot(ctx, t, db, func(opts telemetry.Options) telemetry.Options {
|
||||
opts.Clock = clock
|
||||
return opts
|
||||
})
|
||||
require.Equal(t, &telemetry.UserSecretsSummary{
|
||||
UsersWithSecrets: 1,
|
||||
TotalSecrets: 1,
|
||||
EnvNameOnly: 0,
|
||||
FilePathOnly: 0,
|
||||
Both: 0,
|
||||
Neither: 1,
|
||||
SecretsPerUserMax: 1,
|
||||
SecretsPerUserP25: 1,
|
||||
SecretsPerUserP50: 1,
|
||||
SecretsPerUserP75: 1,
|
||||
SecretsPerUserP90: 1,
|
||||
}, snap1.UserSecretsSummary)
|
||||
|
||||
// A second snapshot in the same period simulates a second
|
||||
// replica racing to claim the lock; it should observe the
|
||||
// unique violation and skip reporting.
|
||||
_, snap2 := collectSnapshot(ctx, t, db, func(opts telemetry.Options) telemetry.Options {
|
||||
opts.Clock = clock
|
||||
return opts
|
||||
})
|
||||
require.Nil(t, snap2.UserSecretsSummary)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user