Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: make agent stats' cardinality configurable#12468

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

dannykopping
Copy link
Contributor

Implements#12221

When stats from agents are collected, they are aggregated by 4 dimensions:agent_name,template_name,workspace_name, andusername. This can result in some very high cardinality metrics being scraped by Prometheus in large environments.

This PR adds the ability to tune which labels are include in this aggregation, therefore reducing the cardinality.

For example:

agent_sessions_total{agent_name="main",magic_type="ssh",pty="yes",template_name="docker",username="danny",workspace_name="test1"} 6agent_sessions_total{agent_name="main",magic_type="ssh",pty="yes",template_name="docker",username="danny",workspace_name="test2"} 5

With hundreds of active workspaces, each having a unique name, the cardinality may be unacceptable. In this case the operator may choose to configureCODER_PROMETHEUS_AGGREGATE_AGENT_STATS_BY=username to rather aggregate by user, summing all the metrics' values and only producing a single metric series:

agent_sessions_total{magic_type="ssh",pty="yes",username="danny"} 11

Multiple labels can be provided, e.g.CODER_PROMETHEUS_AGGREGATE_AGENT_STATS_BY=username,agent_name

agent_sessions_total{agent_name="main",magic_type="ssh",pty="yes",username="danny"} 11

The current behaviour remains the default; if no value is passed toCODER_PROMETHEUS_AGGREGATE_AGENT_STATS_BY there will be no change.

Note to reviewer: I made a sweeping refactor to define the label names in a single location instead of duplicating them, which may make the PR larger than it seems.

@dannykoppingdannykopping changed the titlefeat: configurable agent stats cardinalityfeat: make agent stats cardinality configurableMar 8, 2024
@dannykoppingdannykopping changed the titlefeat: make agent stats cardinality configurablefeat: make agent stats' cardinality configurableMar 8, 2024
Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

Noticed some minor nits, but in general look good, nice work!

"golang.org/x/xerrors"

"github.com/coder/coder/v2/coderd/agentmetrics"
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately our formatter doesn't handle merging import groups and leaves things in a messy state (depending on what program injected them). 😔

If you notice these, please feel free to fix, but the standard is we try our best but sometimes these slip through, so don't worry too much.

return nil
}

acceptable := make(map[string]any, len(AcceptedMetricAggregationLabels))
Copy link
Member

Choose a reason for hiding this comment

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

I think usingmap[string]bool would be preferable/clearer here, also simplifies the map lookup later. Typically I'd use eitherbool or if we're looking for space savings, I'd use the zero struct (map[string]struct{}).

dannykoppingand others added8 commitsMarch 8, 2024 14:33
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
…ebug worksSigned-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
@dannykoppingdannykoppingforce-pushed thedk/configurable-cardinality branch from5ba04c5 to3e569ffCompareMarch 8, 2024 12:37
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
@dannykoppingdannykoppingforce-pushed thedk/configurable-cardinality branch from881cd6a to9b16a3bCompareMarch 11, 2024 08:48
Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

LGTM! I know tests aren't passing now but looks unrelated so I don't need to re-review, nice work!

Copy link
Member

@mtojekmtojek left a comment

Choose a reason for hiding this comment

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

Nice!

I know tests aren't passing now but looks unrelated so I don't need to re-review, nice work!

Agree, feel free tot.Skip them for now.

@dannykoppingdannykoppingenabled auto-merge (squash)March 11, 2024 13:43
@dannykoppingdannykopping merged commit21d1873 intocoder:mainMar 11, 2024
@dannykoppingdannykopping deleted the dk/configurable-cardinality branchMarch 11, 2024 14:04
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsMar 11, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@mafredrimafredrimafredri approved these changes

@johnstcnjohnstcnjohnstcn approved these changes

@mtojekmtojekmtojek approved these changes

Assignees

@dannykoppingdannykopping

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@dannykopping@mafredri@johnstcn@mtojek

[8]ページ先頭

©2009-2025 Movatter.jp