search

gubernator-io / gubernator

High Performance Rate Limiting MicroService and Library - Developed at Mailgun
Apache License 2.0
93 stars 7 forks source link

Comment TraceLevelInfoFilter as it breaks OTel compatibility #21

Closed vikmik closed 4 months ago

vikmik commented 4 months ago

TraceLevelInfoFilter isn't used - its only use was in https://github.com/gubernator-io/gubernator/blob/e054be398e78e5daad7a3d60e9045f8991f1f8d4/daemon.go#L116 but is currently commented.

This PR unblocks consumers of gubernator who also use the latest OTel libraries (version 0.52), as the type otelgrpc.Filter changed in https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5196 ). See change here. The code I commented is now invalid with latest versions of OTel, which prevents consumers of OTel libraries to upgrade them if they also use gubernator. Building gubernator with the latest OTel libraries fails with:

# github.com/gubernator-io/gubernator/v2
../../../../pkg/mod/github.com/gubernator-io/gubernator/v2@v2.7.4/config.go:739:44: cannot convert func(info *otelgrpc.InterceptorInfo) bool {…} (value of type func(info *otelgrpc.InterceptorInfo) bool) to type otelgrpc.Filter

Allegedly this could now be fixed with the merge of the above PR (and the code in daemon.go could be uncommented), but it requires a larger change.

I am opening this PR to unblock users of OTel and gubernator as it is the most pressing issue.

Commenting feels a bit weird (I did it because of the commented code in daemon.go), let me know if you want to address this differently.

vikmik commented 4 months ago

Also cc @thrawn01 just in case :)

thrawn01 commented 4 months ago

There have been so many golang mod compatibility errors with otel, over the past few months. I'm beginning to wonder if it's worth the hassle.

The maintainers of different otel libraries don't appear to be managing releases properly. 😞

thrawn01 commented 4 months ago

For my Ref: https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4575 no deprecation notice, with no suggestions of an alternative. 👎

vikmik commented 4 months ago

Just seeing this. Thank you! Yes it seems that having an OTel dependency can make things difficult. I made this PR recently too. Thanks for merging this PR

pbennett commented 3 weeks ago

There have been so many golang mod compatibility errors with otel, over the past few months. I'm beginning to wonder if it's worth the hassle.

The maintainers of different otel libraries don't appear to be managing releases properly. 😞

I'm inclined to agree. I use some other packages that recently switched (somewhat forced) to otel as well and it seems like it might becoming a bit of a nightmare dependency.