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

Not possible to use both otelmetrichttp and otelmetricgrpc #5251

Closed
pellared opened this issue Apr 22, 2024 · 4 comments · Fixed by #5253
Closed

Not possible to use both otelmetrichttp and otelmetricgrpc #5251

pellared opened this issue Apr 22, 2024 · 4 comments · Fixed by #5253
Assignees
Labels
bug Something isn't working
Milestone

Comments

@pellared
Copy link
Member

FYI. it looks like its no longer possible to have both otelmetrichttp and otelmetricgrpc in the same binary. I'm not sure how common this is, but go.opentelemetry.io/contrib/exporters/autoexport is an example.

$ git clone https://github.com/open-telemetry/opentelemetry-go-contrib.git
$ cd opentelemetry-go-contrib/exporters/autoexport/
$ go get go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp@main
go: upgraded go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.25.0 => v1.25.1-0.20240422142149-b34cfc47c4e0
go: added go.opentelemetry.io/proto/slim/otlp v1.2.0
$ go get go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc@main
go: downloading go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.25.1-0.20240422142149-b34cfc47c4e0
go: upgraded go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.25.0 => v1.25.1-0.20240422142149-b34cfc47c4e0
$ go test ./...
panic: proto: file "opentelemetry/proto/common/v1/common.proto" is already registered
	previously from: "go.opentelemetry.io/proto/otlp/common/v1"
	currently from:  "go.opentelemetry.io/proto/slim/otlp/common/v1"
See https://protobuf.dev/reference/go/faq#namespace-conflict

Originally posted by @gregoryfranklin in #2579 (comment)

@pellared pellared added this to the v1.26.0 milestone Apr 22, 2024
@pellared
Copy link
Member Author

pellared commented Apr 22, 2024

This has be double-check before making a new release as it can be a blocker.

@pellared pellared added the bug Something isn't working label Apr 22, 2024
@MrAlias
Copy link
Contributor

MrAlias commented Apr 22, 2024

Please coordinate with @XSAM who is working on #5250

@pellared pellared changed the title Not possible to have both otelmetrichttp and otelmetricgrpc in the same binary. I'm not sure how common this is, but go.opentelemetry.io/contrib/exporters/autoexport is an example. Not possible to have both otelmetrichttp and otelmetricgrpc in the same binary Apr 22, 2024
@pellared pellared changed the title Not possible to have both otelmetrichttp and otelmetricgrpc in the same binary Not possible to use both otelmetrichttp and otelmetricgrpc Apr 22, 2024
@pellared
Copy link
Member Author

pellared commented Apr 22, 2024

Most probably we will have to revert these two PRs:

@gregoryfranklin, thanks for reporting the issue ❤️

@XSAM
Copy link
Member

XSAM commented Apr 22, 2024

I have checked the protobuf codes regarding this behavior. The implementation prevents importing go.opentelemetry.io/proto/otlp and go.opentelemetry.io/proto/slim at the same time.

https://github.com/protocolbuffers/protobuf-go/blob/e4ad8f9dfc8bc3184ebb70c296bd63955343e903/reflect/protoregistry/registry.go#L126-L133

Though we could use go build -ldflags "-X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=warn" to change the conflict policy and probably pass this check, it requires our end-users to do this, which is not acceptable.

Plan for the release

Rollback these two PRs:

After the release

We might need to deprecate package go.opentelemetry.io/proto/slim, as it is not compatible with go.opentelemetry.io/proto/otlp, which provides grpc support. And it makes the package somewhat unusable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

3 participants