search

open-telemetry / opentelemetry-operator

Kubernetes Operator for OpenTelemetry Collector
Apache License 2.0
1.2k stars 438 forks source link

[target allocator] Replace deprecated `gorilla/mux` dependency #1352

Closed matej-g closed 1 year ago

matej-g commented 1 year ago

The allocator server uses the router from gorilla/mux, however the project has been deprecated recently (https://github.com/gorilla#gorilla-toolkit) and is no longer actively maintained. We should replace it with an alternative that's still maintained.

matej-g commented 1 year ago

(also if there's agreement on this, happy to take this / propose and replace with an alternative 🙃)

pavolloffay commented 1 year ago

cc) @jaronoff97

@matej-g thanks for reporting this. Do you have any good alternatives in mind?

matej-g commented 1 year ago

Not sure what else is commonly used within Go projects these days, in other projects I'm involved in we use https://github.com/go-chi/chi and we had good experience with it.

frzifus commented 1 year ago

seems that only effects the otel-allocator.

jaronoff97 commented 1 year ago

@matej-g the other framework I was considering was gin is there a reason to use chi over gin? I don't have a preference FWIW, just want to add another option in to the mix.

frzifus commented 1 year ago

Is there any specific need to such a framework? As far as I can see, the only functionality that is not in the std lib is the query of the url path variable {job_id}.

matej-g commented 1 year ago

Hm that's also a good point @frzifus, I think I can try to prepare a PR with either alternative (standard lib or one of the frameworks if we need the extra features) and we could continue from there.

jaronoff97 commented 1 year ago

@matej-g that would be great, thank you! My worry with the standard library is that if we need to use features that something like gin or chi provide how hard it would be to switch? If it's low enough and we can make it work with the stdlib, then let's go that route. Either way having a PR to compare would be super helpful. Thank you again @matej-g.

matej-g commented 1 year ago

👋 I prepared PR replacing gorilla/mux with gin (https://github.com/open-telemetry/opentelemetry-operator/pull/1383), after all due to the need to handle the variable in the path as @frzifus mentioned, there would be more added complexed if we were to coded that by hand, so I believe using gin is a good alternative. Feedback welcome!

frzifus commented 1 year ago

Awesome, I will have a look soon. :)