elixir-grpc / grpc

An Elixir implementation of gRPC
https://hex.pm/packages/grpc
Apache License 2.0
1.36k stars 210 forks source link

GRPC Prometheus in interop tests #293

Closed wingyplus closed 1 year ago

wingyplus commented 1 year ago

I see the interop test use grpc prometheus package but I didn't see we use it to measure anything. And I found that it can be compile on my local machine with Elixir 1.14.2/OTP 25 due to error:

$ cd interop/
$ mix compile
==> prometheus_ex
Compiling 19 files (.ex)

== Compilation error in file lib/prometheus/buckets.ex ==
** (UndefinedFunctionError) function Kernel.Utils.defdelegate/2 is undefined or private. Did you mean:

      * defdelegate_all/3
      * defdelegate_each/2

    (elixir 1.14.2) Kernel.Utils.defdelegate({:new, [line: 18], [{:arg, [line: 18], nil}]}, [])
    lib/prometheus/buckets.ex:18: (module)
could not compile dependency :prometheus_ex, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile prometheus_ex", update it with "mix deps.update prometheus_ex" or clean it with "mix deps.clean prometheus_ex"

I guess that it's because of too old prometheus_ex library. So if we didn't use it, I purpose to removing it from the code base.

What do you think?

polvalente commented 1 year ago

I think we can remove this, but perhaps @tony612 has a reason to keep it. Although we need to update grpc-prometheus in this regard as well, so perhaps an issue on the repo would be welcome.

wingyplus commented 1 year ago

@polvalente The issue is open. 🙇

wingyplus commented 1 year ago

I found the prometheus_ex is quite too old. They didn't update the code for a long time (from https://github.com/deadtrickster/prometheus.ex). I start thinking that we should not keep it or we should find another way to tackle it (like #229).

polvalente commented 1 year ago

Let's discuss this on the other repo :)

sleipnir commented 1 year ago

I think this should be inside the main library. Maybe being handled via https://github.com/elixir-grpc/grpc/issues/264 or https://github.com/elixir-grpc/grpc/issues/229

polvalente commented 1 year ago

@sleipnir That's what I suggested. #229 can be used for this together with TelemetryMetricsPrometheus

sleipnir commented 1 year ago

@polvalente Sorry I didn't see your suggestion before

tony612 commented 1 year ago

I think it's still used? https://github.com/elixir-grpc/grpc/blob/master/interop/lib/interop/endpoint.ex

polvalente commented 1 year ago

@tony612 it's used in that sense, but no interop tests actually use it. I think it's fine for us to remove it for now so we can use Elixir 1.14 properly in CI.