Skip to content

chore: unify activity into workspaceapps.StatsCollector #13355

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
move more
  • Loading branch information
f0ssel committed May 23, 2024
commit 636b5b8d69e815fde1dd2ece6b2f7efcb28d45bf
52 changes: 8 additions & 44 deletions coderd/agentapi/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"github.com/coder/coder/v2/coderd/database/pubsub"
"github.com/coder/coder/v2/coderd/prometheusmetrics"
"github.com/coder/coder/v2/coderd/schedule"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/coderd/workspaceapps"
)

type StatsBatcher interface {
Expand All @@ -34,6 +34,7 @@
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
AgentStatsRefreshInterval time.Duration
UpdateAgentMetricsFn func(ctx context.Context, labels prometheusmetrics.AgentMetricLabels, metrics []*agentproto.Stats_Metric)
StatsCollector workspaceapps.StatsCollector

TimeNowFn func() time.Time // defaults to dbtime.Now()
}
Expand Down Expand Up @@ -70,28 +71,6 @@
)

now := a.now()
// if req.Stats.ConnectionCount > 0 {
// var nextAutostart time.Time
// if workspace.AutostartSchedule.String != "" {
// templateSchedule, err := (*(a.TemplateScheduleStore.Load())).Get(ctx, a.Database, workspace.TemplateID)
// // If the template schedule fails to load, just default to bumping
// // without the next transition and log it.
// if err != nil {
// a.Log.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
// slog.F("workspace_id", workspace.ID),
// slog.F("template_id", workspace.TemplateID),
// slog.Error(err),
// )
// } else {
// next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
// if allowed {
// nextAutostart = next
// }
// }
// }
// ActivityBumpWorkspace(ctx, a.Log.Named("activity_bump"), a.Database, workspace.ID, nextAutostart)
// }

var errGroup errgroup.Group
errGroup.Go(func() error {
err := a.StatsBatcher.Add(now, workspaceAgent.ID, workspace.TemplateID, workspace.OwnerID, workspace.ID, req.Stats)
Expand All @@ -101,17 +80,6 @@
}
return nil
})
errGroup.Go(func() error {
// nolint:gocritic // (#13146) Will be moved soon as part of refactor.
err := a.Database.UpdateWorkspaceLastUsedAt(ctx, database.UpdateWorkspaceLastUsedAtParams{
ID: workspace.ID,
LastUsedAt: now,
})
if err != nil {
return xerrors.Errorf("update workspace LastUsedAt: %w", err)
}
return nil
})
if a.UpdateAgentMetricsFn != nil {
errGroup.Go(func() error {
user, err := a.Database.GetUserByID(ctx, workspace.OwnerID)
Expand All @@ -133,16 +101,12 @@
return nil, xerrors.Errorf("update stats in database: %w", err)
}

// Tell the frontend about the new agent report, now that everything is updated
a.publishWorkspaceAgentStats(ctx, workspace.ID)
// Flushing the stats collector will update last_used_at,
// dealine for the workspace, and will publish a workspace update event.

Check warning on line 105 in coderd/agentapi/stats.go

View workflow job for this annotation

GitHub Actions / lint

"dealine" should be "deadline".
a.StatsCollector.CollectAndFlush(ctx, workspaceapps.StatsReport{
WorkspaceID: workspace.ID,
// TODO: fill out
})

return res, nil
}

func (a *StatsAPI) publishWorkspaceAgentStats(ctx context.Context, workspaceID uuid.UUID) {
err := a.Pubsub.Publish(codersdk.WorkspaceNotifyChannel(workspaceID), []byte{})
if err != nil {
a.Log.Warn(ctx, "failed to publish workspace agent stats",
slog.F("workspace_id", workspaceID), slog.Error(err))
}
}
61 changes: 61 additions & 0 deletions coderd/workspaceapps/activity_bump.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package workspaceapps

import (
"context"
"time"

"github.com/google/uuid"
"golang.org/x/xerrors"

"cdr.dev/slog"
"github.com/coder/coder/v2/coderd/database"
)

// ActivityBumpWorkspace automatically bumps the workspace's auto-off timer
// if it is set to expire soon. The deadline will be bumped by 1 hour*.
// If the bump crosses over an autostart time, the workspace will be
// bumped by the workspace ttl instead.
//
// If nextAutostart is the zero value or in the past, the workspace
// will be bumped by 1 hour.
// It handles the edge case in the example:
// 1. Autostart is set to 9am.
// 2. User works all day, and leaves a terminal open to the workspace overnight.
// 3. The open terminal continually bumps the workspace deadline.
// 4. 9am the next day, the activity bump pushes to 10am.
// 5. If the user goes inactive for 1 hour during the day, the workspace will
// now stop, because it has been extended by 1 hour durations. Despite the TTL
// being set to 8hrs from the autostart time.
//
// So the issue is that when the workspace is bumped across an autostart
// deadline, we should treat the workspace as being "started" again and
// extend the deadline by the autostart time + workspace ttl instead.
//
// The issue still remains with build_max_deadline. We need to respect the original
// maximum deadline, so that will need to be handled separately.
// A way to avoid this is to configure the max deadline to something that will not
// span more than 1 day. This will force the workspace to restart and reset the deadline
// each morning when it autostarts.
func ActivityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID, nextAutostart time.Time) {
// We set a short timeout so if the app is under load, these
// low priority operations fail first.
ctx, cancel := context.WithTimeout(ctx, time.Second*15)
defer cancel()
err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
NextAutostart: nextAutostart.UTC(),
WorkspaceID: workspaceID,
})
if err != nil {
if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) {
// Bump will fail if the context is canceled, but this is ok.
log.Error(ctx, "activity bump failed", slog.Error(err),
slog.F("workspace_id", workspaceID),
)
}
return
}

log.Debug(ctx, "bumped deadline from activity",
slog.F("workspace_id", workspaceID),
)
}
12 changes: 10 additions & 2 deletions coderd/workspaceapps/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import (

"cdr.dev/slog"

"github.com/coder/coder/v2/coderd/agentapi"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/database/pubsub"
"github.com/coder/coder/v2/coderd/schedule"
"github.com/coder/coder/v2/coderd/util/slice"
"github.com/coder/coder/v2/codersdk"
)

const (
Expand Down Expand Up @@ -63,6 +64,7 @@ var _ StatsReporter = (*StatsDBReporter)(nil)
// StatsDBReporter writes workspace app StatsReports to the database.
type StatsDBReporter struct {
db database.Store
pubsub pubsub.Pubsub
logger slog.Logger
templateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
batchSize int
Expand Down Expand Up @@ -171,7 +173,13 @@ func (r *StatsDBReporter) Report(ctx context.Context, stats []StatsReport) error
}
}
}
agentapi.ActivityBumpWorkspace(ctx, r.logger.Named("activity_bump"), r.db, workspace.ID, nextAutostart)
ActivityBumpWorkspace(ctx, r.logger.Named("activity_bump"), r.db, workspace.ID, nextAutostart)

err := r.pubsub.Publish(codersdk.WorkspaceNotifyChannel(workspace.ID), []byte{})
if err != nil {
r.logger.Warn(ctx, "failed to publish workspace agent stats",
slog.F("workspace_id", workspace.ID), slog.Error(err))
}
}

return nil
Expand Down
Loading