Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Apr 4, 2022

Reason for Change:

  • Moves metrics to the CNS service port (from previously a dedicated metrics port).
  • Adds healthz handler to the root mux.
  • Adds pprof handlers to the root mux conditionally based on a new debug config opt.

Issue Fixed:

Closes #1294 (because I can't push to it)

Requirements:

Notes:

@rbtr rbtr requested a review from paulgmiller April 4, 2022 18:16
@rbtr rbtr requested a review from neaggarwMS April 4, 2022 18:52
@rbtr rbtr self-assigned this Apr 4, 2022
@rbtr rbtr requested review from rsagasthya and thatmattlong April 4, 2022 19:03
mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
}

// to reduce
Copy link
Member

Choose a reason for hiding this comment

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

sadparital comment left behind by paul.

paulgmiller
paulgmiller previously approved these changes Apr 4, 2022
return errors.Wrapf(err, "failed to setup reconciler with manager")
}

mux := httpRestServiceImplementation.Listener.GetMux()
Copy link
Member

Choose a reason for hiding this comment

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

we have to remember to update the metrics annotation port when we bring this in. Weee non atomic changes.

mux.Handle("/metrics", promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{
ErrorHandling: promhttp.HTTPErrorOnError,
}))
healthzhandler := healthz.Handler{}
Copy link
Member

Choose a reason for hiding this comment

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

no idea if this little heathz framework from controller-runtime is good or not. Feel freee to toss.

@rbtr rbtr force-pushed the healthz branch 3 times, most recently from bc1cc6e to ab917ee Compare April 4, 2022 22:36
thatmattlong
thatmattlong previously approved these changes Apr 4, 2022
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr enabled auto-merge (squash) April 4, 2022 22:58
@rbtr rbtr added cns Related to CNS. swift Related to SWIFT networking. labels Apr 5, 2022
@rbtr rbtr merged commit 5ec6a3f into Azure:master Apr 5, 2022
@rbtr rbtr deleted the healthz branch April 5, 2022 22:27
rbtr added a commit to rbtr/azure-container-networking that referenced this pull request May 4, 2022
rbtr added a commit to rbtr/azure-container-networking that referenced this pull request May 4, 2022
rbtr added a commit to rbtr/azure-container-networking that referenced this pull request May 4, 2022
neaggarwMS pushed a commit that referenced this pull request May 5, 2022
rsagasthya pushed a commit to rsagasthya/azure-container-networking that referenced this pull request May 18, 2022
matmerr pushed a commit to matmerr/azure-container-networking that referenced this pull request Jun 29, 2022
* combine metrics and healthz

* use root mux and bind pprof routes on debug config opt

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

Co-authored-by: Paul Miller <pmiller@microsoft.com>
matmerr pushed a commit to matmerr/azure-container-networking that referenced this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS. swift Related to SWIFT networking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants