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.

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

Move prompb to its own module #10371

Closed
wenjianqiao opened this issue Feb 28, 2022 · 9 comments
Closed

Move prompb to its own module #10371

wenjianqiao opened this issue Feb 28, 2022 · 9 comments

Comments

@wenjianqiao
Copy link

wenjianqiao commented Feb 28, 2022

Proposal

Use case. Why is this important?

For our project, I need to implement Prometheus Remote Write to collect Prometheus metrics sent from Prometheus servers. It only requires the github.com/prometheus/prometheus/prompb package, but I have to add the entire github.com/prometheus/prometheus module to go.mod. This pulls in a bunch of irrelevant go modules and some of the dependency modules expose security vulnerabilities per our code scan.

Moreover, as reported in #8417 and #8852, we have to use github.com/prometheus/prometheus v1.8.2-0.20210331101223-3cafc58827d1 in go.mod, which raises a security vulnerability in code scan.

Prometheus Remote Write is a nice feature and easy to implement. However, it becomes complicated because we have to include entire prometheus server module. Moving github.com/prometheus/prometheus/prompb to its own module will solve the problem.

@roidelapluie
Copy link
Member

Could you try https://buf.build/prometheus/prometheus/assets ?

go get go.buf.build/protocolbuffers/go/prometheus/prometheus

@wenjianqiao
Copy link
Author

wenjianqiao commented Mar 1, 2022

Thank you for the response. I was able to run go get go.buf.build/protocolbuffers/go/prometheus/prometheus and add go.buf.build/protocolbuffers/go/prometheus/prometheus v1.1.1 to go.mod.

However, the new go.buf.build/protocolbuffers/go/prometheus/prometheus package defines different structs than the github.com/prometheus/prometheus/prompb package.

For example, go.buf.build/protocolbuffers/go/prometheus/prometheus has:

type TimeSeries struct {
	state         protoimpl.MessageState
	sizeCache     protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields

	// For a timeseries to be valid, and for the samples and exemplars
	// to be ingested by the remote system properly, the labels field is required.
	Labels    []*Label    `protobuf:"bytes,1,rep,name=labels,proto3" json:"labels,omitempty"`
	Samples   []*Sample   `protobuf:"bytes,2,rep,name=samples,proto3" json:"samples,omitempty"`
	Exemplars []*Exemplar `protobuf:"bytes,3,rep,name=exemplars,proto3" json:"exemplars,omitempty"`
}

but github.com/prometheus/prometheus/prompb has

type TimeSeries struct {
	Labels               []Label  `protobuf:"bytes,1,rep,name=labels,proto3" json:"labels"`
	Samples              []Sample `protobuf:"bytes,2,rep,name=samples,proto3" json:"samples"`
	XXX_NoUnkeyedLiteral struct{} `json:"-"`
	XXX_unrecognized     []byte   `json:"-"`
	XXX_sizecache        int32    `json:"-"`
}

@wenjianqiao
Copy link
Author

When using go.buf.build/protocolbuffers/go/prometheus/prometheus v1.1.1 module, I got panic when trying to marshal and unmarshall a WriteRequest object. Is there a newer version?

panic: protobuf tag not enough fields in WriteRequest.state:

goroutine 1 [running]:
github.com/gogo/protobuf/proto.(*unmarshalInfo).computeUnmarshalInfo(0xc0000c2320)
	/Users/[email protected]/gopath/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:341 +0x138a
github.com/gogo/protobuf/proto.(*unmarshalInfo).unmarshal(0xc0000c2320, {0x123f720}, {0x147d548, 0x0, 0x0})
	/Users/[email protected]/gopath/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:138 +0x67
github.com/gogo/protobuf/proto.(*InternalMessageInfo).Unmarshal(0xc0000cc0a0, {0x12a6d80, 0xc00008c300}, {0x147d548, 0x0, 0x0})
	/Users/[email protected]/gopath/pkg/mod/github.com/gogo/[email protected]/proto/table_unmarshal.go:63 +0xd0
github.com/gogo/protobuf/proto.(*Buffer).Unmarshal(0xc0000b3ef0, {0x12a6d80, 0xc00008c300})
	/Users/[email protected]/gopath/pkg/mod/github.com/gogo/[email protected]/proto/decode.go:424 +0x153
github.com/gogo/protobuf/proto.Unmarshal({0x147d548, 0x0, 0x0}, {0x12a6d80, 0xc00008c300})
	/Users/[email protected]/gopath/pkg/mod/github.com/gogo/[email protected]/proto/decode.go:342 +0xef
main.main()
	/Users/[email protected]/gopath/src/github.ibm.com/Observability/main.go:17 +0xe5

My code can be found at https://go.dev/play/p/zsTA3N25ss2 - it simply marshals a WriteRequest and then unmarshals it.

@roidelapluie
Copy link
Member

Please use golang/protobuf instead: https://go.dev/play/p/yz7yH5NQnCG

@wenjianqiao
Copy link
Author

Thank you. Changing to github.com/golang/protobuf/proto works, but the package is deprecated. The google.golang.org/protobuf package also works.

$ go get github.com/golang/protobuf
go: module github.com/golang/protobuf is deprecated: Use the "google.golang.org/protobuf" module instead.

@wenjianqiao
Copy link
Author

I have one more question. As I mentioned earlier in this comment #10371 (comment), github.com/prometheus/prometheus/prompb and go.buf.build/protocolbuffers/go/prometheus/prometheus have different definitions for Timeseries struct. Is it on purpose or just different versions?

type TimeSeries struct {
	Labels               []Label  `protobuf:"bytes,1,rep,name=labels,proto3" json:"labels"`
	Samples              []Sample `protobuf:"bytes,2,rep,name=samples,proto3" json:"samples"`
        ......
}

vs.

type TimeSeries struct {
	Labels    []*Label    `protobuf:"bytes,1,rep,name=labels,proto3" json:"labels,omitempty"`
	Samples   []*Sample   `protobuf:"bytes,2,rep,name=samples,proto3" json:"samples,omitempty"`
	......
}

@roidelapluie
Copy link
Member

go.buf.build/protocolbuffers/go/prometheus/prometheus is autogenerated, this is something we have introduced recently. it's very much possible that there are minor changes, but they should both work on the wire.

@wenjianqiao
Copy link
Author

wenjianqiao commented Mar 24, 2022

The go.buf.build/protocolbuffers/go/prometheus/prometheus module solved the dependency problem. We include the following module in go.mod.

go.buf.build/protocolbuffers/go/prometheus/prometheus v1.1.1

To unmarshal the protobuf messages, you need to import google.golang.org/protobuf/proto or github.com/golang/protobuf/proto (already deprecated).

@IzhakJakov
Copy link

I confirm that it works but I could not find where this is documented.
Where should I look?

@prometheus prometheus locked as resolved and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants