Skip to content

Commit 628e4ec

Browse files
mtojekmtojek
committed
Address PR comments
1 parent 12fcdab commit 628e4ec

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

‎coderd/prometheusmetrics/aggregator.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ const (
3030
defaultMetricsCleanupInterval = 2 * time.Minute
3131
)
3232

33+
var MetricLabelValueEncoder = strings.NewReplacer("|", "\\|", ",", "\\,", "=", "\\=")
34+
3335
type MetricsAggregator struct {
3436
store map[metricKey]annotatedMetric
3537

@@ -88,9 +90,13 @@ func (m1 metricKey) Equal(m2 metricKey) bool {
8890
func hashKey(req *updateRequest, m *agentproto.Stats_Metric) metricKey {
8991
var sb strings.Builder
9092
for _, label := range m.GetLabels() {
93+
if label.Value == "" {
94+
continue
95+
}
96+
9197
_, _ = sb.WriteString(label.Name)
9298
_ = sb.WriteByte('=')
93-
_, _ = sb.WriteString(label.Value)
99+
_, _ = sb.WriteString(MetricLabelValueEncoder.Replace(label.Value))
94100
_ = sb.WriteByte(',')
95101
}
96102
labels := strings.TrimRight(sb.String(), ",")
@@ -209,6 +215,7 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
209215
timer := prometheus.NewTimer(ma.updateHistogram)
210216
for _, m := range req.metrics {
211217
key := hashKey(&req, m)
218+
212219
if val, ok := ma.store[key]; ok {
213220
val.Stats_Metric.Value = m.Value
214221
val.expiryDate = req.timestamp.Add(ma.metricsCleanupInterval)

‎coderd/prometheusmetrics/aggregator_test.go

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,24 @@ func TestUpdateMetrics_MetricsDoNotExpire(t *testing.T) {
7171
{Name: "hello", Value: "world"},
7272
}},
7373
{Name: "d_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 6},
74+
{Name: "e_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 15, Labels: []*agentproto.Stats_Metric_Label{
75+
{Name: "foobar", Value: "Foo,ba=z"},
76+
{Name: "hello", Value: "wo,,r=d"},
77+
}},
78+
{Name: "f_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 6, Labels: []*agentproto.Stats_Metric_Label{
79+
{Name: "foobar", Value: "foobaz"},
80+
{Name: "empty", Value: ""},
81+
}},
82+
}
83+
84+
given3 := []*agentproto.Stats_Metric{
85+
{Name: "e_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 17, Labels: []*agentproto.Stats_Metric_Label{
86+
{Name: "cat", Value: "do,=g"},
87+
{Name: "hello", Value: "wo,,rld"},
88+
}},
89+
{Name: "f_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 8, Labels: []*agentproto.Stats_Metric_Label{
90+
{Name: "foobar", Value: "foobaz"},
91+
}},
7492
}
7593

7694
commonLabels := []*agentproto.Stats_Metric_Label{
@@ -99,11 +117,35 @@ func TestUpdateMetrics_MetricsDoNotExpire(t *testing.T) {
99117
}},
100118
{Name: "c_gauge_three", Type: agentproto.Stats_Metric_GAUGE, Value: 5, Labels: commonLabels},
101119
{Name: "d_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 6, Labels: commonLabels},
120+
{Name: "e_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 17, Labels: []*agentproto.Stats_Metric_Label{
121+
{Name: "agent_name", Value: testAgentName},
122+
{Name: "cat", Value: "do,=g"},
123+
{Name: "hello", Value: "wo,,rld"},
124+
{Name: "username", Value: testUsername},
125+
{Name: "workspace_name", Value: testWorkspaceName},
126+
{Name: "template_name", Value: testTemplateName},
127+
}},
128+
{Name: "e_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 15, Labels: []*agentproto.Stats_Metric_Label{
129+
{Name: "agent_name", Value: testAgentName},
130+
{Name: "foobar", Value: "Foo,ba=z"},
131+
{Name: "hello", Value: "wo,,r=d"},
132+
{Name: "username", Value: testUsername},
133+
{Name: "workspace_name", Value: testWorkspaceName},
134+
{Name: "template_name", Value: testTemplateName},
135+
}},
136+
{Name: "f_gauge_four", Type: agentproto.Stats_Metric_GAUGE, Value: 8, Labels: []*agentproto.Stats_Metric_Label{
137+
{Name: "agent_name", Value: testAgentName},
138+
{Name: "foobar", Value: "foobaz"},
139+
{Name: "username", Value: testUsername},
140+
{Name: "workspace_name", Value: testWorkspaceName},
141+
{Name: "template_name", Value: testTemplateName},
142+
}},
102143
}
103144

104145
// when
105146
metricsAggregator.Update(ctx, testLabels, given1)
106147
metricsAggregator.Update(ctx, testLabels, given2)
148+
metricsAggregator.Update(ctx, testLabels, given3)
107149

108150
// then
109151
require.Eventually(t, func() bool {
@@ -178,21 +220,24 @@ func prometheusMetricToString(t *testing.T, m prometheus.Metric) string {
178220
return dtoLabels[i].Name < dtoLabels[j].Name
179221
})
180222

181-
for i, dtoLabel := range dtoLabels {
223+
for _, dtoLabel := range dtoLabels {
224+
if dtoLabel.Value == "" {
225+
continue
226+
}
182227
_, _ = sb.WriteString(dtoLabel.Name)
183228
_ = sb.WriteByte('=')
184-
_, _ = sb.WriteString(dtoLabel.Value)
185-
186-
if i-1 != len(dtoLabels) {
187-
_ = sb.WriteByte(',')
188-
}
229+
_, _ = sb.WriteString(prometheusmetrics.MetricLabelValueEncoder.Replace(dtoLabel.Value))
189230
}
190-
return sb.String()
231+
return strings.TrimRight(sb.String(), ",")
191232
}
192233

193234
func asMetricAgentLabels(dtoLabels []*dto.LabelPair) []*agentproto.Stats_Metric_Label {
194235
metricLabels := make([]*agentproto.Stats_Metric_Label, 0, len(dtoLabels))
195236
for _, dtoLabel := range dtoLabels {
237+
if dtoLabel.GetValue() == "" {
238+
continue
239+
}
240+
196241
metricLabels = append(metricLabels, &agentproto.Stats_Metric_Label{
197242
Name: dtoLabel.GetName(),
198243
Value: dtoLabel.GetValue(),

0 commit comments

Comments
 (0)