Skip to content

Commit 1f4f78b

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

File tree

2 files changed

+59
-56
lines changed

2 files changed

+59
-56
lines changed

prometheus/promhttp/http.go

+46-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,18 @@ func HandlerForTransactional(reg prometheus.TransactionalGatherer, opts HandlerO
143133
}
144134
}
145135

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

191-
w, err := GetWriter(req, rsp, opts.DisableCompression, opts.OfferedCompressions)
193+
w, err := GetWriter(req, rsp, opts.DisableCompression, compressions)
192194
if err != nil {
193195
if opts.ErrorLog != nil {
194196
opts.ErrorLog.Println("error getting writer", err)
@@ -419,48 +421,42 @@ func httpError(rsp http.ResponseWriter, err error) {
419421
)
420422
}
421423

422-
func GetWriter(r *http.Request, rsp http.ResponseWriter, disableCompression bool, offeredCompressions []Compression) (io.Writer, error) {
424+
func GetWriter(r *http.Request, rsp http.ResponseWriter, disableCompression bool, compressions []string) (io.Writer, error) {
423425
w := io.Writer(rsp)
424426
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())
427+
if disableCompression {
428+
return w, nil
429+
}
430+
431+
// TODO(mrueg): Replace internal/github.com/gddo once https://github.com/golang/go/issues/19307 is implemented.
432+
compression := httputil.NegotiateContentEncoding(r, compressions)
433+
switch compression {
434+
case "zstd":
435+
rsp.Header().Set(contentEncodingHeader, "zstd")
436+
// TODO(mrueg): Replace klauspost/compress with stdlib implementation once https://github.com/golang/go/issues/62513 is implemented.
437+
z, err := zstd.NewWriter(rsp, zstd.WithEncoderLevel(zstd.SpeedFastest))
438+
if err != nil {
439+
return nil, err
433440
}
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-
}
444441

445-
z.Reset(w)
446-
defer z.Close()
442+
z.Reset(w)
443+
defer z.Close()
447444

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

454-
gz.Reset(w)
455-
defer gz.Close()
451+
gz.Reset(w)
452+
defer gz.Close()
456453

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-
}
454+
w = gz
455+
case "identity":
456+
// This means the content is not compressed.
457+
default:
458+
// The content encoding was not implemented yet.
459+
return w, fmt.Errorf("content compression format not recognized: %s. Valid formats are: %s", compression, defaultCompressionFormats)
464460
}
465461
return w, nil
466462
}

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)