From 24e4ff388fd182124b63f25edf1cb68c04ed2287 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 13 Oct 2023 15:05:21 +0200 Subject: [PATCH 01/19] WIP --- cli/server.go | 13 ++++ .../insights/metricscollector.go | 72 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 coderd/prometheusmetrics/insights/metricscollector.go diff --git a/cli/server.go b/cli/server.go index 9f33ced438f84..d43a2345b12ca 100644 --- a/cli/server.go +++ b/cli/server.go @@ -78,6 +78,7 @@ import ( "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/oauthpki" "github.com/coder/coder/v2/coderd/prometheusmetrics" + "github.com/coder/coder/v2/coderd/prometheusmetrics/insights" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/tracing" @@ -198,6 +199,18 @@ func enablePrometheus( } afterCtx(ctx, closeWorkspacesFunc) + insightsMetricsCollector := insights.NewMetricsCollector(options.Database, 0) + err = options.PrometheusRegistry.Register(insightsMetricsCollector) + if err != nil { + return nil, xerrors.Errorf("unable to register insights metrics collector: %w", err) + } + + closeInsightsMetricsCollector, err := insightsMetricsCollector.Run(ctx) + if err != nil { + return nil, xerrors.Errorf("unable to run insights metrics collector: %w", err) + } + afterCtx(ctx, closeInsightsMetricsCollector) + if vals.Prometheus.CollectAgentStats { closeAgentStatsFunc, err := prometheusmetrics.AgentStats(ctx, logger, options.PrometheusRegistry, options.Database, time.Now(), 0) if err != nil { diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go new file mode 100644 index 0000000000000..39687d95b14d9 --- /dev/null +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -0,0 +1,72 @@ +package insights + +import ( + "context" + "time" + + "github.com/prometheus/client_golang/prometheus" + + "github.com/coder/coder/v2/coderd/database" +) + +var ( + activeUsersDesc = prometheus.NewDesc("coderd_insights_active_users", "The number of active users of the template.", []string{"template_name"}, nil) + applicationsUsageSecondsDesc = prometheus.NewDesc("coderd_insights_applications_usage_seconds", "The application usage per template.", []string{"application_name", "template_name"}, nil) + parametersDesc = prometheus.NewDesc("coderd_insights_parameters", "The parameter usage per template.", []string{"template_name", "name", "value"}, nil) +) + +type MetricsCollector struct { + database database.Store + duration time.Duration +} + +var _ prometheus.Collector = new(MetricsCollector) + +func NewMetricsCollector(db database.Store, duration time.Duration) *MetricsCollector { + if duration == 0 { + duration = 5 * time.Minute + } + + return &MetricsCollector{ + database: db, + duration: duration, + } +} + +func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { + ctx, closeFunc := context.WithCancel(ctx) + done := make(chan struct{}) + + // Use time.Nanosecond to force an initial tick. It will be reset to the + // correct duration after executing once. + ticker := time.NewTicker(time.Nanosecond) + doTick := func() { + defer ticker.Reset(mc.duration) + } + + go func() { + defer close(done) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + doTick() + } + } + }() + return func() { + closeFunc() + <-done + }, nil +} + +func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) { + descCh <- activeUsersDesc + descCh <- applicationsUsageSecondsDesc + descCh <- parametersDesc +} + +func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { +} From a6c9c1a3bdad14a6dbf2b161c3c4a4cab9bda293 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 13 Oct 2023 15:36:51 +0200 Subject: [PATCH 02/19] Use db --- cli/server.go | 5 +++- .../insights/metricscollector.go | 23 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/cli/server.go b/cli/server.go index d43a2345b12ca..2a33b5ae2e90d 100644 --- a/cli/server.go +++ b/cli/server.go @@ -199,7 +199,10 @@ func enablePrometheus( } afterCtx(ctx, closeWorkspacesFunc) - insightsMetricsCollector := insights.NewMetricsCollector(options.Database, 0) + insightsMetricsCollector, err := insights.NewMetricsCollector(options.Database, options.Logger, 0) + if err != nil { + return nil, xerrors.Errorf("unable to initialize insights metrics collector: %w", err) + } err = options.PrometheusRegistry.Register(insightsMetricsCollector) if err != nil { return nil, xerrors.Errorf("unable to register insights metrics collector: %w", err) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 39687d95b14d9..04fc49ef5d122 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -4,7 +4,9 @@ import ( "context" "time" + "cdr.dev/slog" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" ) @@ -17,20 +19,25 @@ var ( type MetricsCollector struct { database database.Store + logger slog.Logger duration time.Duration } var _ prometheus.Collector = new(MetricsCollector) -func NewMetricsCollector(db database.Store, duration time.Duration) *MetricsCollector { +func NewMetricsCollector(db database.Store, logger slog.Logger, duration time.Duration) (*MetricsCollector, error) { if duration == 0 { duration = 5 * time.Minute } + if duration < 5*time.Minute { + return nil, xerrors.Errorf("refresh interval must be at least 5 mins") + } return &MetricsCollector{ database: db, + logger: logger.Named("insights_metrics_collector"), duration: duration, - } + }, nil } func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { @@ -42,6 +49,18 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { ticker := time.NewTicker(time.Nanosecond) doTick := func() { defer ticker.Reset(mc.duration) + + now := time.Now() + + parameterRows, err := mc.database.GetTemplateInsights(ctx, database.GetTemplateInsightsParams{ + StartTime: now.Add(-mc.duration), + EndTime: now, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch template insights from database: %w", err) + return + } + } go func() { From 86f45a064028a6f797dc94c5a7c23c20804fe05b Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 13 Oct 2023 16:54:07 +0200 Subject: [PATCH 03/19] debug --- coderd/prometheusmetrics/insights/metricscollector.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 04fc49ef5d122..bbbe61f1b4dd0 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -4,10 +4,11 @@ import ( "context" "time" - "cdr.dev/slog" "github.com/prometheus/client_golang/prometheus" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" ) @@ -52,15 +53,18 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { now := time.Now() + // TODO collect iteration time + parameterRows, err := mc.database.GetTemplateInsights(ctx, database.GetTemplateInsightsParams{ StartTime: now.Add(-mc.duration), EndTime: now, }) if err != nil { - mc.logger.Error(ctx, "unable to fetch template insights from database: %w", err) + mc.logger.Error(ctx, "unable to fetch template insights from database", slog.Error(err)) return } + mc.logger.Info(ctx, "debug", slog.F("parameter_rows", parameterRows)) } go func() { From a3c29a7471e77ceb2501608cb01dd908ce05f6cc Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 17 Oct 2023 13:48:21 +0200 Subject: [PATCH 04/19] DB calls --- .../insights/metricscollector.go | 56 +++++++++++++++++-- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index bbbe61f1b4dd0..c99a4c944efae 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -5,6 +5,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/sync/errgroup" "golang.org/x/xerrors" "cdr.dev/slog" @@ -52,19 +53,61 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { defer ticker.Reset(mc.duration) now := time.Now() + startTime := now.Add(-mc.duration) + endTime := now // TODO collect iteration time - parameterRows, err := mc.database.GetTemplateInsights(ctx, database.GetTemplateInsightsParams{ - StartTime: now.Add(-mc.duration), - EndTime: now, + var templateInsights database.GetTemplateInsightsRow + var appInsights []database.GetTemplateAppInsightsRow + var parameterInsights []database.GetTemplateParameterInsightsRow + + // Phase I: Fetch insights from database + eg, egCtx := errgroup.WithContext(ctx) + eg.SetLimit(3) + + eg.Go(func() error { + var err error + templateInsights, err = mc.database.GetTemplateInsights(egCtx, database.GetTemplateInsightsParams{ + StartTime: startTime, + EndTime: endTime, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch template insights from database", slog.Error(err)) + } + return err + }) + eg.Go(func() error { + var err error + appInsights, err = mc.database.GetTemplateAppInsights(ctx, database.GetTemplateAppInsightsParams{ + StartTime: startTime, + EndTime: endTime, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch app insights from database", slog.Error(err)) + } + return err + }) + eg.Go(func() error { + var err error + parameterInsights, err = mc.database.GetTemplateParameterInsights(ctx, database.GetTemplateParameterInsightsParams{ + StartTime: startTime, + EndTime: endTime, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch parameter insights from database", slog.Error(err)) + } + return err }) + + err := eg.Wait() if err != nil { - mc.logger.Error(ctx, "unable to fetch template insights from database", slog.Error(err)) return } - mc.logger.Info(ctx, "debug", slog.F("parameter_rows", parameterRows)) + // Phase 2: Collect resource IDs (templates, applications, parameters), and fetch relevant details + + // TODO } go func() { @@ -92,4 +135,7 @@ func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) { } func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { + // Phase 3: Collect metrics + + // TODO } From 24ae33227e513e858ee7cedf26fad9525bc9cd57 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 17 Oct 2023 16:09:34 +0200 Subject: [PATCH 05/19] WIP --- .../insights/metricscollector.go | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index c99a4c944efae..4ff8735990c81 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -58,7 +59,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // TODO collect iteration time - var templateInsights database.GetTemplateInsightsRow + var userActivity []database.GetUserActivityInsightsRow var appInsights []database.GetTemplateAppInsightsRow var parameterInsights []database.GetTemplateParameterInsightsRow @@ -68,7 +69,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { eg.Go(func() error { var err error - templateInsights, err = mc.database.GetTemplateInsights(egCtx, database.GetTemplateInsightsParams{ + userActivity, err = mc.database.GetUserActivityInsights(egCtx, database.GetUserActivityInsightsParams{ StartTime: startTime, EndTime: endTime, }) @@ -105,9 +106,17 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { return } - // Phase 2: Collect resource IDs (templates, applications, parameters), and fetch relevant details + // Phase 2: Collect template IDs, and fetch relevant details + templateIDs := uniqueTemplateIDs(userActivity, appInsights, parameterInsights) + templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ + IDs: templateIDs, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch template details from database", slog.Error(err)) + return + } - // TODO + templateNames := onlyTemplateNames(templates) } go func() { @@ -139,3 +148,41 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { // TODO } + +// Helper functions below. + +func uniqueTemplateIDs(userActivity []database.GetUserActivityInsightsRow, appInsights []database.GetTemplateAppInsightsRow, parameterInsights []database.GetTemplateParameterInsightsRow) []uuid.UUID { + tids := map[uuid.UUID]bool{} + for _, t := range userActivity { + for _, tid := range t.TemplateIDs { + tids[tid] = true + } + } + + for _, a := range appInsights { + for _, tid := range a.TemplateIDs { + tids[tid] = true + } + } + + for _, p := range parameterInsights { + for _, tid := range p.TemplateIDs { + tids[tid] = true + } + } + + uniqueUUIDs := make([]uuid.UUID, len(tids)) + var i int + for t := range tids { + uniqueUUIDs[i] = t + } + return uniqueUUIDs +} + +func onlyTemplateNames(templates []database.Template) map[uuid.UUID]string { + m := map[uuid.UUID]string{} + for _, t := range templates { + m[t.ID] = t.Name + } + return m +} From 0cb1206760ef2732d371fb23eaa492c57a504345 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 11:18:22 +0200 Subject: [PATCH 06/19] WIP dbfake --- coderd/database/dbauthz/dbauthz.go | 4 + coderd/database/dbfake/dbfake.go | 99 +++++++++++++++++++ coderd/database/dbmetrics/dbmetrics.go | 7 ++ coderd/database/dbmock/dbmock.go | 15 +++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 73 ++++++++++++++ coderd/database/queries/insights.sql | 28 ++++++ .../insights/metricscollector.go | 3 +- 8 files changed, 229 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 038f4e0c92807..9eaacab556ab9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1324,6 +1324,10 @@ func (q *querier) GetTemplateInsightsByInterval(ctx context.Context, arg databas return q.db.GetTemplateInsightsByInterval(ctx, arg) } +func (q *querier) GetTemplateInsightsByTemplate(ctx context.Context, arg database.GetTemplateInsightsByTemplateParams) ([]database.GetTemplateInsightsByTemplateRow, error) { + panic("not implemented") +} + func (q *querier) GetTemplateParameterInsights(ctx context.Context, arg database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { for _, templateID := range arg.TemplateIDs { template, err := q.db.GetTemplateByID(ctx, templateID) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index bffd855da6b6b..9a135789f66c3 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -2500,6 +2500,10 @@ func (q *FakeQuerier) GetTemplateInsights(_ context.Context, arg database.GetTem templateIDSet := make(map[uuid.UUID]struct{}) appUsageIntervalsByUser := make(map[uuid.UUID]map[time.Time]*database.GetTemplateInsightsRow) + + q.mutex.RLock() + defer q.mutex.RUnlock() + for _, s := range q.workspaceAgentStats { if s.CreatedAt.Before(arg.StartTime) || s.CreatedAt.Equal(arg.EndTime) || s.CreatedAt.After(arg.EndTime) { continue @@ -2648,6 +2652,101 @@ func (q *FakeQuerier) GetTemplateInsightsByInterval(ctx context.Context, arg dat return result, nil } +func (q *FakeQuerier) GetTemplateInsightsByTemplate(_ context.Context, arg database.GetTemplateInsightsByTemplateParams) ([]database.GetTemplateInsightsByTemplateRow, error) { + err := validateDatabaseType(arg) + if err != nil { + return nil, err + } + + q.mutex.RLock() + defer q.mutex.RUnlock() + + // map time.Time x TemplateID x UserID x + appUsageByTemplateAndUser := map[time.Time]map[uuid.UUID]map[uuid.UUID]database.GetTemplateInsightsByTemplateRow{} + + // Review agent stats in terms of usage + templateIDSet := make(map[uuid.UUID]struct{}) + + for _, s := range q.workspaceAgentStats { + if s.CreatedAt.Before(arg.StartTime) || s.CreatedAt.Equal(arg.EndTime) || s.CreatedAt.After(arg.EndTime) { + continue + } + if s.ConnectionCount == 0 { + continue + } + + t := s.CreatedAt.Truncate(time.Minute) + templateIDSet[s.TemplateID] = struct{}{} + + if _, ok := appUsageByTemplateAndUser[t]; !ok { + appUsageByTemplateAndUser[t] = make(map[uuid.UUID]map[uuid.UUID]database.GetTemplateInsightsByTemplateRow) + } + + if _, ok := appUsageByTemplateAndUser[t][s.TemplateID]; !ok { + appUsageByTemplateAndUser[t][s.TemplateID] = make(map[uuid.UUID]database.GetTemplateInsightsByTemplateRow) + } + + if _, ok := appUsageByTemplateAndUser[t][s.TemplateID][s.UserID]; !ok { + appUsageByTemplateAndUser[t][s.TemplateID][s.UserID] = database.GetTemplateInsightsByTemplateRow{} + } + + u := appUsageByTemplateAndUser[t][s.TemplateID][s.UserID] + if s.SessionCountJetBrains > 0 { + u.UsageJetbrainsSeconds = 60 + } + if s.SessionCountVSCode > 0 { + u.UsageVscodeSeconds = 60 + } + if s.SessionCountReconnectingPTY > 0 { + u.UsageReconnectingPtySeconds = 60 + } + if s.SessionCountSSH > 0 { + u.UsageSshSeconds = 60 + } + appUsageByTemplateAndUser[t][s.TemplateID][s.UserID] = u + } + + // Sort used templates + templateIDs := make([]uuid.UUID, 0, len(templateIDSet)) + for templateID := range templateIDSet { + templateIDs = append(templateIDs, templateID) + } + slices.SortFunc(templateIDs, func(a, b uuid.UUID) int { + return slice.Ascending(a.String(), b.String()) + }) + + // Build result + var result []database.GetTemplateInsightsByTemplateRow + for _, templateID := range templateIDs { + r := database.GetTemplateInsightsByTemplateRow{ + TemplateID: templateID, + } + + uniqueUsers := map[uuid.UUID]struct{}{} + + for _, mTemplateUserUsage := range appUsageByTemplateAndUser { + mUserUsage, ok := mTemplateUserUsage[templateID] + if !ok { + continue // template was not used in this time window + } + + for userID, usage := range mUserUsage { + uniqueUsers[userID] = struct{}{} + + r.UsageJetbrainsSeconds += usage.UsageJetbrainsSeconds + r.UsageVscodeSeconds += usage.UsageVscodeSeconds + r.UsageReconnectingPtySeconds += usage.UsageReconnectingPtySeconds + r.UsageSshSeconds += usage.UsageSshSeconds + } + } + + r.ActiveUsers = int64(len(uniqueUsers)) + + result = append(result, r) + } + return result, nil +} + func (q *FakeQuerier) GetTemplateParameterInsights(ctx context.Context, arg database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { err := validateDatabaseType(arg) if err != nil { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index ece7020139b0f..50df68878e892 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -704,6 +704,13 @@ func (m metricsStore) GetTemplateInsightsByInterval(ctx context.Context, arg dat return r0, r1 } +func (m metricsStore) GetTemplateInsightsByTemplate(ctx context.Context, arg database.GetTemplateInsightsByTemplateParams) ([]database.GetTemplateInsightsByTemplateRow, error) { + start := time.Now() + r0, r1 := m.s.GetTemplateInsightsByTemplate(ctx, arg) + m.queryLatencies.WithLabelValues("GetTemplateInsightsByTemplate").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetTemplateParameterInsights(ctx context.Context, arg database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { start := time.Now() r0, r1 := m.s.GetTemplateParameterInsights(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 31614be3ae919..616230d2e0fc1 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1433,6 +1433,21 @@ func (mr *MockStoreMockRecorder) GetTemplateInsightsByInterval(arg0, arg1 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTemplateInsightsByInterval", reflect.TypeOf((*MockStore)(nil).GetTemplateInsightsByInterval), arg0, arg1) } +// GetTemplateInsightsByTemplate mocks base method. +func (m *MockStore) GetTemplateInsightsByTemplate(arg0 context.Context, arg1 database.GetTemplateInsightsByTemplateParams) ([]database.GetTemplateInsightsByTemplateRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTemplateInsightsByTemplate", arg0, arg1) + ret0, _ := ret[0].([]database.GetTemplateInsightsByTemplateRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTemplateInsightsByTemplate indicates an expected call of GetTemplateInsightsByTemplate. +func (mr *MockStoreMockRecorder) GetTemplateInsightsByTemplate(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTemplateInsightsByTemplate", reflect.TypeOf((*MockStore)(nil).GetTemplateInsightsByTemplate), arg0, arg1) +} + // GetTemplateParameterInsights mocks base method. func (m *MockStore) GetTemplateParameterInsights(arg0 context.Context, arg1 database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 99503ba40e3d6..5cbbd9c5bbfb4 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -145,6 +145,7 @@ type sqlcQuerier interface { // that interval will be shorter than a full one. If there is no data for a selected // interval/template, it will be included in the results with 0 active users. GetTemplateInsightsByInterval(ctx context.Context, arg GetTemplateInsightsByIntervalParams) ([]GetTemplateInsightsByIntervalRow, error) + GetTemplateInsightsByTemplate(ctx context.Context, arg GetTemplateInsightsByTemplateParams) ([]GetTemplateInsightsByTemplateRow, error) // GetTemplateParameterInsights does for each template in a given timeframe, // look for the latest workspace build (for every workspace) that has been // created in the timeframe and return the aggregate usage counts of parameter diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fc301d427fa8d..2e480c8958553 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1937,6 +1937,79 @@ func (q *sqlQuerier) GetTemplateInsightsByInterval(ctx context.Context, arg GetT return items, nil } +const getTemplateInsightsByTemplate = `-- name: GetTemplateInsightsByTemplate :many +WITH agent_stats_by_interval_and_user AS ( + SELECT + date_trunc('minute', was.created_at), + was.template_id, + was.user_id, + CASE WHEN SUM(was.session_count_vscode) > 0 THEN 60 ELSE 0 END AS usage_vscode_seconds, + CASE WHEN SUM(was.session_count_jetbrains) > 0 THEN 60 ELSE 0 END AS usage_jetbrains_seconds, + CASE WHEN SUM(was.session_count_reconnecting_pty) > 0 THEN 60 ELSE 0 END AS usage_reconnecting_pty_seconds, + CASE WHEN SUM(was.session_count_ssh) > 0 THEN 60 ELSE 0 END AS usage_ssh_seconds + FROM workspace_agent_stats was + WHERE + was.created_at >= $1::timestamptz + AND was.created_at < $2::timestamptz + AND was.connection_count > 0 + GROUP BY date_trunc('minute', was.created_at), was.template_id, was.user_id +) + +SELECT + template_id, + COALESCE(COUNT(DISTINCT user_id))::bigint AS active_users, + COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, + COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, + COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, + COALESCE(SUM(usage_ssh_seconds), 0)::bigint AS usage_ssh_seconds +FROM agent_stats_by_interval_and_user +GROUP BY template_id +` + +type GetTemplateInsightsByTemplateParams struct { + StartTime time.Time `db:"start_time" json:"start_time"` + EndTime time.Time `db:"end_time" json:"end_time"` +} + +type GetTemplateInsightsByTemplateRow struct { + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + ActiveUsers int64 `db:"active_users" json:"active_users"` + UsageVscodeSeconds int64 `db:"usage_vscode_seconds" json:"usage_vscode_seconds"` + UsageJetbrainsSeconds int64 `db:"usage_jetbrains_seconds" json:"usage_jetbrains_seconds"` + UsageReconnectingPtySeconds int64 `db:"usage_reconnecting_pty_seconds" json:"usage_reconnecting_pty_seconds"` + UsageSshSeconds int64 `db:"usage_ssh_seconds" json:"usage_ssh_seconds"` +} + +func (q *sqlQuerier) GetTemplateInsightsByTemplate(ctx context.Context, arg GetTemplateInsightsByTemplateParams) ([]GetTemplateInsightsByTemplateRow, error) { + rows, err := q.db.QueryContext(ctx, getTemplateInsightsByTemplate, arg.StartTime, arg.EndTime) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetTemplateInsightsByTemplateRow + for rows.Next() { + var i GetTemplateInsightsByTemplateRow + if err := rows.Scan( + &i.TemplateID, + &i.ActiveUsers, + &i.UsageVscodeSeconds, + &i.UsageJetbrainsSeconds, + &i.UsageReconnectingPtySeconds, + &i.UsageSshSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getTemplateParameterInsights = `-- name: GetTemplateParameterInsights :many WITH latest_workspace_builds AS ( SELECT diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 7fb48100d5d8a..49e6e84f90c72 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -134,6 +134,34 @@ SELECT COALESCE(SUM(usage_ssh_seconds), 0)::bigint AS usage_ssh_seconds FROM agent_stats_by_interval_and_user; +-- name: GetTemplateInsightsByTemplate :many +WITH agent_stats_by_interval_and_user AS ( + SELECT + date_trunc('minute', was.created_at), + was.template_id, + was.user_id, + CASE WHEN SUM(was.session_count_vscode) > 0 THEN 60 ELSE 0 END AS usage_vscode_seconds, + CASE WHEN SUM(was.session_count_jetbrains) > 0 THEN 60 ELSE 0 END AS usage_jetbrains_seconds, + CASE WHEN SUM(was.session_count_reconnecting_pty) > 0 THEN 60 ELSE 0 END AS usage_reconnecting_pty_seconds, + CASE WHEN SUM(was.session_count_ssh) > 0 THEN 60 ELSE 0 END AS usage_ssh_seconds + FROM workspace_agent_stats was + WHERE + was.created_at >= @start_time::timestamptz + AND was.created_at < @end_time::timestamptz + AND was.connection_count > 0 + GROUP BY date_trunc('minute', was.created_at), was.template_id, was.user_id +) + +SELECT + template_id, + COALESCE(COUNT(DISTINCT user_id))::bigint AS active_users, + COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, + COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, + COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, + COALESCE(SUM(usage_ssh_seconds), 0)::bigint AS usage_ssh_seconds +FROM agent_stats_by_interval_and_user +GROUP BY template_id; + -- name: GetTemplateAppInsights :many -- GetTemplateAppInsights returns the aggregate usage of each app in a given -- timeframe. The result can be filtered on template_ids, meaning only user data diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 4ff8735990c81..93ec038002887 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -116,7 +116,8 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { return } - templateNames := onlyTemplateNames(templates) + /*templateNames := */ + onlyTemplateNames(templates) } go func() { From a764cef16e67a8a215e6fcd1a2152752cb388ab6 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 12:14:26 +0200 Subject: [PATCH 07/19] Collect metrics data --- .../insights/metricscollector.go | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 93ec038002887..f4e4f6b8d1368 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -2,6 +2,7 @@ package insights import ( "context" + "sync/atomic" "time" "github.com/google/uuid" @@ -24,6 +25,16 @@ type MetricsCollector struct { database database.Store logger slog.Logger duration time.Duration + + data atomic.Pointer[insightsData] +} + +type insightsData struct { + templates []database.GetTemplateInsightsByTemplateRow + apps []database.GetTemplateAppInsightsRow + parameters []database.GetTemplateParameterInsightsRow + + templateNames map[uuid.UUID]string } var _ prometheus.Collector = new(MetricsCollector) @@ -59,7 +70,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // TODO collect iteration time - var userActivity []database.GetUserActivityInsightsRow + var templateInsights []database.GetTemplateInsightsByTemplateRow var appInsights []database.GetTemplateAppInsightsRow var parameterInsights []database.GetTemplateParameterInsightsRow @@ -69,7 +80,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { eg.Go(func() error { var err error - userActivity, err = mc.database.GetUserActivityInsights(egCtx, database.GetUserActivityInsightsParams{ + templateInsights, err = mc.database.GetTemplateInsightsByTemplate(egCtx, database.GetTemplateInsightsByTemplateParams{ StartTime: startTime, EndTime: endTime, }) @@ -107,7 +118,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { } // Phase 2: Collect template IDs, and fetch relevant details - templateIDs := uniqueTemplateIDs(userActivity, appInsights, parameterInsights) + templateIDs := uniqueTemplateIDs(templateInsights, appInsights, parameterInsights) templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ IDs: templateIDs, }) @@ -116,8 +127,15 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { return } - /*templateNames := */ - onlyTemplateNames(templates) + templateNames := onlyTemplateNames(templates) + + mc.data.Store(&insightsData{ + templates: templateInsights, + apps: appInsights, + parameters: parameterInsights, + + templateNames: templateNames, + }) } go func() { @@ -147,17 +165,24 @@ func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) { func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { // Phase 3: Collect metrics - // TODO + data := mc.data.Load() + if data == nil { + return // insights data not loaded yet + } + + for _, templateRow := range data.templates { + metricsCh <- prometheus.MustNewConstMetric(activeUsersDesc, prometheus.GaugeValue, float64(templateRow.ActiveUsers), data.templateNames[templateRow.TemplateID]) + + // TODO applicationsUsageSeconds, parameters + } } // Helper functions below. -func uniqueTemplateIDs(userActivity []database.GetUserActivityInsightsRow, appInsights []database.GetTemplateAppInsightsRow, parameterInsights []database.GetTemplateParameterInsightsRow) []uuid.UUID { +func uniqueTemplateIDs(templateInsights []database.GetTemplateInsightsByTemplateRow, appInsights []database.GetTemplateAppInsightsRow, parameterInsights []database.GetTemplateParameterInsightsRow) []uuid.UUID { tids := map[uuid.UUID]bool{} - for _, t := range userActivity { - for _, tid := range t.TemplateIDs { - tids[tid] = true - } + for _, t := range templateInsights { + tids[t.TemplateID] = true } for _, a := range appInsights { From 32f524581a2b7eb73d62d33f621899db8e040ed5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 12:35:09 +0200 Subject: [PATCH 08/19] Only template --- .../insights/metricscollector.go | 62 +++---------------- 1 file changed, 8 insertions(+), 54 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index f4e4f6b8d1368..684ba9117fc16 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -16,9 +16,7 @@ import ( ) var ( - activeUsersDesc = prometheus.NewDesc("coderd_insights_active_users", "The number of active users of the template.", []string{"template_name"}, nil) - applicationsUsageSecondsDesc = prometheus.NewDesc("coderd_insights_applications_usage_seconds", "The application usage per template.", []string{"application_name", "template_name"}, nil) - parametersDesc = prometheus.NewDesc("coderd_insights_parameters", "The parameter usage per template.", []string{"template_name", "name", "value"}, nil) + activeUsersDesc = prometheus.NewDesc("coderd_insights_active_users", "The number of active users of the template.", []string{"template_name"}, nil) ) type MetricsCollector struct { @@ -30,9 +28,7 @@ type MetricsCollector struct { } type insightsData struct { - templates []database.GetTemplateInsightsByTemplateRow - apps []database.GetTemplateAppInsightsRow - parameters []database.GetTemplateParameterInsightsRow + templates []database.GetTemplateInsightsByTemplateRow templateNames map[uuid.UUID]string } @@ -71,12 +67,11 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // TODO collect iteration time var templateInsights []database.GetTemplateInsightsByTemplateRow - var appInsights []database.GetTemplateAppInsightsRow - var parameterInsights []database.GetTemplateParameterInsightsRow - // Phase I: Fetch insights from database + // Phase 1: Fetch insights from database + // FIXME errorGroup will be used to fetch insights for apps and parameters eg, egCtx := errgroup.WithContext(ctx) - eg.SetLimit(3) + eg.SetLimit(1) eg.Go(func() error { var err error @@ -89,36 +84,13 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { } return err }) - eg.Go(func() error { - var err error - appInsights, err = mc.database.GetTemplateAppInsights(ctx, database.GetTemplateAppInsightsParams{ - StartTime: startTime, - EndTime: endTime, - }) - if err != nil { - mc.logger.Error(ctx, "unable to fetch app insights from database", slog.Error(err)) - } - return err - }) - eg.Go(func() error { - var err error - parameterInsights, err = mc.database.GetTemplateParameterInsights(ctx, database.GetTemplateParameterInsightsParams{ - StartTime: startTime, - EndTime: endTime, - }) - if err != nil { - mc.logger.Error(ctx, "unable to fetch parameter insights from database", slog.Error(err)) - } - return err - }) - err := eg.Wait() if err != nil { return } // Phase 2: Collect template IDs, and fetch relevant details - templateIDs := uniqueTemplateIDs(templateInsights, appInsights, parameterInsights) + templateIDs := uniqueTemplateIDs(templateInsights) templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ IDs: templateIDs, }) @@ -130,9 +102,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { templateNames := onlyTemplateNames(templates) mc.data.Store(&insightsData{ - templates: templateInsights, - apps: appInsights, - parameters: parameterInsights, + templates: templateInsights, templateNames: templateNames, }) @@ -158,8 +128,6 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) { descCh <- activeUsersDesc - descCh <- applicationsUsageSecondsDesc - descCh <- parametersDesc } func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { @@ -172,31 +140,17 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { for _, templateRow := range data.templates { metricsCh <- prometheus.MustNewConstMetric(activeUsersDesc, prometheus.GaugeValue, float64(templateRow.ActiveUsers), data.templateNames[templateRow.TemplateID]) - - // TODO applicationsUsageSeconds, parameters } } // Helper functions below. -func uniqueTemplateIDs(templateInsights []database.GetTemplateInsightsByTemplateRow, appInsights []database.GetTemplateAppInsightsRow, parameterInsights []database.GetTemplateParameterInsightsRow) []uuid.UUID { +func uniqueTemplateIDs(templateInsights []database.GetTemplateInsightsByTemplateRow) []uuid.UUID { tids := map[uuid.UUID]bool{} for _, t := range templateInsights { tids[t.TemplateID] = true } - for _, a := range appInsights { - for _, tid := range a.TemplateIDs { - tids[tid] = true - } - } - - for _, p := range parameterInsights { - for _, tid := range p.TemplateIDs { - tids[tid] = true - } - } - uniqueUUIDs := make([]uuid.UUID, len(tids)) var i int for t := range tids { From 0c81a7a564a87ad767dfdfeb008a6f649efdcb95 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 12:50:08 +0200 Subject: [PATCH 09/19] Fixes --- coderd/prometheusmetrics/insights/metricscollector.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 684ba9117fc16..5b6a0cf55c757 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -16,7 +16,7 @@ import ( ) var ( - activeUsersDesc = prometheus.NewDesc("coderd_insights_active_users", "The number of active users of the template.", []string{"template_name"}, nil) + templatesActiveUsersDesc = prometheus.NewDesc("coderd_insights_templates_active_users", "The number of active users of the template.", []string{"template_name"}, nil) ) type MetricsCollector struct { @@ -101,6 +101,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { templateNames := onlyTemplateNames(templates) + // Refresh the collector state mc.data.Store(&insightsData{ templates: templateInsights, @@ -127,7 +128,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { } func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) { - descCh <- activeUsersDesc + descCh <- templatesActiveUsersDesc } func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { @@ -139,7 +140,7 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { } for _, templateRow := range data.templates { - metricsCh <- prometheus.MustNewConstMetric(activeUsersDesc, prometheus.GaugeValue, float64(templateRow.ActiveUsers), data.templateNames[templateRow.TemplateID]) + metricsCh <- prometheus.MustNewConstMetric(templatesActiveUsersDesc, prometheus.GaugeValue, float64(templateRow.ActiveUsers), data.templateNames[templateRow.TemplateID]) } } @@ -155,6 +156,7 @@ func uniqueTemplateIDs(templateInsights []database.GetTemplateInsightsByTemplate var i int for t := range tids { uniqueUUIDs[i] = t + i++ } return uniqueUUIDs } From 432af16932d8c998cd690f1cf9f873e797c85c46 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 12:55:30 +0200 Subject: [PATCH 10/19] fmt --- coderd/prometheusmetrics/insights/metricscollector.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 5b6a0cf55c757..c052cd6646c35 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -15,9 +15,7 @@ import ( "github.com/coder/coder/v2/coderd/database" ) -var ( - templatesActiveUsersDesc = prometheus.NewDesc("coderd_insights_templates_active_users", "The number of active users of the template.", []string{"template_name"}, nil) -) +var templatesActiveUsersDesc = prometheus.NewDesc("coderd_insights_templates_active_users", "The number of active users of the template.", []string{"template_name"}, nil) type MetricsCollector struct { database database.Store From 5ebbfe93baf292e6ad28b95b47af4139dffb99a3 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 12:57:57 +0200 Subject: [PATCH 11/19] fix: dbauthz --- coderd/database/dbauthz/dbauthz.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9eaacab556ab9..07f4f792bc324 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1325,7 +1325,10 @@ func (q *querier) GetTemplateInsightsByInterval(ctx context.Context, arg databas } func (q *querier) GetTemplateInsightsByTemplate(ctx context.Context, arg database.GetTemplateInsightsByTemplateParams) ([]database.GetTemplateInsightsByTemplateRow, error) { - panic("not implemented") + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { + return nil, err + } + return q.db.GetTemplateInsightsByTemplate(ctx, arg) } func (q *querier) GetTemplateParameterInsights(ctx context.Context, arg database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { From f4c12021854ccdad2e60d0185570954a9cce7734 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 14:58:26 +0200 Subject: [PATCH 12/19] Unit test --- cli/server.go | 2 +- .../insights/metricscollector.go | 53 ++++---- .../insights/metricscollector_test.go | 123 ++++++++++++++++++ 3 files changed, 154 insertions(+), 24 deletions(-) create mode 100644 coderd/prometheusmetrics/insights/metricscollector_test.go diff --git a/cli/server.go b/cli/server.go index 2a33b5ae2e90d..3ec0c3c631b3f 100644 --- a/cli/server.go +++ b/cli/server.go @@ -199,7 +199,7 @@ func enablePrometheus( } afterCtx(ctx, closeWorkspacesFunc) - insightsMetricsCollector, err := insights.NewMetricsCollector(options.Database, options.Logger, 0) + insightsMetricsCollector, err := insights.NewMetricsCollector(options.Database, options.Logger, 0, 0) if err != nil { return nil, xerrors.Errorf("unable to initialize insights metrics collector: %w", err) } diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index c052cd6646c35..6ac9dba185556 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -18,9 +18,10 @@ import ( var templatesActiveUsersDesc = prometheus.NewDesc("coderd_insights_templates_active_users", "The number of active users of the template.", []string{"template_name"}, nil) type MetricsCollector struct { - database database.Store - logger slog.Logger - duration time.Duration + database database.Store + logger slog.Logger + timeWindow time.Duration + tickInterval time.Duration data atomic.Pointer[insightsData] } @@ -33,18 +34,22 @@ type insightsData struct { var _ prometheus.Collector = new(MetricsCollector) -func NewMetricsCollector(db database.Store, logger slog.Logger, duration time.Duration) (*MetricsCollector, error) { - if duration == 0 { - duration = 5 * time.Minute +func NewMetricsCollector(db database.Store, logger slog.Logger, timeWindow time.Duration, tickInterval time.Duration) (*MetricsCollector, error) { + if timeWindow == 0 { + timeWindow = 5 * time.Minute } - if duration < 5*time.Minute { - return nil, xerrors.Errorf("refresh interval must be at least 5 mins") + if timeWindow < 5*time.Minute { + return nil, xerrors.Errorf("time window must be at least 5 mins") + } + if tickInterval == 0 { + tickInterval = timeWindow } return &MetricsCollector{ - database: db, - logger: logger.Named("insights_metrics_collector"), - duration: duration, + database: db, + logger: logger.Named("insights_metrics_collector"), + timeWindow: timeWindow, + tickInterval: tickInterval, }, nil } @@ -56,10 +61,10 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // correct duration after executing once. ticker := time.NewTicker(time.Nanosecond) doTick := func() { - defer ticker.Reset(mc.duration) + defer ticker.Reset(mc.tickInterval) now := time.Now() - startTime := now.Add(-mc.duration) + startTime := now.Add(-mc.timeWindow) endTime := now // TODO collect iteration time @@ -89,20 +94,22 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // Phase 2: Collect template IDs, and fetch relevant details templateIDs := uniqueTemplateIDs(templateInsights) - templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ - IDs: templateIDs, - }) - if err != nil { - mc.logger.Error(ctx, "unable to fetch template details from database", slog.Error(err)) - return - } - templateNames := onlyTemplateNames(templates) + templateNames := map[uuid.UUID]string{} + if len(templateIDs) > 0 { + templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ + IDs: templateIDs, + }) + if err != nil { + mc.logger.Error(ctx, "unable to fetch template details from database", slog.Error(err)) + return + } + templateNames = onlyTemplateNames(templates) + } // Refresh the collector state mc.data.Store(&insightsData{ - templates: templateInsights, - + templates: templateInsights, templateNames: templateNames, }) } diff --git a/coderd/prometheusmetrics/insights/metricscollector_test.go b/coderd/prometheusmetrics/insights/metricscollector_test.go new file mode 100644 index 0000000000000..0b817109485fb --- /dev/null +++ b/coderd/prometheusmetrics/insights/metricscollector_test.go @@ -0,0 +1,123 @@ +package insights_test + +import ( + "context" + "io" + "testing" + "time" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent/agenttest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/prometheusmetrics/insights" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/provisioner/echo" + "github.com/coder/coder/v2/testutil" +) + +func TestCollect_TemplateInsights(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil) + db, ps := dbtestutil.NewDB(t) + + options := &coderdtest.Options{ + IncludeProvisionerDaemon: true, + AgentStatsRefreshInterval: time.Millisecond * 100, + Database: db, + Pubsub: ps, + } + client := coderdtest.New(t, options) + + // Given + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Initialize metrics collector + mc, err := insights.NewMetricsCollector(db, logger.Named("metrics_collector"), 0, time.Millisecond) + require.NoError(t, err) + + registry := prometheus.NewRegistry() + registry.Register(mc) + + closeFunc, err := mc.Run(ctx) + require.NoError(t, err) + t.Cleanup(closeFunc) + + // Create two users, one that will appear in the report and another that + // won't (due to not having/using a workspace). + user := coderdtest.CreateFirstUser(t, client) + _, _ = coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + require.Empty(t, template.BuildTimeStats[codersdk.WorkspaceTransitionStart]) + + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + + // Start an agent so that we can generate stats. + _ = agenttest.New(t, client.URL, authToken) + resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + + // Connect to the agent to generate usage/latency stats. + conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{ + Logger: logger.Named("client"), + }) + require.NoError(t, err) + defer conn.Close() + + sshConn, err := conn.SSHClient(ctx) + require.NoError(t, err) + defer sshConn.Close() + + sess, err := sshConn.NewSession() + require.NoError(t, err) + defer sess.Close() + + r, w := io.Pipe() + defer r.Close() + defer w.Close() + sess.Stdin = r + sess.Stdout = io.Discard + err = sess.Start("cat") + require.NoError(t, err) + + collected := map[string]int{} + assert.Eventuallyf(t, func() bool { + // When + metrics, err := registry.Gather() + require.NoError(t, err) + + // Then + for _, metric := range metrics { + switch metric.GetName() { + case "coderd_insights_templates_active_users": + for _, m := range metric.Metric { + collected[metric.GetName()] = int(m.Gauge.GetValue()) + } + default: + require.FailNowf(t, "unexpected metric collected", "metric: %s", metric.GetName()) + } + } + + return len(collected) > 0 + }, testutil.WaitMedium, testutil.IntervalFast, "template insights are missing") + + // We got our latency metrics, close the connection. + _ = sess.Close() + _ = sshConn.Close() + + require.EqualValues(t, nil, collected) +} From 62ad6a442a0d51ba323aa70b9a4a8ebf1439fd5e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 15:27:57 +0200 Subject: [PATCH 13/19] golden file --- .../insights/metricscollector_test.go | 12 ++++++++++-- .../insights/testdata/insights-metrics.json | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 coderd/prometheusmetrics/insights/testdata/insights-metrics.json diff --git a/coderd/prometheusmetrics/insights/metricscollector_test.go b/coderd/prometheusmetrics/insights/metricscollector_test.go index 0b817109485fb..32e0e1348c78c 100644 --- a/coderd/prometheusmetrics/insights/metricscollector_test.go +++ b/coderd/prometheusmetrics/insights/metricscollector_test.go @@ -2,7 +2,9 @@ package insights_test import ( "context" + "encoding/json" "io" + "os" "testing" "time" @@ -94,6 +96,12 @@ func TestCollect_TemplateInsights(t *testing.T) { err = sess.Start("cat") require.NoError(t, err) + goldenFile, err := os.ReadFile("testdata/insights-metrics.json") + require.NoError(t, err) + golden := map[string]int{} + err = json.Unmarshal(goldenFile, &golden) + require.NoError(t, err) + collected := map[string]int{} assert.Eventuallyf(t, func() bool { // When @@ -112,12 +120,12 @@ func TestCollect_TemplateInsights(t *testing.T) { } } - return len(collected) > 0 + return assert.ObjectsAreEqualValues(golden, collected) }, testutil.WaitMedium, testutil.IntervalFast, "template insights are missing") // We got our latency metrics, close the connection. _ = sess.Close() _ = sshConn.Close() - require.EqualValues(t, nil, collected) + require.EqualValues(t, golden, collected) } diff --git a/coderd/prometheusmetrics/insights/testdata/insights-metrics.json b/coderd/prometheusmetrics/insights/testdata/insights-metrics.json new file mode 100644 index 0000000000000..01c96a78b64a4 --- /dev/null +++ b/coderd/prometheusmetrics/insights/testdata/insights-metrics.json @@ -0,0 +1,3 @@ +{ + "coderd_insights_templates_active_users": 1 +} From 85dc07841ec06a3b93c67cee95ff3e6949efcf83 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 15:38:07 +0200 Subject: [PATCH 14/19] Fix --- coderd/prometheusmetrics/insights/metricscollector.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 6ac9dba185556..55dc37cd5b0fe 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -67,15 +67,13 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { startTime := now.Add(-mc.timeWindow) endTime := now - // TODO collect iteration time - - var templateInsights []database.GetTemplateInsightsByTemplateRow - // Phase 1: Fetch insights from database // FIXME errorGroup will be used to fetch insights for apps and parameters eg, egCtx := errgroup.WithContext(ctx) eg.SetLimit(1) + var templateInsights []database.GetTemplateInsightsByTemplateRow + eg.Go(func() error { var err error templateInsights, err = mc.database.GetTemplateInsightsByTemplate(egCtx, database.GetTemplateInsightsByTemplateParams{ From 2d03fedb9b5b1d80f3ef3a0bb67cbb15d8bb40dc Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 15:53:07 +0200 Subject: [PATCH 15/19] Fix? --- .../insights/metricscollector_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector_test.go b/coderd/prometheusmetrics/insights/metricscollector_test.go index 32e0e1348c78c..dc7b6095a562a 100644 --- a/coderd/prometheusmetrics/insights/metricscollector_test.go +++ b/coderd/prometheusmetrics/insights/metricscollector_test.go @@ -38,9 +38,6 @@ func TestCollect_TemplateInsights(t *testing.T) { client := coderdtest.New(t, options) // Given - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - // Initialize metrics collector mc, err := insights.NewMetricsCollector(db, logger.Named("metrics_collector"), 0, time.Millisecond) require.NoError(t, err) @@ -48,10 +45,6 @@ func TestCollect_TemplateInsights(t *testing.T) { registry := prometheus.NewRegistry() registry.Register(mc) - closeFunc, err := mc.Run(ctx) - require.NoError(t, err) - t.Cleanup(closeFunc) - // Create two users, one that will appear in the report and another that // won't (due to not having/using a workspace). user := coderdtest.CreateFirstUser(t, client) @@ -73,6 +66,14 @@ func TestCollect_TemplateInsights(t *testing.T) { _ = agenttest.New(t, client.URL, authToken) resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Run metrics collector + closeFunc, err := mc.Run(ctx) + require.NoError(t, err) + t.Cleanup(closeFunc) + // Connect to the agent to generate usage/latency stats. conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{ Logger: logger.Named("client"), From e41bfbcb54427b17a90482b3d6ced3bba803a629 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 18 Oct 2023 16:09:37 +0200 Subject: [PATCH 16/19] t.cleanup to defer --- coderd/prometheusmetrics/insights/metricscollector_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector_test.go b/coderd/prometheusmetrics/insights/metricscollector_test.go index dc7b6095a562a..0c1726a910e96 100644 --- a/coderd/prometheusmetrics/insights/metricscollector_test.go +++ b/coderd/prometheusmetrics/insights/metricscollector_test.go @@ -26,7 +26,7 @@ import ( func TestCollect_TemplateInsights(t *testing.T) { t.Parallel() - logger := slogtest.Make(t, nil) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) db, ps := dbtestutil.NewDB(t) options := &coderdtest.Options{ @@ -39,7 +39,7 @@ func TestCollect_TemplateInsights(t *testing.T) { // Given // Initialize metrics collector - mc, err := insights.NewMetricsCollector(db, logger.Named("metrics_collector"), 0, time.Millisecond) + mc, err := insights.NewMetricsCollector(db, logger, 0, time.Second) require.NoError(t, err) registry := prometheus.NewRegistry() @@ -72,7 +72,7 @@ func TestCollect_TemplateInsights(t *testing.T) { // Run metrics collector closeFunc, err := mc.Run(ctx) require.NoError(t, err) - t.Cleanup(closeFunc) + defer closeFunc() // Connect to the agent to generate usage/latency stats. conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{ From cd8a809ed16bb25ac26c75e2fb4c0a91c82ffd40 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 19 Oct 2023 10:11:06 +0200 Subject: [PATCH 17/19] ticker.stop --- coderd/prometheusmetrics/insights/metricscollector.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 55dc37cd5b0fe..fa4b68bc9d96d 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -120,6 +120,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { case <-ctx.Done(): return case <-ticker.C: + ticker.Stop() doTick() } } From 3dbb6eb2c3266d10da4bb7d23449f829c7bf755e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 19 Oct 2023 10:20:40 +0200 Subject: [PATCH 18/19] GROUP BY alias --- coderd/database/queries.sql.go | 4 ++-- coderd/database/queries/insights.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2e480c8958553..d8b30e0d9fc4a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1940,7 +1940,7 @@ func (q *sqlQuerier) GetTemplateInsightsByInterval(ctx context.Context, arg GetT const getTemplateInsightsByTemplate = `-- name: GetTemplateInsightsByTemplate :many WITH agent_stats_by_interval_and_user AS ( SELECT - date_trunc('minute', was.created_at), + date_trunc('minute', was.created_at) AS created_at_trunc, was.template_id, was.user_id, CASE WHEN SUM(was.session_count_vscode) > 0 THEN 60 ELSE 0 END AS usage_vscode_seconds, @@ -1952,7 +1952,7 @@ WITH agent_stats_by_interval_and_user AS ( was.created_at >= $1::timestamptz AND was.created_at < $2::timestamptz AND was.connection_count > 0 - GROUP BY date_trunc('minute', was.created_at), was.template_id, was.user_id + GROUP BY created_at_trunc, was.template_id, was.user_id ) SELECT diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 49e6e84f90c72..01863fede1aed 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -137,7 +137,7 @@ FROM agent_stats_by_interval_and_user; -- name: GetTemplateInsightsByTemplate :many WITH agent_stats_by_interval_and_user AS ( SELECT - date_trunc('minute', was.created_at), + date_trunc('minute', was.created_at) AS created_at_trunc, was.template_id, was.user_id, CASE WHEN SUM(was.session_count_vscode) > 0 THEN 60 ELSE 0 END AS usage_vscode_seconds, @@ -149,7 +149,7 @@ WITH agent_stats_by_interval_and_user AS ( was.created_at >= @start_time::timestamptz AND was.created_at < @end_time::timestamptz AND was.connection_count > 0 - GROUP BY date_trunc('minute', was.created_at), was.template_id, was.user_id + GROUP BY created_at_trunc, was.template_id, was.user_id ) SELECT From c25c49962ddd57032fc98390868ab13e9f3bee47 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 19 Oct 2023 10:30:07 +0200 Subject: [PATCH 19/19] Preallocate --- coderd/prometheusmetrics/insights/metricscollector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index fa4b68bc9d96d..d19785e8e6131 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -93,7 +93,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { // Phase 2: Collect template IDs, and fetch relevant details templateIDs := uniqueTemplateIDs(templateInsights) - templateNames := map[uuid.UUID]string{} + templateNames := make(map[uuid.UUID]string, len(templateIDs)) if len(templateIDs) > 0 { templates, err := mc.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ IDs: templateIDs,