Skip to content

Commit fa89bc9

Browse files
committed
pkg/metrics_store: add error handling to WriteAll
Signed-off-by: Damien Grisonnet <[email protected]>
1 parent e355c48 commit fa89bc9

File tree

5 files changed

+36
-11
lines changed

5 files changed

+36
-11
lines changed

pkg/metrics_store/metrics_store_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ func TestObjectsSameNameDifferentNamespaces(t *testing.T) {
8282

8383
w := strings.Builder{}
8484
mw := NewMetricsWriter(ms)
85-
mw.WriteAll(&w)
85+
err := mw.WriteAll(&w)
86+
if err != nil {
87+
t.Fatalf("failed to write metrics: %v", err)
88+
}
8689
m := w.String()
8790

8891
for _, id := range serviceIDS {

pkg/metrics_store/metrics_writer.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ limitations under the License.
1616

1717
package metricsstore
1818

19-
import "io"
19+
import (
20+
"fmt"
21+
"io"
22+
)
2023

2124
// MetricsWriterList represent a list of MetricsWriter
2225
type MetricsWriterList []*MetricsWriter
@@ -43,9 +46,9 @@ func NewMetricsWriter(stores ...*MetricsStore) *MetricsWriter {
4346
//
4447
// WriteAll writes metrics so that the ones with the same name
4548
// are grouped together when written out.
46-
func (m MetricsWriter) WriteAll(w io.Writer) {
49+
func (m MetricsWriter) WriteAll(w io.Writer) error {
4750
if len(m.stores) == 0 {
48-
return
51+
return nil
4952
}
5053

5154
for _, s := range m.stores {
@@ -56,12 +59,19 @@ func (m MetricsWriter) WriteAll(w io.Writer) {
5659
}
5760

5861
for i, help := range m.stores[0].headers {
59-
w.Write([]byte(help))
60-
w.Write([]byte{'\n'})
62+
_, err := w.Write([]byte(help + "\n"))
63+
if err != nil {
64+
return fmt.Errorf("failed to write help text: %v", err)
65+
}
66+
6167
for _, s := range m.stores {
6268
for _, metricFamilies := range s.metrics {
63-
w.Write(metricFamilies[i])
69+
_, err := w.Write(metricFamilies[i])
70+
if err != nil {
71+
return fmt.Errorf("failed to write metrics family: %v", err)
72+
}
6473
}
6574
}
6675
}
76+
return nil
6777
}

pkg/metrics_store/metrics_writer_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ func TestWriteAllWithSingleStore(t *testing.T) {
8585

8686
multiNsWriter := metricsstore.NewMetricsWriter(store)
8787
w := strings.Builder{}
88-
multiNsWriter.WriteAll(&w)
88+
err := multiNsWriter.WriteAll(&w)
89+
if err != nil {
90+
t.Fatalf("failed to write metrics: %v", err)
91+
}
8992
result := w.String()
9093

9194
resultLines := strings.Split(strings.TrimRight(result, "\n"), "\n")
@@ -194,7 +197,10 @@ func TestWriteAllWithMultipleStores(t *testing.T) {
194197

195198
multiNsWriter := metricsstore.NewMetricsWriter(s1, s2)
196199
w := strings.Builder{}
197-
multiNsWriter.WriteAll(&w)
200+
err := multiNsWriter.WriteAll(&w)
201+
if err != nil {
202+
t.Fatalf("failed to write metrics: %v", err)
203+
}
198204
result := w.String()
199205

200206
resultLines := strings.Split(strings.TrimRight(result, "\n"), "\n")

pkg/metricshandler/metrics_handler.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,10 @@ func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
200200
}
201201

202202
for _, w := range m.metricsWriters {
203-
w.WriteAll(writer)
203+
err := w.WriteAll(writer)
204+
if err != nil {
205+
klog.ErrorS(err, "Failed to write metrics")
206+
}
204207
}
205208

206209
// In case we gzipped the response, we have to close the writer.

tests/lib/lib_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ func TestAsLibrary(t *testing.T) {
5656

5757
w := strings.Builder{}
5858
mw := metricsstore.NewMetricsWriter(c)
59-
mw.WriteAll(&w)
59+
err = mw.WriteAll(&w)
60+
if err != nil {
61+
t.Fatalf("failed to write metrics: %v", err)
62+
}
6063
m := w.String()
6164

6265
if !strings.Contains(m, service.ObjectMeta.Name) {

0 commit comments

Comments
 (0)