Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unexport Format strings #576

Merged
merged 1 commit into from
Feb 20, 2024
+65 −44
Merged

Unexport Format strings #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions expfmt/decode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,29 +45,29 @@ func ResponseFormat(h http.Header) Format {

mediatype, params, err := mime.ParseMediaType(ct)
if err != nil {
return FmtUnknown
return fmtUnknown
}

const textType = "text/plain"

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.
Expand Down
14 changes: 7 additions & 7 deletions expfmt/decode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 12 additions & 12 deletions expfmt/encode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 8 additions & 8 deletions expfmt/encode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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(),
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down
41 changes: 31 additions & 10 deletions expfmt/expfmt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to generate a fmtOpenMetrics_0_0_1?

Copy link

@slonka slonka Mar 1, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, need the same here https://github.com/kumahq/kuma/pull/9462/files#diff-3da7f423b56d8debbb5ba7872115cead6f0d632d927ee3e515ec0334a05cb990R12

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the new functions in expfmt.go can create these for you: https://github.com/prometheus/common/blob/main/expfmt/expfmt.go#L77

The story is, because the exposition formats are getting more complex, simple string comparison is no longer safe. There are too many equivalent permutations, and therefore things were moved to functions. If the available NewFormat function is not sufficient for your needs, please @ me or file an issue and I can fix it up

Copy link
Contributor

@mrueg mrueg Mar 1, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now TypeOpenMetrics only generates fmtOpenMetrics_1_0_0

If for some reason you need fmtOpenMetrics_0_0_1 there's no way to generate it.

We probably need a TypeOpenMetrics_0_0_1 or an optional version argument for the NewFormat func

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see -- yup I can prep a PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like I should also increase the test coverage -- I'll get to this next week. sorry for the breakage in the meantime!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrueg please let me know if this approach works for you

#596

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), ";")
Expand Down