Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: wire up usage tracking for managed agents#19096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

deansheather
Copy link
Member

@deansheatherdeansheather commentedJul 30, 2025•
edited
Loading

Wires up the usage collector and publisher to coderd.

Relates tocoder/internal#814

@deansheatherGraphite App
Copy link
MemberAuthor

deansheather commentedJul 30, 2025•
edited
Loading

@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch 3 times, most recently from6d0a33f to89b1757CompareJuly 30, 2025 07:08
@github-actionsgithub-actionsbot added the staleThis issue is like stale bread. labelAug 7, 2025
@deansheatherdeansheather removed the staleThis issue is like stale bread. labelAug 12, 2025
@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch from89b1757 to397a84dCompareAugust 15, 2025 09:28
@deansheatherdeansheatherforce-pushed the07-30-chore_add_usage_tracking_package branch frome376311 to82ea8e1CompareAugust 15, 2025 09:28
@deansheatherdeansheather marked this pull request as ready for reviewAugust 15, 2025 09:29
sidebarAppID= uuid.NullUUID{}
}

ifhasAITask {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we only want to do this on START transitions?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

You're right 🤦‍♂️ Added a test

// Duplicate events are ignored by the query, so we don't need to check the
// error.
returntx.InsertUsageEvent(ctx, database.InsertUsageEventParams{
returntx.InsertUsageEvent(dbauthz.AsUsageTracker(ctx), database.InsertUsageEventParams{//nolint:gocritic // intentionally want to insert usage events
Copy link
Contributor

Choose a reason for hiding this comment

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

If we override the context this way it means that anything that can access the Inserter can create usage events, including totally unauthenticated API routes.

We need to let RBAC do its job and actually authorize the appropriaterbac.Subjects. Right now the only legit caller is provisionerdserver, which already has it's own subject in dbauthz.go L189.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Fixed and enabled dbauthz in provisionerdserver tests too to ensure it's tested.

@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch 3 times, most recently from40d1681 to7c36cc0CompareAugust 15, 2025 15:19
@deansheatherdeansheatherforce-pushed the07-30-chore_add_usage_tracking_package branch fromdca0890 to2529c80CompareAugust 15, 2025 15:19
@deansheatherdeansheather changed the base branch from07-30-chore_add_usage_tracking_package tographite-base/19096August 15, 2025 15:30
@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch from7c36cc0 toaf6661dCompareAugust 15, 2025 15:31
@graphite-appgraphite-appbot changed the base branch fromgraphite-base/19096 tomainAugust 15, 2025 15:31
@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch fromaf6661d to3aea6d1CompareAugust 15, 2025 15:31
@deansheatherdeansheather removed the request for review fromaslilacAugust 15, 2025 17:55
@deansheatherGraphite App
Copy link
MemberAuthor

I added dbauthz to the provisionerdserver tests to ensure the permissions were correct. To do so I had to refactor a bunch of the tests since they didn't have many required DB resources (jobs are RBAC checked by their linked workspace build or template version).

Also, I disabled thedbauthz.As*() ruleguard rules for_test.go files after consulting with Steven. I probably should've done this in another PR but it's only 100 lines of comment removals so I figured it wouldn't complicate the review that much. Sorry about that

Copy link
Member

@EmyrkEmyrk left a comment

Choose a reason for hiding this comment

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

LG, just mainly the comment on the dbauthz function for licenses.

What happens to these usage events if you run the enterprise coderd binary without a license? Or without the entitlement? Are all events still sent to Tallyman?

Comment on lines -40 to +42
m["f"].Text.Matches("^As"),
m["f"].Text.Matches("^As")&&
// Ignore test usages of dbauthz contexts.
!m.File().Name.Matches(`_test\.go$`),
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@deansheatherdeansheatherforce-pushed the07-30-chore_wire_up_usage_tracking_for_managed_agents branch from2d92d73 to29655e5CompareAugust 20, 2025 13:15
@deansheatherGraphite App
Copy link
MemberAuthor

deansheather commentedAug 20, 2025•
edited
Loading

Merge activity

  • Aug 20, 1:37 PM UTC: A user started a stack merge that includes this pull request viaGraphite.
  • Aug 20, 1:38 PM UTC:@deansheather merged this pull request withGraphite.

@deansheatherdeansheather merged commit6eb02d1 intomainAug 20, 2025
27 checks passed
@deansheatherdeansheather deleted the 07-30-chore_wire_up_usage_tracking_for_managed_agents branchAugust 20, 2025 13:38
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@EmyrkEmyrkEmyrk approved these changes

@spikecurtisspikecurtisAwaiting requested review from spikecurtisspikecurtis is a code owner

@ethanndicksonethanndicksonAwaiting requested review from ethanndickson

Assignees

@deansheatherdeansheather

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@deansheather@spikecurtis@Emyrk
[8]ページ先頭
©2009-2025 Movatter.jp