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

Commit92d22e2

Browse files
authored
chore: track usage of organizations in telemetry (#16323)
Addressescoder/internal#317.## ChangesRequirements are quoted below:> how many orgs does deployment haveAdds the Organization entity to telemetry.> ensuring resources are associated with orgsAll resources that reference an org already report the org id totelemetry. Adds a test to check that.> whether org sync is configuredAdds the `IDPOrgSync` boolean field to the Deployment entity.## Implementation of the org sync checkWhile there's an `OrganizationSyncEnabled` method on the IDPSyncinterface, I decided not to use it directly and implemented acounterpart just for telemetry purposes. It's a compromise I'm not happyabout, but I found that it's a simpler approach than the alternative.There are multiple reasons:1. The telemetry package cannot statically access the IDPSync interfacedue to a circular import.2. We can't dynamically pass a reference to the`OrganizationSyncEnabled` function at the time of instantiating thetelemetry object, because our server initialization logic depends on thetelemetry object being created before the IDPSync object.3. If we circumvent that problem by passing the reference as aninitially empty pointer, initializing telemetry, then IDPSync, thenupdating the pointer to point to `OrganizationSyncEnabled`, we have torefactor the initialization logic of the telemetry object itself toavoid a race condition where the first telemetry report is performedwithout a valid reference.I actually implemented that approach in#16307, but realized I'm unable tofully test it. It changed the initialization order in the servercommand, and I wanted to test our CLI with Org Sync configured with apremium license. As far as I'm aware, we don't have the tooling to dothat. I couldn't figure out a way to start the CLI with a mock license,and I didn't want to go down further into the refactoring rabbit hole.So I decided that reimplementing the org sync checking logic is simpler.
1 parentb77b543 commit92d22e2

File tree

4 files changed

+152
-6
lines changed

4 files changed

+152
-6
lines changed

‎coderd/idpsync/organization.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func (s AGPLIDPSync) UpdateOrganizationSettings(ctx context.Context, db database
4545
}
4646

4747
func (sAGPLIDPSync)OrganizationSyncSettings(ctx context.Context,db database.Store) (*OrganizationSyncSettings,error) {
48+
// If this logic is ever updated, make sure to update the corresponding
49+
// checkIDPOrgSync in coderd/telemetry/telemetry.go.
4850
rlv:=s.Manager.Resolver(db)
4951
orgSettings,err:=s.SyncSettings.Organization.Resolve(ctx,rlv)
5052
iferr!=nil {

‎coderd/telemetry/telemetry.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"crypto/sha256"
7+
"database/sql"
78
"encoding/json"
89
"errors"
910
"fmt"
@@ -244,6 +245,11 @@ func (r *remoteReporter) deployment() error {
244245
returnxerrors.Errorf("install source must be <=64 chars: %s",installSource)
245246
}
246247

248+
idpOrgSync,err:=checkIDPOrgSync(r.ctx,r.options.Database,r.options.DeploymentConfig)
249+
iferr!=nil {
250+
r.options.Logger.Debug(r.ctx,"check IDP org sync",slog.Error(err))
251+
}
252+
247253
data,err:=json.Marshal(&Deployment{
248254
ID:r.options.DeploymentID,
249255
Architecture:sysInfo.Architecture,
@@ -263,6 +269,7 @@ func (r *remoteReporter) deployment() error {
263269
MachineID:sysInfo.UniqueID,
264270
StartedAt:r.startedAt,
265271
ShutdownAt:r.shutdownAt,
272+
IDPOrgSync:&idpOrgSync,
266273
})
267274
iferr!=nil {
268275
returnxerrors.Errorf("marshal deployment: %w",err)
@@ -284,6 +291,45 @@ func (r *remoteReporter) deployment() error {
284291
returnnil
285292
}
286293

294+
// idpOrgSyncConfig is a subset of
295+
// https://github.com/coder/coder/blob/5c6578d84e2940b9cfd04798c45e7c8042c3fe0e/coderd/idpsync/organization.go#L148
296+
typeidpOrgSyncConfigstruct {
297+
Fieldstring`json:"field"`
298+
}
299+
300+
// checkIDPOrgSync inspects the server flags and the runtime config. It's based on
301+
// the OrganizationSyncEnabled function from enterprise/coderd/enidpsync/organizations.go.
302+
// It has one distinct difference: it doesn't check if the license entitles to the
303+
// feature, it only checks if the feature is configured.
304+
//
305+
// The above function is not used because it's very hard to make it available in
306+
// the telemetry package due to coder/coder package structure and initialization
307+
// order of the coder server.
308+
//
309+
// We don't check license entitlements because it's also hard to do from the
310+
// telemetry package, and the config check should be sufficient for telemetry purposes.
311+
//
312+
// While this approach duplicates code, it's simpler than the alternative.
313+
//
314+
// See https://github.com/coder/coder/pull/16323 for more details.
315+
funccheckIDPOrgSync(ctx context.Context,db database.Store,values*codersdk.DeploymentValues) (bool,error) {
316+
// key based on https://github.com/coder/coder/blob/5c6578d84e2940b9cfd04798c45e7c8042c3fe0e/coderd/idpsync/idpsync.go#L168
317+
syncConfigRaw,err:=db.GetRuntimeConfig(ctx,"organization-sync-settings")
318+
iferr!=nil {
319+
iferrors.Is(err,sql.ErrNoRows) {
320+
// If the runtime config is not set, we check if the deployment config
321+
// has the organization field set.
322+
returnvalues!=nil&&values.OIDC.OrganizationField!="",nil
323+
}
324+
returnfalse,xerrors.Errorf("get runtime config: %w",err)
325+
}
326+
syncConfig:=idpOrgSyncConfig{}
327+
iferr:=json.Unmarshal([]byte(syncConfigRaw),&syncConfig);err!=nil {
328+
returnfalse,xerrors.Errorf("unmarshal runtime config: %w",err)
329+
}
330+
returnsyncConfig.Field!="",nil
331+
}
332+
287333
// createSnapshot collects a full snapshot from the database.
288334
func (r*remoteReporter)createSnapshot() (*Snapshot,error) {
289335
var (
@@ -518,6 +564,21 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) {
518564
}
519565
returnnil
520566
})
567+
eg.Go(func()error {
568+
// Warning: When an organization is deleted, it's completely removed from
569+
// the database. It will no longer be reported, and there will be no other
570+
// indicator that it was deleted. This requires special handling when
571+
// interpreting the telemetry data later.
572+
orgs,err:=r.options.Database.GetOrganizations(r.ctx, database.GetOrganizationsParams{})
573+
iferr!=nil {
574+
returnxerrors.Errorf("get organizations: %w",err)
575+
}
576+
snapshot.Organizations=make([]Organization,0,len(orgs))
577+
for_,org:=rangeorgs {
578+
snapshot.Organizations=append(snapshot.Organizations,ConvertOrganization(org))
579+
}
580+
returnnil
581+
})
521582

522583
err:=eg.Wait()
523584
iferr!=nil {
@@ -916,6 +977,14 @@ func ConvertExternalProvisioner(id uuid.UUID, tags map[string]string, provisione
916977
}
917978
}
918979

980+
funcConvertOrganization(org database.Organization)Organization {
981+
returnOrganization{
982+
ID:org.ID,
983+
CreatedAt:org.CreatedAt,
984+
IsDefault:org.IsDefault,
985+
}
986+
}
987+
919988
// Snapshot represents a point-in-time anonymized database dump.
920989
// Data is aggregated by latest on the server-side, so partial data
921990
// can be sent without issue.
@@ -942,6 +1011,7 @@ type Snapshot struct {
9421011
WorkspaceModules []WorkspaceModule`json:"workspace_modules"`
9431012
Workspaces []Workspace`json:"workspaces"`
9441013
NetworkEvents []NetworkEvent`json:"network_events"`
1014+
Organizations []Organization`json:"organizations"`
9451015
}
9461016

9471017
// Deployment contains information about the host running Coder.
@@ -964,6 +1034,9 @@ type Deployment struct {
9641034
MachineIDstring`json:"machine_id"`
9651035
StartedAt time.Time`json:"started_at"`
9661036
ShutdownAt*time.Time`json:"shutdown_at"`
1037+
// While IDPOrgSync will always be set, it's nullable to make
1038+
// the struct backwards compatible with older coder versions.
1039+
IDPOrgSync*bool`json:"idp_org_sync"`
9671040
}
9681041

9691042
typeAPIKeystruct {
@@ -1457,6 +1530,12 @@ func NetworkEventFromProto(proto *tailnetproto.TelemetryEvent) (NetworkEvent, er
14571530
},nil
14581531
}
14591532

1533+
typeOrganizationstruct {
1534+
ID uuid.UUID`json:"id"`
1535+
IsDefaultbool`json:"is_default"`
1536+
CreatedAt time.Time`json:"created_at"`
1537+
}
1538+
14601539
typenoopReporterstruct{}
14611540

14621541
func (*noopReporter)Report(_*Snapshot) {}

‎coderd/telemetry/telemetry_test.go

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import (
2222
"github.com/coder/coder/v2/coderd/database/dbmem"
2323
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2424
"github.com/coder/coder/v2/coderd/database/dbtime"
25+
"github.com/coder/coder/v2/coderd/idpsync"
26+
"github.com/coder/coder/v2/coderd/runtimeconfig"
2527
"github.com/coder/coder/v2/coderd/telemetry"
28+
"github.com/coder/coder/v2/codersdk"
2629
"github.com/coder/coder/v2/testutil"
2730
)
2831

@@ -40,22 +43,33 @@ func TestTelemetry(t *testing.T) {
4043
db:=dbmem.New()
4144

4245
ctx:=testutil.Context(t,testutil.WaitMedium)
46+
47+
org,err:=db.GetDefaultOrganization(ctx)
48+
require.NoError(t,err)
49+
4350
_,_=dbgen.APIKey(t,db, database.APIKey{})
4451
_=dbgen.ProvisionerJob(t,db,nil, database.ProvisionerJob{
45-
Provisioner:database.ProvisionerTypeTerraform,
46-
StorageMethod:database.ProvisionerStorageMethodFile,
47-
Type:database.ProvisionerJobTypeTemplateVersionDryRun,
52+
Provisioner:database.ProvisionerTypeTerraform,
53+
StorageMethod:database.ProvisionerStorageMethodFile,
54+
Type:database.ProvisionerJobTypeTemplateVersionDryRun,
55+
OrganizationID:org.ID,
4856
})
4957
_=dbgen.Template(t,db, database.Template{
50-
Provisioner:database.ProvisionerTypeTerraform,
58+
Provisioner:database.ProvisionerTypeTerraform,
59+
OrganizationID:org.ID,
5160
})
5261
sourceExampleID:=uuid.NewString()
5362
_=dbgen.TemplateVersion(t,db, database.TemplateVersion{
5463
SourceExampleID: sql.NullString{String:sourceExampleID,Valid:true},
64+
OrganizationID:org.ID,
65+
})
66+
_=dbgen.TemplateVersion(t,db, database.TemplateVersion{
67+
OrganizationID:org.ID,
5568
})
56-
_=dbgen.TemplateVersion(t,db, database.TemplateVersion{})
5769
user:=dbgen.User(t,db, database.User{})
58-
_=dbgen.Workspace(t,db, database.WorkspaceTable{})
70+
_=dbgen.Workspace(t,db, database.WorkspaceTable{
71+
OrganizationID:org.ID,
72+
})
5973
_=dbgen.WorkspaceApp(t,db, database.WorkspaceApp{
6074
SharingLevel:database.AppSharingLevelOwner,
6175
Health:database.WorkspaceAppHealthDisabled,
@@ -112,6 +126,7 @@ func TestTelemetry(t *testing.T) {
112126
require.Len(t,snapshot.WorkspaceAgentStats,1)
113127
require.Len(t,snapshot.WorkspaceProxies,1)
114128
require.Len(t,snapshot.WorkspaceModules,1)
129+
require.Len(t,snapshot.Organizations,1)
115130

116131
wsa:=snapshot.WorkspaceAgents[0]
117132
require.Len(t,wsa.Subsystems,2)
@@ -128,6 +143,19 @@ func TestTelemetry(t *testing.T) {
128143
})
129144
require.Equal(t,tvs[0].SourceExampleID,&sourceExampleID)
130145
require.Nil(t,tvs[1].SourceExampleID)
146+
147+
for_,entity:=rangesnapshot.Workspaces {
148+
require.Equal(t,entity.OrganizationID,org.ID)
149+
}
150+
for_,entity:=rangesnapshot.ProvisionerJobs {
151+
require.Equal(t,entity.OrganizationID,org.ID)
152+
}
153+
for_,entity:=rangesnapshot.TemplateVersions {
154+
require.Equal(t,entity.OrganizationID,org.ID)
155+
}
156+
for_,entity:=rangesnapshot.Templates {
157+
require.Equal(t,entity.OrganizationID,org.ID)
158+
}
131159
})
132160
t.Run("HashedEmail",func(t*testing.T) {
133161
t.Parallel()
@@ -243,6 +271,41 @@ func TestTelemetry(t *testing.T) {
243271
require.Equal(t,c.want,telemetry.GetModuleSourceType(c.source))
244272
}
245273
})
274+
t.Run("IDPOrgSync",func(t*testing.T) {
275+
t.Parallel()
276+
ctx:=testutil.Context(t,testutil.WaitMedium)
277+
db,_:=dbtestutil.NewDB(t)
278+
279+
// 1. No org sync settings
280+
deployment,_:=collectSnapshot(t,db,nil)
281+
require.False(t,*deployment.IDPOrgSync)
282+
283+
// 2. Org sync settings set in server flags
284+
deployment,_=collectSnapshot(t,db,func(opts telemetry.Options) telemetry.Options {
285+
opts.DeploymentConfig=&codersdk.DeploymentValues{
286+
OIDC: codersdk.OIDCConfig{
287+
OrganizationField:"organizations",
288+
},
289+
}
290+
returnopts
291+
})
292+
require.True(t,*deployment.IDPOrgSync)
293+
294+
// 3. Org sync settings set in runtime config
295+
org,err:=db.GetDefaultOrganization(ctx)
296+
require.NoError(t,err)
297+
sync:=idpsync.NewAGPLSync(testutil.Logger(t),runtimeconfig.NewManager(), idpsync.DeploymentSyncSettings{})
298+
err=sync.UpdateOrganizationSettings(ctx,db, idpsync.OrganizationSyncSettings{
299+
Field:"organizations",
300+
Mapping:map[string][]uuid.UUID{
301+
"first": {org.ID},
302+
},
303+
AssignDefault:true,
304+
})
305+
require.NoError(t,err)
306+
deployment,_=collectSnapshot(t,db,nil)
307+
require.True(t,*deployment.IDPOrgSync)
308+
})
246309
}
247310

248311
// nolint:paralleltest

‎enterprise/coderd/enidpsync/organizations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ func (e EnterpriseIDPSync) OrganizationSyncEnabled(ctx context.Context, db datab
1919
returnfalse
2020
}
2121

22+
// If this logic is ever updated, make sure to update the corresponding
23+
// checkIDPOrgSync in coderd/telemetry/telemetry.go.
2224
settings,err:=e.OrganizationSyncSettings(ctx,db)
2325
iferr==nil&&settings.Field!="" {
2426
returntrue

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp