grafana / pyroscope

Continuous Profiling Platform. Debug performance issues down to a single line of code
https://grafana.com/oss/pyroscope/
GNU Affero General Public License v3.0
10.43k stars 637 forks source link

ring: Use net.JoinHostPort to support IPv6 addresses #3900

Closed Nihasa350 closed 1 month ago

Nihasa350 commented 1 month ago

Describe the bug

Pyroscope querier cannot construct an IPv6 address correctly and errors with "too many colons in address". Tracked down the issue here: https://github.com/grafana/pyroscope/blob/ae0067f11a7fee693a7b2e6620bc6e8ea34fa5be/pkg/scheduler/schedulerdiscovery/ring.go#L45

To Reproduce

Steps to reproduce the behavior:

  1. Start Pyroscope in kubernetes with IPv6 (1.12.0)

Expected behavior

Querier should be able to process requests.

Environment

Additional Context

Here is PR from mimir that fixes the IPv6 address construction: https://github.com/grafana/mimir/commit/b3d4f613a25631762667a6c80e801e4dd0f61d0d

marcsanmi commented 1 month ago

Hi @Nihasa350,

Thanks for reporting this, I'll take a look.

gjacquet commented 1 month ago

I believe I was facing the same issue trying to run Pyroscope on IPv6 and I tried the fix using docker image grafana/pyroscope:main-0d7771a and I am still seeing a similar error:

ts=2025-02-13T19:55:35.294346853Z caller=pool.go:250 level=warn component=querier-worker msg="removing frontend failing healthcheck" addr=xx:xx:xx:xx:xx::xx:4040 reason="rpc error: code = Unavailable desc = connection error: desc = \"transport: Error while dialing: dial tcp: address xx:xx:xx:xx:xx::xx:4040: too many colons in address\""

I believe there could be a few more places where the endpoint address is not being built properly but I could not really figure out where yet.

gjacquet commented 1 month ago

Actually, I think I found it: https://github.com/grafana/pyroscope/blob/main/pkg/frontend/frontend.go#L148

gjacquet commented 1 month ago

There could be a few other spots that could have the issue based on this search: https://github.com/search?q=repo%3Agrafana%2Fpyroscope%20%22%25s%3A%25d%22&type=code

Other results seem to be unrelated to the issue or tests.

marcsanmi commented 1 month ago

You're right, I've created a follow-up PR.

Thanks for checking on this!