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

Commitb6e86a2

Browse files
committed
Refactoring
Signed-off-by: Danny Kopping <danny@coder.com>
1 parentfcbffe0 commitb6e86a2

File tree

2 files changed

+122
-41
lines changed

2 files changed

+122
-41
lines changed

‎coderd/prometheusmetrics/aggregator.go

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/coder/coder/v2/coderd/agentmetrics"
1111
"github.com/prometheus/client_golang/prometheus"
12+
"github.com/prometheus/common/model"
1213
"golang.org/x/xerrors"
1314

1415
"cdr.dev/slog"
@@ -107,8 +108,9 @@ var _ prometheus.Collector = new(MetricsAggregator)
107108

108109
func (am*annotatedMetric)asPrometheus() (prometheus.Metric,error) {
109110
var (
110-
baseLabelNames[]string=am.aggregateByLabels
111+
baseLabelNames=am.aggregateByLabels
111112
baseLabelValues []string
113+
extraLabels=am.Labels
112114
)
113115

114116
for_,label:=rangeam.aggregateByLabels {
@@ -120,19 +122,17 @@ func (am *annotatedMetric) asPrometheus() (prometheus.Metric, error) {
120122
baseLabelValues=append(baseLabelValues,val)
121123
}
122124

123-
labels:=make([]string,0,len(baseLabelNames)+len(am.Labels))
124-
labelValues:=make([]string,0,len(baseLabelNames)+len(am.Labels))
125+
labels:=make([]string,0,len(baseLabelNames)+len(extraLabels))
126+
labelValues:=make([]string,0,len(baseLabelNames)+len(extraLabels))
125127

126128
labels=append(labels,baseLabelNames...)
127129
labelValues=append(labelValues,baseLabelValues...)
128130

129-
for_,l:=rangeam.Labels {
131+
for_,l:=rangeextraLabels {
130132
labels=append(labels,l.Name)
131133
labelValues=append(labelValues,l.Value)
132134
}
133135

134-
//fmt.Printf(">>>>[%s] [%s] %s [%q] [%q]: %v\n", time.Now().Format(time.RFC3339Nano), am.Type, am.Name, labels, labelValues, am.Value)
135-
136136
desc:=prometheus.NewDesc(am.Name,metricHelpForAgent,labels,nil)
137137
valueType,err:=asPrometheusValueType(am.Type)
138138
iferr!=nil {
@@ -237,50 +237,56 @@ func NewMetricsAggregator(logger slog.Logger, registerer prometheus.Registerer,
237237
},nil
238238
}
239239

240-
typeMetricAggregatorstruct {
240+
// labelAggregator is used to control cardinality of collected Prometheus metrics by pre-aggregating series based on given labels.
241+
typelabelAggregatorstruct {
241242
aggregationsmap[string]float64
242243
metricsmap[string]annotatedMetric
243244
}
244245

245-
funcNewMetricAggregator(sizeint)*MetricAggregator {
246-
return&MetricAggregator{
246+
funcnewLabelAggregator(sizeint)*labelAggregator {
247+
return&labelAggregator{
247248
aggregations:make(map[string]float64,size),
248249
metrics:make(map[string]annotatedMetric,size),
249250
}
250251
}
251252

252-
func (a*MetricAggregator)Aggregate(amannotatedMetric,labels []string)error {
253-
// if we already have an entry for this key, don't clone this am afresh - rather use the existing one
254-
// this will be a bit more memory efficient
255-
// ...do this after unit-test is written
256-
257-
clone:=am.clone()
258-
259-
fields:=make(map[string]string,len(labels))
253+
func (a*labelAggregator)aggregate(amannotatedMetric,labels []string)error {
254+
// Use a LabelSet because it can give deterministic fingerprints of label combinations regardless of map ordering.
255+
labelSet:=make(model.LabelSet,len(labels))
260256
labelValues:=make([]string,0,len(labels))
261257

262258
for_,label:=rangelabels {
263-
val,err:=clone.getFieldByLabel(label)
259+
val,err:=am.getFieldByLabel(label)
264260
iferr!=nil {
265261
returnerr
266262
}
267263

268-
fields[label]=val
264+
labelSet[model.LabelName(label)]=model.LabelValue(val)
269265
labelValues=append(labelValues,val)
270266
}
271267

272-
key:=fmt.Sprintf("%s:%v",clone.Stats_Metric.Name,fields)
268+
// Memoize based on the metric name & the unique combination of labels.
269+
key:=fmt.Sprintf("%s:%v",am.Stats_Metric.Name,labelSet.FastFingerprint())
270+
271+
// Aggregate the value based on the key.
272+
a.aggregations[key]+=am.Value
273+
274+
metric,found:=a.metrics[key]
275+
if!found {
276+
// Take a copy of the given annotatedMetric because it may be manipulated later and contains pointers.
277+
metric=am.clone()
278+
}
273279

274-
clone.aggregateByLabels=labels
275-
a.aggregations[key]+=clone.Value
280+
// Store the metric.
281+
metric.aggregateByLabels=labels
282+
metric.Value=a.aggregations[key]
276283

277-
clone.Value=a.aggregations[key]
278-
a.metrics[key]=clone
284+
a.metrics[key]=metric
279285

280286
returnnil
281287
}
282288

283-
func (a*MetricAggregator)asMetrics() (out []annotatedMetric) {
289+
func (a*labelAggregator)toMetrics() (out []annotatedMetric) {
284290
for_,am:=rangea.metrics {
285291
out=append(out,am)
286292
}
@@ -331,24 +337,25 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
331337

332338
// If custom aggregation labels have not been chosen, generate Prometheus metrics without any pre-aggregation.
333339
// This results in higher cardinality, but may be desirable in larger deployments.
340+
// Default behaviour.
334341
iflen(ma.aggregateByLabels)==0 {
335342
for_,m:=rangema.store {
336-
// Aggregate byhigh cardinality labels.
337-
m.aggregateByLabels=agentMetricsLabels
343+
// Aggregate byall available metrics.
344+
m.aggregateByLabels=defaultAgentMetricsLabels
338345
input=append(input,m)
339346
}
340347
}else {
341348
// However, if custom aggregations have been chosen, we need to aggregate the values from the annotated
342349
// metrics because we cannot register multiple metric series with the same labels.
343-
aggregator:=NewMetricAggregator(len(ma.store)*len(ma.aggregateByLabels))
350+
la:=newLabelAggregator(len(ma.store))
344351

345352
for_,m:=rangema.store {
346-
iferr:=aggregator.Aggregate(m,ma.aggregateByLabels);err!=nil {
353+
iferr:=la.aggregate(m,ma.aggregateByLabels);err!=nil {
347354
ma.log.Error(ctx,"can't aggregate labels",slog.F("labels",strings.Join(ma.aggregateByLabels,",")),slog.Error(err))
348355
}
349356
}
350357

351-
input=aggregator.asMetrics()
358+
input=la.toMetrics()
352359
}
353360

354361
for_,m:=rangeinput {
@@ -395,7 +402,7 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
395402
func (*MetricsAggregator)Describe(_chan<-*prometheus.Desc) {
396403
}
397404

398-
varagentMetricsLabels= []string{agentmetrics.UsernameLabel,agentmetrics.WorkspaceNameLabel,agentmetrics.AgentNameLabel,agentmetrics.TemplateNameLabel}
405+
vardefaultAgentMetricsLabels= []string{agentmetrics.UsernameLabel,agentmetrics.WorkspaceNameLabel,agentmetrics.AgentNameLabel,agentmetrics.TemplateNameLabel}
399406

400407
// AgentMetricLabels are the labels used to decorate an agent's metrics.
401408
// This list should match the list of labels in agentMetricsLabels.

‎coderd/prometheusmetrics/aggregator_test.go

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@ import (
88
"testing"
99
"time"
1010

11+
"cdr.dev/slog/sloggers/slogtest"
1112
"github.com/coder/coder/v2/coderd/agentmetrics"
1213
"github.com/prometheus/client_golang/prometheus"
1314
dto"github.com/prometheus/client_model/go"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
1617

17-
"cdr.dev/slog/sloggers/slogtest"
18-
1918
agentproto"github.com/coder/coder/v2/agent/proto"
2019
"github.com/coder/coder/v2/coderd/prometheusmetrics"
2120
"github.com/coder/coder/v2/cryptorand"
@@ -324,23 +323,19 @@ func TestLabelsAggregation(t *testing.T) {
324323
{
325324
labels:testLabels,
326325
metrics: []*agentproto.Stats_Metric{
327-
{Name:"no_extra_labels",Type:agentproto.Stats_Metric_COUNTER,Value:1},
326+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1},
328327
},
329328
},
330329
{
331330
labels:testLabels,
332331
metrics: []*agentproto.Stats_Metric{
333-
{Name:"extra_label",Type:agentproto.Stats_Metric_COUNTER,Value:27,Labels: []*agentproto.Stats_Metric_Label{
334-
{Name:"lizz",Value:"rizz"},
335-
}},
332+
{Name:"active_conns",Type:agentproto.Stats_Metric_GAUGE,Value:4},
336333
},
337334
},
338335
},
339336
expected: []*agentproto.Stats_Metric{
340-
{Name:"no_extra_labels",Type:agentproto.Stats_Metric_COUNTER,Value:1,Labels:commonLabels},
341-
{Name:"extra_label",Type:agentproto.Stats_Metric_COUNTER,Value:27,Labels:append([]*agentproto.Stats_Metric_Label{
342-
{Name:"lizz",Value:"rizz"},
343-
},commonLabels...)},
337+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1,Labels:commonLabels},
338+
{Name:"active_conns",Type:agentproto.Stats_Metric_GAUGE,Value:4,Labels:commonLabels},
344339
},
345340
},
346341
{
@@ -484,6 +479,85 @@ func TestLabelsAggregation(t *testing.T) {
484479
}},
485480
},
486481
},
482+
{
483+
name:"extra labels are retained, even with label aggregations",
484+
aggregateOn: []string{agentmetrics.UsernameLabel},
485+
given: []statCollection{
486+
{
487+
labels:testLabels,
488+
metrics: []*agentproto.Stats_Metric{
489+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1},
490+
},
491+
},
492+
{
493+
labels:testLabels,
494+
metrics: []*agentproto.Stats_Metric{
495+
{Name:"extra_label",Type:agentproto.Stats_Metric_COUNTER,Value:27,Labels: []*agentproto.Stats_Metric_Label{
496+
{Name:"lizz",Value:"rizz"},
497+
}},
498+
},
499+
},
500+
},
501+
expected: []*agentproto.Stats_Metric{
502+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1,Labels: []*agentproto.Stats_Metric_Label{
503+
{Name:agentmetrics.UsernameLabel,Value:testUsername},
504+
}},
505+
{Name:"extra_label",Type:agentproto.Stats_Metric_COUNTER,Value:27,Labels: []*agentproto.Stats_Metric_Label{
506+
{Name:"lizz",Value:"rizz"},
507+
{Name:agentmetrics.UsernameLabel,Value:testUsername},
508+
}},
509+
},
510+
},
511+
{
512+
// Both counters and gauges should have all their values summed to produce the correct output.
513+
name:"counters & gauges behave identically",
514+
aggregateOn: []string{agentmetrics.TemplateNameLabel},
515+
given: []statCollection{
516+
{
517+
labels: prometheusmetrics.AgentMetricLabels{
518+
Username:"username1",
519+
TemplateName:"template1",
520+
},
521+
metrics: []*agentproto.Stats_Metric{
522+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1},
523+
{Name:"active_conns",Type:agentproto.Stats_Metric_GAUGE,Value:3},
524+
},
525+
},
526+
{
527+
labels: prometheusmetrics.AgentMetricLabels{
528+
Username:"username2",
529+
TemplateName:"template1",
530+
},
531+
metrics: []*agentproto.Stats_Metric{
532+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:2},
533+
{Name:"active_conns",Type:agentproto.Stats_Metric_GAUGE,Value:4},
534+
},
535+
},
536+
},
537+
expected: []*agentproto.Stats_Metric{
538+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:3,Labels: []*agentproto.Stats_Metric_Label{
539+
{Name:agentmetrics.TemplateNameLabel,Value:"template1"},
540+
}},
541+
{Name:"active_conns",Type:agentproto.Stats_Metric_GAUGE,Value:7,Labels: []*agentproto.Stats_Metric_Label{
542+
{Name:agentmetrics.TemplateNameLabel,Value:"template1"},
543+
}},
544+
},
545+
},
546+
{
547+
// Scenario: validation fails and an invalid label is selected for aggregation.
548+
name:"invalid label aggregation",
549+
aggregateOn: []string{"nonsense"},
550+
given: []statCollection{
551+
{
552+
labels:testLabels,
553+
metrics: []*agentproto.Stats_Metric{
554+
{Name:"user_counter",Type:agentproto.Stats_Metric_COUNTER,Value:1},
555+
},
556+
},
557+
},
558+
// Nothing will be returned.
559+
expected: []*agentproto.Stats_Metric{},
560+
},
487561
}
488562

489563
for_,tc:=rangetests {

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp