Skip to content

Commit 82ea8e1

Browse files
deansheatherdeansheather
committed
PR comments
1 parent d431a3e commit 82ea8e1

25 files changed

+360
-249
lines changed

‎CODEOWNERS

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,5 @@ coderd/schedule/autostop.go @deansheather @DanielleMaywood
3636

3737
# Usage tracking code requires intimate knowledge of Tallyman and Metronome, as
3838
# well as guidance from revenue.
39-
coderd/usage/ @deansheather
40-
enterprise/coderd/usage/ @deansheather
39+
coderd/usage/ @deansheather @spikecurtis
40+
enterprise/coderd/usage/ @deansheather @spikecurtis

‎coderd/database/check_constraint.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,24 @@ var (
509509
}),
510510
Scope: rbac.ScopeAll,
511511
}.WithCachedASTValue()
512+
513+
subjectUsageTracker = rbac.Subject{
514+
Type: rbac.SubjectTypeUsageTracker,
515+
FriendlyName: "Usage Tracker",
516+
ID: uuid.Nil.String(),
517+
Roles: rbac.Roles([]rbac.Role{
518+
{
519+
Identifier: rbac.RoleIdentifier{Name: "usage-tracker"},
520+
DisplayName: "Usage Tracker",
521+
Site: rbac.Permissions(map[string][]policy.Action{
522+
rbac.ResourceUsageEvent.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate},
523+
}),
524+
Org: map[string][]rbac.Permission{},
525+
User: []rbac.Permission{},
526+
},
527+
}),
528+
Scope: rbac.ScopeAll,
529+
}.WithCachedASTValue()
512530
)
513531

514532
// AsProvisionerd returns a context with an actor that has permissions required
@@ -579,10 +597,18 @@ func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
579597
return As(ctx, subjectPrebuildsOrchestrator)
580598
}
581599

600+
// AsFileReader returns a context with an actor that has permissions required
601+
// for reading all files.
582602
func AsFileReader(ctx context.Context) context.Context {
583603
return As(ctx, subjectFileReader)
584604
}
585605

606+
// AsUsageTracker returns a context with an actor that has permissions required
607+
// for creating, reading, and updating usage events.
608+
func AsUsageTracker(ctx context.Context) context.Context {
609+
return As(ctx, subjectUsageTracker)
610+
}
611+
586612
var AsRemoveActor = rbac.Subject{
587613
ID: "remove-actor",
588614
}
@@ -3952,7 +3978,7 @@ func (q *querier) InsertTemplateVersionWorkspaceTag(ctx context.Context, arg dat
39523978
}
39533979

39543980
func (q *querier) InsertUsageEvent(ctx context.Context, arg database.InsertUsageEventParams) error {
3955-
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
3981+
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceUsageEvent); err != nil {
39563982
return err
39573983
}
39583984
return q.db.InsertUsageEvent(ctx, arg)
@@ -4315,7 +4341,7 @@ func (q *querier) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string)
43154341

43164342
func (q *querier) SelectUsageEventsForPublishing(ctx context.Context, arg time.Time) ([]database.UsageEvent, error) {
43174343
// ActionUpdate because we're updating the publish_started_at column.
4318-
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
4344+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceUsageEvent); err != nil {
43194345
return nil, err
43204346
}
43214347
return q.db.SelectUsageEventsForPublishing(ctx, arg)
@@ -4803,7 +4829,7 @@ func (q *querier) UpdateTemplateWorkspacesLastUsedAt(ctx context.Context, arg da
48034829
}
48044830

48054831
func (q *querier) UpdateUsageEventsPostPublish(ctx context.Context, arg database.UpdateUsageEventsPostPublishParams) error {
4806-
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
4832+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceUsageEvent); err != nil {
48074833
return err
48084834
}
48094835
return q.db.UpdateUsageEventsPostPublish(ctx, arg)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5668,25 +5668,32 @@ func (s *MethodTestSuite) TestUserSecrets() {
56685668
}
56695669

56705670
func (s *MethodTestSuite) TestUsageEvents() {
5671-
s.Run("InsertUsageEvent", s.Subtest(func(db database.Store, check *expects) {
5672-
check.Args(database.InsertUsageEventParams{
5671+
s.Run("InsertUsageEvent", s.Mocked(func(db *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
5672+
params := database.InsertUsageEventParams{
56735673
ID: "1",
5674-
EventType: database.UsageEventTypeDcManagedAgentsV1,
5674+
EventType: "dc_managed_agents_v1",
56755675
EventData: []byte("{}"),
56765676
CreatedAt: dbtime.Now(),
5677-
}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
5677+
}
5678+
db.EXPECT().InsertUsageEvent(gomock.Any(), params).Return(nil)
5679+
check.Args(params).Asserts(rbac.ResourceUsageEvent, policy.ActionCreate)
56785680
}))
56795681

5680-
s.Run("SelectUsageEventsForPublishing", s.Subtest(func(db database.Store, check *expects) {
5681-
check.Args(dbtime.Now()).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
5682+
s.Run("SelectUsageEventsForPublishing", s.Mocked(func(db *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
5683+
now := dbtime.Now()
5684+
db.EXPECT().SelectUsageEventsForPublishing(gomock.Any(), now).Return([]database.UsageEvent{}, nil)
5685+
check.Args(now).Asserts(rbac.ResourceUsageEvent, policy.ActionUpdate)
56825686
}))
56835687

5684-
s.Run("UpdateUsageEventsPostPublish", s.Subtest(func(db database.Store, check *expects) {
5685-
check.Args(database.UpdateUsageEventsPostPublishParams{
5686-
Now: dbtime.Now(),
5688+
s.Run("UpdateUsageEventsPostPublish", s.Mocked(func(db *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
5689+
now := dbtime.Now()
5690+
params := database.UpdateUsageEventsPostPublishParams{
5691+
Now: now,
56875692
IDs: []string{"1", "2"},
56885693
FailureMessages: []string{"error", "error"},
56895694
SetPublishedAts: []bool{false, false},
5690-
}).Asserts(rbac.ResourceSystem, policy.ActionUpdate)
5695+
}
5696+
db.EXPECT().UpdateUsageEventsPostPublish(gomock.Any(), params).Return(nil)
5697+
check.Args(params).Asserts(rbac.ResourceUsageEvent, policy.ActionUpdate)
56915698
}))
56925699
}

‎coderd/database/dump.sql

Lines changed: 6 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000358_create_usage_events_table.down.sql

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎coderd/database/migrations/000359_create_usage_events_table.down.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DROP TABLE usage_events;

‎coderd/database/migrations/000358_create_usage_events_table.up.sql renamed to ‎coderd/database/migrations/000359_create_usage_events_table.up.sql

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1-
CREATE TYPE usage_event_type AS ENUM (
2-
'dc_managed_agents_v1'
3-
);
4-
5-
COMMENT ON TYPE usage_event_type IS 'The usage event type with version. "dc" means "discrete" (e.g. a single event, for counters), "hb" means "heartbeat" (e.g. a recurring event that contains a total count of usage generated from the database, for gauges).';
6-
71
CREATE TABLE usage_events (
82
id TEXT PRIMARY KEY,
9-
event_type usage_event_type NOT NULL,
3+
-- We use a TEXT column with a CHECK constraint rather than an enum because of
4+
-- the limitations with adding new values to an enum and using them in the
5+
-- same transaction.
6+
event_type TEXT NOT NULL CONSTRAINT usage_event_type_check CHECK (event_type IN ('dc_managed_agents_v1')),
107
event_data JSONB NOT NULL,
118
created_at TIMESTAMP WITH TIME ZONE NOT NULL,
129
publish_started_at TIMESTAMP WITH TIME ZONE DEFAULT NULL,
@@ -16,11 +13,13 @@ CREATE TABLE usage_events (
1613

1714
COMMENT ON TABLE usage_events IS 'usage_events contains usage data that is collected from the product and potentially shipped to the usage collector service.';
1815
COMMENT ON COLUMN usage_events.id IS 'For "discrete" event types, this is a random UUID. For "heartbeat" event types, this is a combination of the event type and a truncated timestamp.';
16+
COMMENT ON COLUMN usage_events.event_type IS 'The usage event type with version. "dc" means "discrete" (e.g. a single event, for counters), "hb" means "heartbeat" (e.g. a recurring event that contains a total count of usage generated from the database, for gauges).';
1917
COMMENT ON COLUMN usage_events.event_data IS 'Event payload. Determined by the matching usage struct for this event type.';
2018
COMMENT ON COLUMN usage_events.publish_started_at IS 'Set to a timestamp while the event is being published by a Coder replica to the usage collector service. Used to avoid duplicate publishes by multiple replicas. Timestamps older than 1 hour are considered expired.';
2119
COMMENT ON COLUMN usage_events.published_at IS 'Set to a timestamp when the event is successfully (or permanently unsuccessfully) published to the usage collector service. If set, the event should never be attempted to be published again.';
2220
COMMENT ON COLUMN usage_events.failure_message IS 'Set to an error message when the event is temporarily or permanently unsuccessfully published to the usage collector service.';
2321

24-
CREATE INDEX idx_usage_events_created_at ON usage_events (created_at);
25-
CREATE INDEX idx_usage_events_publish_started_at ON usage_events (publish_started_at);
26-
CREATE INDEX idx_usage_events_published_at ON usage_events (published_at);
22+
-- Create an index with all three fields used by the
23+
-- SelectUsageEventsForPublishing query.
24+
CREATE INDEX idx_usage_events_select_for_publishing
25+
ON usage_events (published_at, publish_started_at, created_at);

‎coderd/database/migrations/testdata/fixtures/000353_create_usage_events_table.up.sql renamed to ‎coderd/database/migrations/testdata/fixtures/000359_create_usage_events_table.up.sql

File renamed without changes.

‎coderd/database/modelmethods.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"encoding/hex"
55
"sort"
66
"strconv"
7-
"strings"
87
"time"
98

109
"github.com/google/uuid"
@@ -637,11 +636,3 @@ func (m WorkspaceAgentVolumeResourceMonitor) Debounce(
637636
func (s UserSecret) RBACObject() rbac.Object {
638637
return rbac.ResourceUserSecret.WithID(s.ID).WithOwner(s.UserID.String())
639638
}
640-
641-
func (e UsageEventType) IsDiscrete() bool {
642-
return e.Valid() && strings.HasPrefix(string(e), "dc_")
643-
}
644-
645-
func (e UsageEventType) IsHeartbeat() bool {
646-
return e.Valid() && strings.HasPrefix(string(e), "hb_")
647-
}

0 commit comments

Comments
 (0)