Skip to content

feat: add metrics to workspace agent scripts #11132

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 19 commits into from
Dec 13, 2023
Merged

feat: add metrics to workspace agent scripts #11132

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2f978c3
wip: work on agent script metrics
Emyrk Dec 8, 2023
c567eab
push startup script metrics to agent
Emyrk Dec 11, 2023
5d39495
Add metrics pushing to prometheus
Emyrk Dec 11, 2023
07fe74a
make metric a normal prom metric exported, rather than a first class
Emyrk Dec 11, 2023
753f1a6
linting/fmt/naming
Emyrk Dec 11, 2023
aafd29e
add template_name label to workspace agent stats
Emyrk Dec 11, 2023
6c5a560
Fix prom tests
Emyrk Dec 11, 2023
ad3f47f
Update prom docs
Emyrk Dec 11, 2023
19428fa
seed template id in test
Emyrk Dec 11, 2023
8bfa9ad
fixup! seed template id in test
Emyrk Dec 12, 2023
044b942
PR feedback, _s -> seconds, start -> startup
Emyrk Dec 13, 2023
2db41b6
change string params to a struct
Emyrk Dec 13, 2023
dd32401
Change the name of the labels argument struct
Emyrk Dec 13, 2023
b4a3607
Clarify comment
Emyrk Dec 13, 2023
c5da250
Remove extra words
Emyrk Dec 13, 2023
eb1d173
measure workspace startup closer to script finish in lines of code
Emyrk Dec 13, 2023
e803436
Fix unit test call args
Emyrk Dec 13, 2023
99483fe
Make gen
Emyrk Dec 13, 2023
2f89b62
Formatting
Emyrk Dec 13, 2023
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
Fix prom tests
  • Loading branch information
@Emyrk
Emyrk committed Dec 11, 2023
commit 6c5a560883cf73da35185c18c4ddc76ac424e11b
12 changes: 12 additions & 0 deletions agent/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
"cdr.dev/slog"
"cdr.dev/slog/sloggers/sloghuman"
"cdr.dev/slog/sloggers/slogtest"

"github.com/coder/coder/v2/agent"
"github.com/coder/coder/v2/agent/agentproc"
"github.com/coder/coder/v2/agent/agentproc/agentproctest"
Expand Down Expand Up @@ -2235,6 +2236,17 @@ func TestAgent_Metrics_SSH(t *testing.T) {
Type: agentsdk.AgentMetricTypeCounter,
Value: 0,
},
{
Name: "coderd_agentstats_startup_script_s",
Type: agentsdk.AgentMetricTypeGauge,
Value: 0,
Labels: []agentsdk.AgentMetricLabel{
{
Name: "success",
Value: "true",
},
},
},
}

var actual []*promgo.MetricFamily
Expand Down
6 changes: 5 additions & 1 deletion coderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"golang.org/x/xerrors"

"cdr.dev/slog"

"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
Expand Down Expand Up @@ -1063,7 +1064,10 @@ func (s *MethodTestSuite) TestWorkspace() {
check.Args(ws.ID).Asserts(ws, rbac.ActionRead).Returns(b)
}))
s.Run("GetWorkspaceAgentByID", s.Subtest(func(db database.Store, check *expects) {
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
tpl := dbgen.Template(s.T(), db, database.Template{})
ws := dbgen.Workspace(s.T(), db, database.Workspace{
TemplateID: tpl.ID,
})
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
Expand Down
2 changes: 1 addition & 1 deletion coderd/database/models.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/querier.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.sql.go
View file
Open in desktop

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

17 changes: 13 additions & 4 deletions coderd/prometheusmetrics/aggregator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package prometheusmetrics_test

import (
"context"
"sort"
"sync/atomic"
"testing"
"time"
Expand All @@ -12,6 +13,7 @@ import (
"github.com/stretchr/testify/require"

"cdr.dev/slog/sloggers/slogtest"

"github.com/coder/coder/v2/coderd/prometheusmetrics"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/coder/v2/cryptorand"
Expand All @@ -22,6 +24,7 @@ const (
testWorkspaceName = "yogi-workspace"
testUsername = "yogi-bear"
testAgentName = "main-agent"
testTemplateName = "main-template"
)

func TestUpdateMetrics_MetricsDoNotExpire(t *testing.T) {
Expand Down Expand Up @@ -58,6 +61,7 @@ func TestUpdateMetrics_MetricsDoNotExpire(t *testing.T) {
{Name: "agent_name", Value: testAgentName},
{Name: "username", Value: testUsername},
{Name: "workspace_name", Value: testWorkspaceName},
{Name: "template_name", Value: testTemplateName},
}
expected := []agentsdk.AgentMetric{
{Name: "a_counter_one", Type: agentsdk.AgentMetricTypeCounter, Value: 1, Labels: commonLabels},
Expand All @@ -69,13 +73,14 @@ func TestUpdateMetrics_MetricsDoNotExpire(t *testing.T) {
{Name: "hello", Value: "world"},
{Name: "username", Value: testUsername},
{Name: "workspace_name", Value: testWorkspaceName},
{Name: "template_name", Value: testTemplateName},
}},
{Name: "d_gauge_four", Type: agentsdk.AgentMetricTypeGauge, Value: 6, Labels: commonLabels},
}

// when
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, given1)
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, given2)
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, testTemplateName, given1)
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, testTemplateName, given2)

// then
require.Eventually(t, func() bool {
Expand Down Expand Up @@ -119,6 +124,10 @@ func verifyCollectedMetrics(t *testing.T, expected []agentsdk.AgentMetric, actua
}

dtoLabels := asMetricAgentLabels(d.GetLabel())
// dto labels are sorted in alphabetical order.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should move it this routine to asMetricAgentLabels, so the function produces compliant DTO labels.

Copy link
Member Author

Choose a reason for hiding this comment

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

The expected labels that we pass in have to be sorted. The DTO ones are already sorted.
I figured it was fine in the verify function to sort them, otherwise maybe we would have to move it closer to where we define expected?

coder/coderd/prometheusmetrics/aggregator_test.go

Lines 66 to 79 in 8bfa9ad

expected := []agentsdk.AgentMetric{
{Name: "a_counter_one", Type: agentsdk.AgentMetricTypeCounter, Value: 1, Labels: commonLabels},
{Name: "b_counter_two", Type: agentsdk.AgentMetricTypeCounter, Value: 4, Labels: commonLabels},
{Name: "c_gauge_three", Type: agentsdk.AgentMetricTypeGauge, Value: 5, Labels: commonLabels},
{Name: "c_gauge_three", Type: agentsdk.AgentMetricTypeGauge, Value: 2, Labels: []agentsdk.AgentMetricLabel{
{Name: "agent_name", Value: testAgentName},
{Name: "foobar", Value: "Foobaz"},
{Name: "hello", Value: "world"},
{Name: "username", Value: testUsername},
{Name: "workspace_name", Value: testWorkspaceName},
{Name: "template_name", Value: testTemplateName},
}},
{Name: "d_gauge_four", Type: agentsdk.AgentMetricTypeGauge, Value: 6, Labels: commonLabels},
}

But this only needs to be sorted when comparing to dto labels.

Copy link
Member

Choose a reason for hiding this comment

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

I see now what you mean. I guess we can leave it as is, there won't be a huge gain if you refactor it 👍

sort.Slice(e.Labels, func(i, j int) bool {
return e.Labels[i].Name < e.Labels[j].Name
})
require.Equal(t, e.Labels, dtoLabels, d.String())
}
return true
Expand Down Expand Up @@ -154,7 +163,7 @@ func TestUpdateMetrics_MetricsExpire(t *testing.T) {
}

// when
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, given)
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, testTemplateName, given)

time.Sleep(time.Millisecond * 10) // Ensure that metric is expired

Expand Down Expand Up @@ -220,7 +229,7 @@ func Benchmark_MetricsAggregator_Run(b *testing.B) {
b.Logf("N=%d sending %d metrics", b.N, numMetrics)
var nGot atomic.Int64
b.StartTimer()
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, metrics)
metricsAggregator.Update(ctx, testUsername, testWorkspaceName, testAgentName, testTemplateName, metrics)
for i := 0; i < numMetrics; i++ {
select {
case <-ctx.Done():
Expand Down