Repository: prometheus/common
Pull request #576 (2024-02-14 17:17:00)

Unexport Format strings
With the addition of the escaping term, format strings now have many permutations and doing direct string comparisons of formats is not reliable. Instead, users should call FormatType and compare the result against the possible enum values.

I am making this change after breaking a test in prometheus that relied on string matching of formats.


Patch:
diff --git a/expfmt/decode.go b/expfmt/decode.go
index 3597bfd..b2b89b0 100644
--- a/expfmt/decode.go
+++ b/expfmt/decode.go
@@ -45,7 +45,7 @@ func ResponseFormat(h http.Header) Format {
 
 	mediatype, params, err := mime.ParseMediaType(ct)
 	if err != nil {
-		return FmtUnknown
+		return fmtUnknown
 	}
 
 	const textType = "text/plain"
@@ -53,21 +53,21 @@ func ResponseFormat(h http.Header) Format {
 	switch mediatype {
 	case ProtoType:
 		if p, ok := params["proto"]; ok && p != ProtoProtocol {
-			return FmtUnknown
+			return fmtUnknown
 		}
 		if e, ok := params["encoding"]; ok && e != "delimited" {
-			return FmtUnknown
+			return fmtUnknown
 		}
-		return FmtProtoDelim
+		return fmtProtoDelim
 
 	case textType:
 		if v, ok := params["version"]; ok && v != TextVersion {
-			return FmtUnknown
+			return fmtUnknown
 		}
-		return FmtText
+		return fmtText
 	}
 
-	return FmtUnknown
+	return fmtUnknown
 }
 
 // NewDecoder returns a new decoder based on the given input format.
diff --git a/expfmt/encode.go b/expfmt/encode.go
index 97ee673..8fd8061 100644
--- a/expfmt/encode.go
+++ b/expfmt/encode.go
@@ -76,18 +76,18 @@ func Negotiate(h http.Header) Format {
 		if ac.Type+"/"+ac.SubType == ProtoType && ac.Params["proto"] == ProtoProtocol {
 			switch ac.Params["encoding"] {
 			case "delimited":
-				return FmtProtoDelim + escapingScheme
+				return fmtProtoDelim + escapingScheme
 			case "text":
-				return FmtProtoText + escapingScheme
+				return fmtProtoText + escapingScheme
 			case "compact-text":
-				return FmtProtoCompact + escapingScheme
+				return fmtProtoCompact + escapingScheme
 			}
 		}
 		if ac.Type == "text" && ac.SubType == "plain" && (ver == TextVersion || ver == "") {
-			return FmtText + escapingScheme
+			return fmtText + escapingScheme
 		}
 	}
-	return FmtText + escapingScheme
+	return fmtText + escapingScheme
 }
 
 // NegotiateIncludingOpenMetrics works like Negotiate but includes
@@ -109,26 +109,26 @@ func NegotiateIncludingOpenMetrics(h http.Header) Format {
 		if ac.Type+"/"+ac.SubType == ProtoType && ac.Params["proto"] == ProtoProtocol {
 			switch ac.Params["encoding"] {
 			case "delimited":
-				return FmtProtoDelim + escapingScheme
+				return fmtProtoDelim + escapingScheme
 			case "text":
-				return FmtProtoText + escapingScheme
+				return fmtProtoText + escapingScheme
 			case "compact-text":
-				return FmtProtoCompact + escapingScheme
+				return fmtProtoCompact + escapingScheme
 			}
 		}
 		if ac.Type == "text" && ac.SubType == "plain" && (ver == TextVersion || ver == "") {
-			return FmtText + escapingScheme
+			return fmtText + escapingScheme
 		}
 		if ac.Type+"/"+ac.SubType == OpenMetricsType && (ver == OpenMetricsVersion_0_0_1 || ver == OpenMetricsVersion_1_0_0 || ver == "") {
 			switch ver {
 			case OpenMetricsVersion_1_0_0:
-				return FmtOpenMetrics_1_0_0 + escapingScheme
+				return fmtOpenMetrics_1_0_0 + escapingScheme
 			default:
-				return FmtOpenMetrics_0_0_1 + escapingScheme
+				return fmtOpenMetrics_0_0_1 + escapingScheme
 			}
 		}
 	}
-	return FmtText + escapingScheme
+	return fmtText + escapingScheme
 }
 
 // NewEncoder returns a new encoder based on content type negotiation. All
diff --git a/expfmt/expfmt.go b/expfmt/expfmt.go
index f9b6f70..6fc9555 100644
--- a/expfmt/expfmt.go
+++ b/expfmt/expfmt.go
@@ -34,20 +34,21 @@ const (
 	TextVersion              = "0.0.4"
 	ProtoType                = `application/vnd.google.protobuf`
 	ProtoProtocol            = `io.prometheus.client.MetricFamily`
-	ProtoFmt                 = ProtoType + "; proto=" + ProtoProtocol + ";"
+	protoFmt                 = ProtoType + "; proto=" + ProtoProtocol + ";"
 	OpenMetricsType          = `application/openmetrics-text`
 	OpenMetricsVersion_0_0_1 = "0.0.1"
 	OpenMetricsVersion_1_0_0 = "1.0.0"
 
-	// The Content-Type values for the different wire protocols. Do not do direct
-	// comparisons to these constants, instead use the comparison functions.
-	FmtUnknown           Format = `<unknown>`
-	FmtText              Format = `text/plain; version=` + TextVersion + `; charset=utf-8`
-	FmtProtoDelim        Format = ProtoFmt + ` encoding=delimited`
-	FmtProtoText         Format = ProtoFmt + ` encoding=text`
-	FmtProtoCompact      Format = ProtoFmt + ` encoding=compact-text`
-	FmtOpenMetrics_1_0_0 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_1_0_0 + `; charset=utf-8`
-	FmtOpenMetrics_0_0_1 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_0_0_1 + `; charset=utf-8`
+	// The Content-Type values for the different wire protocols. Note that these
+	// values are now unexported. If code was relying on comparisons to these
+	// constants, instead use FormatType().
+	fmtUnknown           Format = `<unknown>`
+	fmtText              Format = `text/plain; version=` + TextVersion + `; charset=utf-8`
+	fmtProtoDelim        Format = protoFmt + ` encoding=delimited`
+	fmtProtoText         Format = protoFmt + ` encoding=text`
+	fmtProtoCompact      Format = protoFmt + ` encoding=compact-text`
+	fmtOpenMetrics_1_0_0 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_1_0_0 + `; charset=utf-8`
+	fmtOpenMetrics_0_0_1 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_0_0_1 + `; charset=utf-8`
 )
 
 const (
@@ -70,6 +71,26 @@ const (
 	TypeOpenMetrics
 )
 
+// NewFormat generates a new Format from the type provided. Mostly used for
+// tests, most Formats should be generated as part of content negotiation in
+// encode.go.
+func NewFormat(t FormatType) Format {
+	switch t {
+	case TypeProtoCompact:
+		return fmtProtoCompact
+	case TypeProtoDelim:
+		return fmtProtoDelim
+	case TypeProtoText:
+		return fmtProtoText
+	case TypeTextPlain:
+		return fmtText
+	case TypeOpenMetrics:
+		return fmtOpenMetrics_1_0_0
+	default:
+		return fmtUnknown
+	}
+}
+
 // FormatType deduces an overall FormatType for the given format.
 func (f Format) FormatType() FormatType {
 	toks := strings.Split(string(f), ";")


Test patch:
diff --git a/expfmt/decode_test.go b/expfmt/decode_test.go
index 3c023f5..e5e245d 100644
--- a/expfmt/decode_test.go
+++ b/expfmt/decode_test.go
@@ -421,27 +421,27 @@ func testDiscriminatorHTTPHeader(t testing.TB) {
 	}{
 		{
 			input:  map[string]string{"Content-Type": `application/vnd.google.protobuf; proto="io.prometheus.client.MetricFamily"; encoding="delimited"`},
-			output: FmtProtoDelim,
+			output: fmtProtoDelim,
 		},
 		{
 			input:  map[string]string{"Content-Type": `application/vnd.google.protobuf; proto="illegal"; encoding="delimited"`},
-			output: FmtUnknown,
+			output: fmtUnknown,
 		},
 		{
 			input:  map[string]string{"Content-Type": `application/vnd.google.protobuf; proto="io.prometheus.client.MetricFamily"; encoding="illegal"`},
-			output: FmtUnknown,
+			output: fmtUnknown,
 		},
 		{
 			input:  map[string]string{"Content-Type": `text/plain; version=0.0.4`},
-			output: FmtText,
+			output: fmtText,
 		},
 		{
 			input:  map[string]string{"Content-Type": `text/plain`},
-			output: FmtText,
+			output: fmtText,
 		},
 		{
 			input:  map[string]string{"Content-Type": `text/plain; version=0.0.3`},
-			output: FmtUnknown,
+			output: fmtUnknown,
 		},
 	}
 
@@ -547,7 +547,7 @@ func TestTextDecoderWithBufioReader(t *testing.T) {
 
 	var decoded bool
 	r := bufio.NewReader(strings.NewReader(example))
-	dec := NewDecoder(r, FmtText)
+	dec := NewDecoder(r, fmtText)
 	for {
 		var mf dto.MetricFamily
 		if err := dec.Decode(&mf); err != nil {
diff --git a/expfmt/encode_test.go b/expfmt/encode_test.go
index 66893bc..c5e4146 100644
--- a/expfmt/encode_test.go
+++ b/expfmt/encode_test.go
@@ -201,7 +201,7 @@ func TestNegotiateOpenMetrics(t *testing.T) {
 
 func TestEncode(t *testing.T) {
 	var buff bytes.Buffer
-	delimEncoder := NewEncoder(&buff, FmtProtoDelim)
+	delimEncoder := NewEncoder(&buff, fmtProtoDelim)
 	metric := &dto.MetricFamily{
 		Name: proto.String("foo_metric"),
 		Type: dto.MetricType_UNTYPED.Enum(),
@@ -226,7 +226,7 @@ func TestEncode(t *testing.T) {
 
 	buff.Reset()
 
-	compactEncoder := NewEncoder(&buff, FmtProtoCompact)
+	compactEncoder := NewEncoder(&buff, fmtProtoCompact)
 	err = compactEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())
@@ -239,7 +239,7 @@ func TestEncode(t *testing.T) {
 
 	buff.Reset()
 
-	protoTextEncoder := NewEncoder(&buff, FmtProtoText)
+	protoTextEncoder := NewEncoder(&buff, fmtProtoText)
 	err = protoTextEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())
@@ -252,7 +252,7 @@ func TestEncode(t *testing.T) {
 
 	buff.Reset()
 
-	textEncoder := NewEncoder(&buff, FmtText)
+	textEncoder := NewEncoder(&buff, fmtText)
 	err = textEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())
@@ -273,7 +273,7 @@ func TestEncode(t *testing.T) {
 
 func TestEscapedEncode(t *testing.T) {
 	var buff bytes.Buffer
-	delimEncoder := NewEncoder(&buff, FmtProtoDelim+"; escaping=underscores")
+	delimEncoder := NewEncoder(&buff, fmtProtoDelim+"; escaping=underscores")
 	metric := &dto.MetricFamily{
 		Name: proto.String("foo.metric"),
 		Type: dto.MetricType_UNTYPED.Enum(),
@@ -309,7 +309,7 @@ func TestEscapedEncode(t *testing.T) {
 
 	buff.Reset()
 
-	compactEncoder := NewEncoder(&buff, FmtProtoCompact)
+	compactEncoder := NewEncoder(&buff, fmtProtoCompact)
 	err = compactEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())
@@ -322,7 +322,7 @@ func TestEscapedEncode(t *testing.T) {
 
 	buff.Reset()
 
-	protoTextEncoder := NewEncoder(&buff, FmtProtoText)
+	protoTextEncoder := NewEncoder(&buff, fmtProtoText)
 	err = protoTextEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())
@@ -335,7 +335,7 @@ func TestEscapedEncode(t *testing.T) {
 
 	buff.Reset()
 
-	textEncoder := NewEncoder(&buff, FmtText)
+	textEncoder := NewEncoder(&buff, fmtText)
 	err = textEncoder.Encode(metric)
 	if err != nil {
 		t.Errorf("unexpected error during encode: %s", err.Error())