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

metrics: Do not rely on global HTTP server #11071

Merged
merged 1 commit into from Apr 21, 2020

Conversation

gandro
Copy link
Member

@gandro gandro commented Apr 20, 2020

This moves away from the global Golang HTTP server for serving the metrics for both Hubble and Cilium. Instead, a local instance is created, which allows the two metrics server to run concurrently.

Going forward, there also needs to be a discussion if we really want to have two different metrics server running inside cilium-agent. We either might want to move it out to hubble-relay (formerly hubble-proxy) or integrate the Hubble metrics with the Cilium ones.

This PR simply sidesteps that discussion by fixing the referenced bug in the simplest way possible.

Fixes: #11066

@gandro gandro added pending-review area/metrics Impacts statistics / metrics gathering, eg via Prometheus. release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay labels Apr 20, 2020
@gandro gandro requested a review from a team April 20, 2020 16:18
@gandro gandro requested a review from a team as a code owner April 20, 2020 16:18
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 20, 2020
@gandro
Copy link
Member Author

gandro commented Apr 20, 2020

test-me-please

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is odd indeed why it does fail with the official golang library.

@gandro
Copy link
Member Author

gandro commented Apr 20, 2020

@aanm

it is odd indeed why it does fail with the official golang library.

The global http.HandleFunc and http.ServeAndListen() call both the http.DefaultServeMux handler in their implementation. Therefore that causes a conflict, since both try to install a sub-handler for the /metrics patterns. This panics, as documented here: https://golang.org/pkg/net/http/#ServeMux.Handle

Copy link
Contributor

@soumynathan soumynathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gandro
Copy link
Member Author

gandro commented Apr 21, 2020

@gandro
Copy link
Member Author

gandro commented Apr 21, 2020

test-me-please

This moves away from the global Golang HTTP server for serving the
metrics for both Hubble and Cilium. Instead, a local instance is
created, which allows the two metrics server to run concurrently.

Fixes: #11066

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/serve-metrics-on-nondefault-httpserver branch from a15fb54 to 07805df Compare April 21, 2020 10:32
@gandro
Copy link
Member Author

gandro commented Apr 21, 2020

test-me-please

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 44.664% when pulling 07805df on pr/gandro/serve-metrics-on-nondefault-httpserver into 1598f74 on master.

@pchaigno pchaigno merged commit 90b256e into master Apr 21, 2020
1.8.0 automation moved this from In progress to Merged Apr 21, 2020
@pchaigno pchaigno deleted the pr/gandro/serve-metrics-on-nondefault-httpserver branch April 21, 2020 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

hubble: Hubble metrics server conflicts with cilium metrics
8 participants