Skip to content

Commit 905aae2

Browse files
committed
Integrate review comments
* String typed enum
1 parent adcec1e commit 905aae2

File tree

2 files changed

+60
-56
lines changed

2 files changed

+60
-56
lines changed

prometheus/promhttp/http.go

+47-50
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,14 @@ const (
5555
processStartTimeHeader = "Process-Start-Time-Unix"
5656
)
5757

58-
type Compression int
58+
type Compression string
5959

6060
const (
61-
Identity Compression = iota
62-
Gzip
63-
Zstd
61+
Identity Compression = "identity"
62+
Gzip Compression = "gzip"
63+
Zstd Compression ="zstd"
6464
)
6565

66-
var compressions = [...]string{
67-
"identity",
68-
"gzip",
69-
"zstd",
70-
}
71-
72-
func (c Compression) String() string {
73-
return compressions[c]
74-
}
75-
7666
var defaultCompressionFormats = []Compression{Identity, Gzip, Zstd}
7767

7868
var gzipPool = sync.Pool{
@@ -143,6 +133,19 @@ func HandlerForTransactional(reg prometheus.TransactionalGatherer, opts HandlerO
143133
}
144134
}
145135

136+
137+
// Select all supported compression formats
138+
var compressions []string
139+
if !opts.DisableCompression {
140+
offers := defaultCompressionFormats
141+
if len(opts.OfferedCompressions) > 0 {
142+
offers = opts.OfferedCompressions
143+
}
144+
for _, comp := range offers {
145+
compressions = append(compressions, string(comp))
146+
}
147+
}
148+
146149
h := http.HandlerFunc(func(rsp http.ResponseWriter, req *http.Request) {
147150
if !opts.ProcessStartTime.IsZero() {
148151
rsp.Header().Set(processStartTimeHeader, strconv.FormatInt(opts.ProcessStartTime.Unix(), 10))
@@ -188,7 +191,7 @@ func HandlerForTransactional(reg prometheus.TransactionalGatherer, opts HandlerO
188191
}
189192
rsp.Header().Set(contentTypeHeader, string(contentType))
190193

191-
w, err := GetWriter(req, rsp, opts.DisableCompression, opts.OfferedCompressions)
194+
w, err := GetWriter(req, rsp, opts.DisableCompression, compressions)
192195
if err != nil {
193196
if opts.ErrorLog != nil {
194197
opts.ErrorLog.Println("error getting writer", err)
@@ -419,48 +422,42 @@ func httpError(rsp http.ResponseWriter, err error) {
419422
)
420423
}
421424

422-
func GetWriter(r *http.Request, rsp http.ResponseWriter, disableCompression bool, offeredCompressions []Compression) (io.Writer, error) {
425+
func GetWriter(r *http.Request, rsp http.ResponseWriter, disableCompression bool, compressions []string) (io.Writer, error) {
423426
w := io.Writer(rsp)
424427
rsp.Header().Set(contentEncodingHeader, "identity")
425-
if !disableCompression {
426-
offers := defaultCompressionFormats
427-
if len(offeredCompressions) > 0 {
428-
offers = offeredCompressions
429-
}
430-
var compressions []string
431-
for _, comp := range offers {
432-
compressions = append(compressions, comp.String())
428+
if disableCompression {
429+
return w, nil
430+
}
431+
432+
// TODO(mrueg): Replace internal/github.com/gddo once https://github.com/golang/go/issues/19307 is implemented.
433+
compression := httputil.NegotiateContentEncoding(r, compressions)
434+
switch compression {
435+
case "zstd":
436+
rsp.Header().Set(contentEncodingHeader, "zstd")
437+
// TODO(mrueg): Replace klauspost/compress with stdlib implementation once https://github.com/golang/go/issues/62513 is implemented.
438+
z, err := zstd.NewWriter(rsp, zstd.WithEncoderLevel(zstd.SpeedFastest))
439+
if err != nil {
440+
return nil, err
433441
}
434-
// TODO(mrueg): Replace internal/github.com/gddo once https://github.com/golang/go/issues/19307 is implemented.
435-
compression := httputil.NegotiateContentEncoding(r, compressions)
436-
switch compression {
437-
case "zstd":
438-
rsp.Header().Set(contentEncodingHeader, "zstd")
439-
// TODO(mrueg): Replace klauspost/compress with stdlib implementation once https://github.com/golang/go/issues/62513 is implemented.
440-
z, err := zstd.NewWriter(rsp, zstd.WithEncoderLevel(zstd.SpeedFastest))
441-
if err != nil {
442-
return nil, err
443-
}
444442

445-
z.Reset(w)
446-
defer z.Close()
443+
z.Reset(w)
444+
defer z.Close()
447445

448-
w = z
449-
case "gzip":
450-
rsp.Header().Set(contentEncodingHeader, "gzip")
451-
gz := gzipPool.Get().(*gzip.Writer)
452-
defer gzipPool.Put(gz)
446+
w = z
447+
case "gzip":
448+
rsp.Header().Set(contentEncodingHeader, "gzip")
449+
gz := gzipPool.Get().(*gzip.Writer)
450+
defer gzipPool.Put(gz)
453451

454-
gz.Reset(w)
455-
defer gz.Close()
452+
gz.Reset(w)
453+
defer gz.Close()
456454

457-
w = gz
458-
case "identity":
459-
// This means the content is not compressed.
460-
default:
461-
// The content encoding was not implemented yet.
462-
return w, fmt.Errorf("content compression format not recognized: %s. Valid formats are: %s", compression, defaultCompressionFormats)
463-
}
455+
w = gz
456+
case "identity":
457+
// This means the content is not compressed.
458+
default:
459+
// The content encoding was not implemented yet.
460+
return w, fmt.Errorf("content compression format not recognized: %s. Valid formats are: %s", compression, defaultCompressionFormats)
464461
}
465462
return w, nil
466463
}

prometheus/promhttp/http_test.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -333,50 +333,57 @@ func TestHandlerTimeout(t *testing.T) {
333333
}
334334

335335
func TestGetWriter(t *testing.T) {
336+
337+
var defaultCompressions []string
338+
339+
for _, comp := range defaultCompressionFormats {
340+
defaultCompressions = append(defaultCompressions,string(comp))
341+
}
342+
336343
testCases := []struct {
337344
name string
338345
disableCompression bool
339-
offeredCompressions []Compression
346+
offeredCompressions []string
340347
acceptEncoding string
341348
expectedCompression string
342349
err error
343350
}{
344351
{
345352
name: "test without compression enabled",
346353
disableCompression: true,
347-
offeredCompressions: defaultCompressionFormats,
354+
offeredCompressions: defaultCompressions,
348355
acceptEncoding: "",
349356
expectedCompression: "identity",
350357
err: nil,
351358
},
352359
{
353360
name: "test with compression enabled with empty accept-encoding header",
354361
disableCompression: false,
355-
offeredCompressions: defaultCompressionFormats,
362+
offeredCompressions: defaultCompressions,
356363
acceptEncoding: "",
357364
expectedCompression: "identity",
358365
err: nil,
359366
},
360367
{
361368
name: "test with gzip compression requested",
362369
disableCompression: false,
363-
offeredCompressions: defaultCompressionFormats,
370+
offeredCompressions: defaultCompressions,
364371
acceptEncoding: "gzip",
365372
expectedCompression: "gzip",
366373
err: nil,
367374
},
368375
{
369376
name: "test with gzip, zstd compression requested",
370377
disableCompression: false,
371-
offeredCompressions: defaultCompressionFormats,
378+
offeredCompressions: defaultCompressions,
372379
acceptEncoding: "gzip,zstd",
373380
expectedCompression: "gzip",
374381
err: nil,
375382
},
376383
{
377384
name: "test with zstd, gzip compression requested",
378385
disableCompression: false,
379-
offeredCompressions: defaultCompressionFormats,
386+
offeredCompressions: defaultCompressions,
380387
acceptEncoding: "zstd,gzip",
381388
expectedCompression: "gzip",
382389
err: nil,

0 commit comments

Comments
 (0)