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

Breaking change #19

Closed
CodeLieutenant opened this issue Oct 25, 2022 · 9 comments
Closed

Breaking change #19

CodeLieutenant opened this issue Oct 25, 2022 · 9 comments

Comments

@CodeLieutenant
Copy link

CodeLieutenant commented Oct 25, 2022

After the release of v1.0.3 this library is no longer backwards compatible with the old proto.Message interface

V1 Message Interface

type MessageV1 interface {
	Reset()
	String() [string](https://pkg.go.dev/builtin#string)
	ProtoMessage()
}

After the import of new protobuf package from google.golang.org/protobuf/proto proto.Message is https://pkg.go.dev/google.golang.org/protobuf@v1.28.1/reflect/protoreflect#ProtoMessage

type ProtoMessage interface{ 
   ProtoReflect() Message 
}

This currently breaks https://github.com/prometheus/common/blob/main/expfmt/decode.go

go/pkg/mod/github.com/prometheus/common@v0.37.0/expfmt/decode.go:89:38: cannot use v (variable of type *io_prometheus_client.MetricFamily) as type protoreflect.ProtoMessage in argument to pbutil.ReadDelimited:
        *io_prometheus_client.MetricFamily does not implement protoreflect.ProtoMessage (missing ProtoReflect method)

go/pkg/mod/github.com/prometheus/common@v0.37.0/expfmt/encode.go:120:40: cannot use v (variable of type *io_prometheus_client.MetricFamily) as type protoreflect.ProtoMessage in argument to pbutil.WriteDelimited:
        *io_prometheus_client.MetricFamily does not implement protoreflect.ProtoMessage (missing ProtoReflect method)

Could you please remove this release and tag it with v2.0.0 or something?

@nicolaasuni-vonage
Copy link

After upgrading to github.com/matttproud/golang_protobuf_extensions v1.0.3 I get the following errors:

# github.com/prometheus/common/expfmt
../../../../pkg/mod/github.com/prometheus/common@v0.37.0/expfmt/decode.go:89:38: cannot use v (variable of type *io_prometheus_client.MetricFamily) as type protoreflect.ProtoMessage in argument to pbutil.ReadDelimited:
	*io_prometheus_client.MetricFamily does not implement protoreflect.ProtoMessage (missing ProtoReflect method)
../../../../pkg/mod/github.com/prometheus/common@v0.37.0/expfmt/encode.go:120:40: cannot use v (variable of type *io_prometheus_client.MetricFamily) as type protoreflect.ProtoMessage in argument to pbutil.WriteDelimited:
	*io_prometheus_client.MetricFamily does not implement protoreflect.ProtoMessage (missing ProtoReflect method)

The linter golangci-linter fails with:

ERRO Running error: 1 error occurred:
	* can't run linter goanalysis_metalinter: inspect: failed to load package prometheus: could not load export data: no export data for "github.com/****/****/prometheus"

@matttproud
Copy link
Owner

matttproud commented Oct 25, 2022 via email

@nomad-software
Copy link

nomad-software commented Oct 25, 2022

Hi @matttproud could this be worked on as a priority please this is a big breaking change and just took me ages to find the cause and it affects many of our services. Thanks.

@matttproud
Copy link
Owner

matttproud commented Oct 25, 2022 via email

@CodeLieutenant
Copy link
Author

Thank you for your time, hope it will be fixed soon.

@nomad-software
Copy link

nomad-software commented Oct 25, 2022

Please either pin to v1.0.2 or upgrade your Protocol Buffer version and message assets.

The issue in my case is that this library is a transitive dependency of Prometheus which we use for all gRPC services.

I can pin it in my go.mod and it compiles fine, which is good. I guess a lot more people are going to have the same issue.

@CodeLieutenant
Copy link
Author

Please either pin to v1.0.2 or upgrade your Protocol Buffer version and message assets.

The issue in my case is that this library is a transitive dependency of Prometheus.

I can pin it in my go.mod and it compiles fine, which is good. I guess a lot more people are going to have the same issue.

As I can see, this library is used in Prometheus common library and their GRPC models are not generated with new grpc version aka google.golang.org/protobuf/proto. Most likely as this will be breaking change for them and they don't want that.
Haven't seen this library used anywhere else except in Prometheus lib

@matttproud
Copy link
Owner

matttproud commented Oct 25, 2022 via email

@matttproud
Copy link
Owner

Thanks for bearing with me on this. I apologize for any inconvenience this caused. I was on the other side of Alps from my computer by the time the first defect report arose, which was why I couldn't immediately rectify this.

I have pushed a new tag v1.0.4, which is pinned to the v1.0.2 code. For now, the behavior of v1.0.4 and similar now lives in a legacy v1 branch. I expect that branch to go dormant long-term with a subsequent update that provides deprecation notices on the old APIs advising users to migrate to a new major version 2.

Going forward, the master branch will take the code of v1.0.3 and track what is today's current Protocol Buffer implementation and generated code API. This will become major version 2. Users should adapt to this where possible, though there is no need to worry that v1 will go away. Migrating to version 2 will require updating both the version of the Protocol Buffer library you use as well as any generated Protocol Buffer message file assets. For small and medium projects, this should be a simple affair. I will cut a version 2 release soon hereafter.

My instinct told me before cutting v1.0.3 I should have made a new major version instead, but I was not 100% certain due to these reasons. And then there is some extra ceremony involved with creating new module major versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants