Skip to content

feat: expose workspace statuses (with details) as a prometheus metric #12762

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Apr 2, 2024
Merged

feat: expose workspace statuses (with details) as a prometheus metric #12762

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
843d650
Add metric to show operators what statuses workspaces are in, with re…
dannykopping Mar 25, 2024
2ed42a3
Clear gauges if no db entries are found
dannykopping Mar 26, 2024
c31b498
Change default refresh rates of Agent and Workspaces calls which seem…
dannykopping Mar 26, 2024
2f5a948
Added tests for workspace detail metrics
dannykopping Mar 26, 2024
fc61d37
Refactor common funcs
dannykopping Mar 26, 2024
6f95371
Remove unnecessary metric from test
dannykopping Mar 26, 2024
f42af07
Use NoLock variety to fix hung tests
dannykopping Mar 26, 2024
2cb8ccc
Fix duration on windows builds
dannykopping Mar 26, 2024
b118044
Changing join type so that latest_build_status is non-nullable
dannykopping Mar 27, 2024
acd104c
Remove SuperShort duration as extraneous
dannykopping Mar 28, 2024
a333f98
Remove workspace cardinality; rename for clarity
dannykopping Mar 28, 2024
c920508
Fixing tests
dannykopping Mar 28, 2024
cf14b9d
Merge branch 'main' of github.com:/coder/coder into dk/workspace-metric
dannykopping Mar 28, 2024
a94914f
Merge branch 'main' of github.com:/coder/coder into dk/workspace-metric
dannykopping Mar 28, 2024
8e6cde9
Renaming for clarity
dannykopping Mar 28, 2024
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
Changing join type so that latest_build_status is non-nullable 
Signed-off-by: Danny Kopping <danny@coder.com>
  • Loading branch information
@dannykopping
dannykopping committed Mar 27, 2024
commit b118044549d20a76ffe6c1a45a440a64e3f7f9a9
2 changes: 1 addition & 1 deletion coderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac
}

if pj, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID); err == nil {
wr.LatestBuildStatus = database.NullProvisionerJobStatus{ProvisionerJobStatus: pj.JobStatus, Valid: true}
wr.LatestBuildStatus = pj.JobStatus
}

wr.LatestBuildTransition = build.Transition
Expand Down
52 changes: 26 additions & 26 deletions coderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion coderd/database/queries/workspaces.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ LEFT JOIN LATERAL (
provisioner_jobs.job_status
FROM
workspace_builds
LEFT JOIN
JOIN
Copy link
Member

Choose a reason for hiding this comment

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

Check this with @mafredri if it was not intentional 👍

Copy link
Member

@mafredri mafredri Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I would avoid making this change. I believe there's a window of time when there exists a new build but it's not assigned to a job. So the nullability is something that should be handled.

This logic change essentially results in returning the previous build until the it's been assigned to a job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if that's possible?

create table if not exists workspace_builds
(
    id                  uuid                                                                                not null
    ...
    workspace_id        uuid                                                                                not null
        references public.workspaces
            on delete cascade,
    ...
    job_id              uuid                                                                                not null
        unique
        references public.provisioner_jobs
            on delete cascade,
    ...
);

These keys are all non-nullable.
Is there something I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should say though that I'd rather not make this change if you in any way suspect this will cause issues.
The way I had it prior to #12762 (comment) worked fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Builds are inserted with their respective job in a single transaction. c.f. wsbuilder

Also, Danny's right that job_id is NOT NULL with a foreign key constraint, so even if we broke wsbuilder it still wouldn't be possible to have a build with no job.

Copy link
Member

Choose a reason for hiding this comment

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

Given the constraint, the change is perfectly fine 👍. I should’ve checked it myself before commenting. Sorry for the noise @dannykopping.

Copy link
Member

@mafredri mafredri Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The DB schema should probably be updated to include NOT NULL on the foreign keys, though. Otherwise NULL entries are not enforced by the DB.

Actually, where did you get the schema @dannykopping you referenced above? Doesn’t look the same as our dump which has NOT NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries @mafredri!

I just generated the DDL from an existing database in my IDE (GoLand). The above also has not null but you have to scroll over a bit to the right.

Copy link
Member

Choose a reason for hiding this comment

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

Haha, it was perfectly hidden by the code block size 😂.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know right??! Sorry about that lol

provisioner_jobs
ON
provisioner_jobs.id = workspace_builds.job_id
Expand Down
10 changes: 1 addition & 9 deletions coderd/prometheusmetrics/prometheusmetrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R

workspaceDetails.Reset()
for _, w := range ws {
// TODO: there may be a more elegant/idiomatic way to do this?
buildStatus := string(database.ProvisionerJobStatusUnknown)
if val, err := w.LatestBuildStatus.Value(); err == nil {
if status, ok := val.(string); ok {
buildStatus = status
}
}

workspaceDetails.WithLabelValues(buildStatus, w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1)
workspaceDetails.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1)
}
}

Expand Down