Skip to content

chore: add usage tracking package #19095

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Jul 30, 2025

Not used in coderd yet, see stack.

Adds two new packages:

  • coderd/usage: provides an interface for the "Collector" as well as a stub implementation for AGPL
  • enterprise/coderd/usage: provides an interface for the "Publisher" as well as a Tallyman implementation

Relates to coder/internal#814

Copy link
Member Author

deansheather commented Jul 30, 2025

@deansheather deansheather marked this pull request as ready for review July 30, 2025 06:05
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I have some non-blocking comments below but might need to take another pass.

const (
CoderLicenseJWTHeader = "Coder-License-JWT"

tallymanURL = "https://tallyman-ingress.coder.com"
Copy link
Member

Choose a reason for hiding this comment

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

I can see an argument for making this a var so it's configurable at build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we do need that we can add it very easily in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Obligatory reminder to check migration number before merging!

@github-actions github-actions bot added the stale This issue is like stale bread. label Aug 7, 2025
@github-actions github-actions bot closed this Aug 10, 2025
@deansheather deansheather reopened this Aug 12, 2025
@deansheather deansheather removed the stale This issue is like stale bread. label Aug 12, 2025
@deansheather deansheather force-pushed the 07-30-chore_add_usage_tracking_package branch from e376311 to 82ea8e1 Compare August 15, 2025 09:28
@deansheather deansheather requested a review from Emyrk as a code owner August 15, 2025 09:28
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I don't have any further blocking comments and the dbauthz stuff looks OK to me. I'd like a +1 from Spike before merging though. You probably also need to run make gen again.

Comment on lines +52 to +55
jsonData, err := json.Marshal(event.Fields())
if err != nil {
return xerrors.Errorf("marshal event as JSON: %w", err)
}
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth adding a test case for an un-marshalable event

Comment on lines +631 to +636
func jsoninate(t *testing.T, v any) string {
t.Helper()
buf, err := json.Marshal(v)
require.NoError(t, err)
return string(buf)
}
Copy link
Member

Choose a reason for hiding this comment

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

suggestion, non-blocking: could be useful as testutil.MustJSON

Copy link
Contributor

@spikecurtis spikecurtis left a comment

Choose a reason for hiding this comment

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

LGTM modulo gen failures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants