weaveworks / common

Libraries used in multiple Weave projects
Other
129 stars 92 forks source link

Update prometheus/exporter-toolkit to v0.8.2 and gRPC to 1.47 #262

Closed rabenhorst closed 1 year ago

rabenhorst commented 1 year ago

Updated prometheus/exporter-toolkit to v0.8.2 to make it work with Prometheus v0.40.1.

bboreham commented 1 year ago

Thanks for the PR, by the way.

bboreham commented 1 year ago

Looks like the chain of dependencies that updates gRPC is:

github.com/prometheus/exporter-toolkit@v0.8.1 -> golang.org/x/oauth2@v0.0.0-20220909003341-f21342109be1 here -> cloud.google.com/go/compute@v1.7.0 here -> google.golang.org/grpc@v1.47.0 here

I don't see a way to break that, except larger-scale copy-paste.

krajorama commented 1 year ago

This is actually needed for native historgrams work as well in mimir-prometheus.

krajorama commented 1 year ago

Please run make protos and add the result to the PR, thanks

Actually this lead to errors for Mimir, so let's forget it for now. I'm trying to figure out what happened.

krajorama commented 1 year ago

We should merge #264 first. Fixes the make protos issue above.

rabenhorst commented 1 year ago

We should merge #264 first. Fixes the make protos issue above.

I rebased on master, ran make protos on both macOS arm and linux x86 machine and always get an error:

gogoproto/gogo.proto: File not found.
httpgrpc/httpgrpc.proto:5:1: Import “gogoproto/gogo.proto” was not found or had errors.

Any idea how to fix this?

cristiangreco commented 1 year ago

Drive-by: prometheus/exporter-toolkit@v0.8.2 has been released today, which fixes https://pkg.go.dev/vuln/GO-2022-1130

krajorama commented 1 year ago

We should merge #264 first. Fixes the make protos issue above.

I rebased on master, ran make protos on both macOS arm and linux x86 machine and always get an error:

gogoproto/gogo.proto: File not found.
httpgrpc/httpgrpc.proto:5:1: Import “gogoproto/gogo.proto” was not found or had errors.

Any idea how to fix this?

Put the path to gogoproto/gogo.proto in the -I include path of protoc compiler. (Since it is already a dependency of thanos-io).

E.g. in mimir: https://github.com/grafana/mimir/blob/0f2929359b8b02b8427f1a901839b1805f43d58b/Makefile#L243

But since you're not using vendoring, you might have to do like in prometheus/alertmanager: https://github.com/prometheus/alertmanager/blob/6ef6e6868dbeb7984d2d577dd4bf75c65bf1904f/Makefile.common#L241 https://github.com/prometheus/alertmanager/blob/main/scripts/genproto.sh

And possibly add a tools.go to make go get actually download it :( https://github.com/weaveworks/common/blob/master/httpgrpc/tools.go

Background: we found that make protos here in weaveworks/common did not reproduce the generated files since it was missing the gogo extension definitions. Now this is added so CI can check protobuf definitions and generated files automatically.

rabenhorst commented 1 year ago

Put the path to gogoproto/gogo.proto in the -I include path of protoc compiler. (Since it is already a dependency of thanos-io).

Thx. It worked and there is no change in generated files. Is this expected?

Also I updated exporter-toolkit to v0.8.2.

krajorama commented 1 year ago

Put the path to gogoproto/gogo.proto in the -I include path of protoc compiler. (Since it is already a dependency of thanos-io).

Thx. It worked and there is no change in generated files. Is this expected?

Also I updated exporter-toolkit to v0.8.2.

Yes, that's expected, we just wanted to get to a reproducable build where make protos does not result in diffs in git.