Skip to content

Commit 53f6c63

Browse files
mtojekmtojek
committed
refactored
1 parent 2edc7d5 commit 53f6c63

File tree

3 files changed

+52
-71
lines changed

3 files changed

+52
-71
lines changed

‎coderd/database/dbmem/dbmem.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,20 +1690,6 @@ func (*FakeQuerier) DeleteOldNotificationMessages(_ context.Context) error {
16901690
return nil
16911691
}
16921692

1693-
func (q *FakeQuerier) DeleteOldNotificationReportGeneratorLogs(_ context.Context, params database.DeleteOldNotificationReportGeneratorLogsParams) error {
1694-
q.mutex.Lock()
1695-
defer q.mutex.Unlock()
1696-
1697-
var validLogs []database.NotificationReportGeneratorLog
1698-
for _, record := range q.notificationReportGeneratorLogs {
1699-
if record.NotificationTemplateID != params.NotificationTemplateID || record.LastGeneratedAt.After(params.Before) {
1700-
validLogs = append(validLogs, record)
1701-
}
1702-
}
1703-
q.notificationReportGeneratorLogs = validLogs
1704-
return nil
1705-
}
1706-
17071693
func (q *FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error {
17081694
q.mutex.Lock()
17091695
defer q.mutex.Unlock()

‎coderd/database/queries/notifications.sql

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,3 @@ WHERE
189189
INSERT INTO notification_report_generator_logs (notification_template_id, last_generated_at) VALUES (@notification_template_id, @last_generated_at)
190190
ON CONFLICT (notification_template_id) DO UPDATE set last_generated_at = EXCLUDED.last_generated_at
191191
WHERE report_generator_logs.notification_template_id = EXCLUDED.notification_template_id;
192-
193-
-- name: DeleteOldNotificationReportGeneratorLogs :exec
194-
-- Delete report generator logs that have been created at least a @before date.
195-
DELETE FROM notification_report_generator_logs WHERE last_generated_at < @before::timestamptz AND notification_template_id = @notification_template_id;

‎coderd/notifications/reports/generator.go

Lines changed: 52 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -106,59 +106,67 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
106106
now := clk.Now()
107107
since := now.Add(-failedWorkspaceBuildsReportFrequency)
108108

109-
statsRows, err := db.GetWorkspaceBuildStatsByTemplates(ctx, dbtime.Time(since).UTC())
110-
if err != nil {
111-
return xerrors.Errorf("unable to fetch failed workspace builds: %w", err)
109+
// Firstly, check if this is the first run of the job ever
110+
reportLog, err := db.GetNotificationReportGeneratorLogByTemplate(ctx, notifications.TemplateWorkspaceBuildsFailedReport)
111+
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
112+
return xerrors.Errorf("unable to read report generator log: %w", err)
112113
}
114+
if xerrors.Is(err, sql.ErrNoRows) {
115+
// First run? Check-in the job, and get back after one week.
116+
logger.Info(ctx, "report generator is executing the job for the first time", slog.F("notification_template_id", notifications.TemplateWorkspaceBuildsFailedReport))
113117

114-
processedUsers := map[uuid.UUID]bool{}
115-
for _, stats := range statsRows {
116-
var failedBuilds []database.GetFailedWorkspaceBuildsByTemplateIDRow
117-
reportData := map[string]any{}
118+
err = db.UpsertNotificationReportGeneratorLog(ctx, database.UpsertNotificationReportGeneratorLogParams{
119+
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
120+
LastGeneratedAt: dbtime.Time(now).UTC(),
121+
})
122+
if err != nil {
123+
return xerrors.Errorf("unable to update report generator logs (first time execution): %w", err)
124+
}
125+
return nil
126+
}
118127

119-
if stats.FailedBuilds > 0 {
120-
failedBuilds, err = db.GetFailedWorkspaceBuildsByTemplateID(ctx, database.GetFailedWorkspaceBuildsByTemplateIDParams{
121-
TemplateID: stats.TemplateID,
122-
Since: dbtime.Time(since).UTC(),
123-
})
124-
if err != nil {
125-
logger.Error(ctx, "unable to fetch failed workspace builds", slog.F("template_id", stats.TemplateID), slog.Error(err))
126-
continue
127-
}
128+
// Secondly, check if the job has not been running recently
129+
if !reportLog.LastGeneratedAt.IsZero() && reportLog.LastGeneratedAt.Add(failedWorkspaceBuildsReportFrequency).After(now) {
130+
return nil // reports sent recently, no need to send them now
131+
}
132+
133+
// Thirdly, fetch workspace build stats by templates
134+
templateStatsRows, err := db.GetWorkspaceBuildStatsByTemplates(ctx, dbtime.Time(since).UTC())
135+
if err != nil {
136+
return xerrors.Errorf("unable to fetch failed workspace builds: %w", err)
137+
}
128138

129-
// There are some failed builds, so we have to prepare input data for the report.
130-
reportData = buildDataForReportFailedWorkspaceBuilds(stats, failedBuilds)
139+
for _, stats := range templateStatsRows {
140+
if stats.FailedBuilds == 0 {
141+
logger.Error(ctx, "no failed workspace builds found for template", slog.F("template_id", stats.TemplateID), slog.Error(err))
142+
continue
131143
}
132144

145+
// Fetch template admins with org access to the templates
133146
templateAdmins, err := findTemplateAdmins(ctx, db, stats)
134147
if err != nil {
135-
logger.Error(ctx, "unable to find template admins", slog.F("template_id", stats.TemplateID), slog.Error(err))
148+
logger.Error(ctx, "unable to find template admins for template", slog.F("template_id", stats.TemplateID), slog.Error(err))
136149
continue
137150
}
138151

139-
for _, templateAdmin := range templateAdmins {
140-
reportLog, err := db.GetNotificationReportGeneratorLogByTemplate(ctx, notifications.TemplateWorkspaceBuildsFailedReport)
141-
if err != nil && !xerrors.Is(err, sql.ErrNoRows) { // sql.ErrNoRows: report not generated yet
142-
continue
143-
}
144-
145-
if !reportLog.LastGeneratedAt.IsZero() && reportLog.LastGeneratedAt.Add(failedWorkspaceBuildsReportFrequency).After(now) {
146-
// report generated recently, no need to send it now
147-
continue
148-
}
149-
150-
processedUsers[templateAdmin.ID] = true
151-
152-
if len(failedBuilds) == 0 {
153-
// no failed workspace builds, no need to send the report
154-
continue
155-
}
152+
// Fetch failed builds by the template
153+
failedBuilds, err := db.GetFailedWorkspaceBuildsByTemplateID(ctx, database.GetFailedWorkspaceBuildsByTemplateIDParams{
154+
TemplateID: stats.TemplateID,
155+
Since: dbtime.Time(since).UTC(),
156+
})
157+
if err != nil {
158+
logger.Error(ctx, "unable to fetch failed workspace builds", slog.F("template_id", stats.TemplateID), slog.Error(err))
159+
continue
160+
}
161+
reportData := buildDataForReportFailedWorkspaceBuilds(stats, failedBuilds)
156162

157-
templateDisplayName := stats.TemplateDisplayName
158-
if templateDisplayName == "" {
159-
templateDisplayName = stats.TemplateName
160-
}
163+
// Send reports to template admins
164+
templateDisplayName := stats.TemplateDisplayName
165+
if templateDisplayName == "" {
166+
templateDisplayName = stats.TemplateName
167+
}
161168

169+
for _, templateAdmin := range templateAdmins {
162170
if _, err := enqueuer.EnqueueWithData(ctx, templateAdmin.ID, notifications.TemplateWorkspaceBuildsFailedReport,
163171
map[string]string{
164172
"template_name": stats.TemplateName,
@@ -173,22 +181,13 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
173181
}
174182
}
175183

176-
for u := range processedUsers {
177-
err = db.UpsertNotificationReportGeneratorLog(ctx, database.UpsertNotificationReportGeneratorLogParams{
178-
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
179-
LastGeneratedAt: dbtime.Time(now).UTC(),
180-
})
181-
if err != nil {
182-
logger.Error(ctx, "unable to update report generator logs", slog.F("user_id", u), slog.Error(err))
183-
}
184-
}
185-
186-
err = db.DeleteOldNotificationReportGeneratorLogs(ctx, database.DeleteOldNotificationReportGeneratorLogsParams{
184+
// Lastly, update the timestamp in the generator log.
185+
err = db.UpsertNotificationReportGeneratorLog(ctx, database.UpsertNotificationReportGeneratorLogParams{
187186
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
188-
Before: dbtime.Time(now.Add(-failedWorkspaceBuildsReportFrequency - time.Hour)).UTC(),
187+
LastGeneratedAt: dbtime.Time(now).UTC(),
189188
})
190189
if err != nil {
191-
return xerrors.Errorf("unable to delete old report generator logs: %w", err)
190+
return xerrors.Errorf("unable to update report generator logs: %w", err)
192191
}
193192
return nil
194193
}

0 commit comments

Comments
 (0)