client-go: export NewDebuggingRoundTripper function and DebugLevel #98324

atosatto · 2021-01-22T20:36:59Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

debuggingRoundTripper is a useful tool to debug Kubernetes API requests and their timing.

Unfortunately it can only be used via the DebugWrappers function, which adjusts the level of debugging information logged by the roundTripper based on the configured klog verbosity.

While DebugWrappers definitely works for clients using klog (like kubectl), this is currently hard to be used from controllers using controller-runtime, as by default the library uses github.com/go-logr/logr for logging.

In this PR we change the visibility of newDebuggingRoundTripper and debugLevel in order to be able to use directly the debugRoundTripper without the DebugWrappers function.

In particular, the changes proposed in this PR allow to use the debuggingRoundTripper to intercept Kubernetes API requests performed by controller-runtime using the following sample code

import (
        ctrl "sigs.k8s.io/controller-runtime"
)

func init() {
	ctrl.SetLogger(zap.New())
}

func main() {
        // wrap the http transport used by the Kubernetes client
	restConfig, err := ctrl.GetConfig()
	checkError(setupLog, err, "unable to get kubernetes client config")

        restConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper {
		return transport.NewDebuggingRoundTripper(rt, transport.DebugJustURL)
	})

        ...
}

Does this PR introduce a user-facing change?:

Export NewDebuggingRoundTripper function and DebugLevel options in the k8s.io/client-go/transport package.

`debuggingRoundTripper` is a useful throbleshooting tool to debug of Kubernetes API requests and their timing. Unfortunately, as of today, it can only be used via the `DebugWrappers` function, which automatically adjust the amount of debug information exposed by the roundTripper based on the configured `klog` verbosity. While `DebugWrappers` definitely fits the purpose for clients using `klog`, this is currently hard to be used for controllers using `controller-runtime`, which uses `github.com/go-logr/logr` for logging. In this PR we change the visibility of `newDebuggingRoundTripper` and `debugLevel` in order to be directly accessible from users of the `k8s.io/client-go/transport` package. In particular, the changes proposed in this PR allow users of `controller-runtime` to use the `debuggingRoundTripper` to intercept Kubernetes API requests as follows ```go import ( ctrl "sigs.k8s.io/controller-runtime" ) func init() { ctrl.SetLogger(zap.New()) } func main() { // wrap the http transport used by the Kubernetes client restConfig, err := ctrl.GetConfig() checkError(setupLog, err, "unable to get kubernetes client config") restConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { return transport.NewDebuggingRoundTripper(rt, transport.DebugJustURL) }) ... } ```

k8s-ci-robot · 2021-01-22T20:37:07Z

Hi @atosatto. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

leilajal · 2021-01-26T21:15:57Z

cc @lavalamp

atosatto · 2021-01-26T21:35:34Z

@lavalamp adding here some more information on my PR.

Currently, the one way we can build a debuggingRoundTripper is using the DebugWrappers function:

// DebugWrappers wraps a round tripper and logs based on the current log level.
func DebugWrappers(rt http.RoundTripper) http.RoundTripper {
	switch {
	case bool(klog.V(9).Enabled()):
		rt = newDebuggingRoundTripper(rt, debugCurlCommand, debugURLTiming, debugResponseHeaders)
	case bool(klog.V(8).Enabled()):
		rt = newDebuggingRoundTripper(rt, debugJustURL, debugRequestHeaders, debugResponseStatus, debugResponseHeaders)
	case bool(klog.V(7).Enabled()):
		rt = newDebuggingRoundTripper(rt, debugJustURL, debugRequestHeaders, debugResponseStatus)
	case bool(klog.V(6).Enabled()):
		rt = newDebuggingRoundTripper(rt, debugURLTiming)
	}

	return rt
}

Unfortunately this is relying on klog's verbosity to customise the level of information exposed by the debuggingRoundTripper and there is no way for consumers of the client-go API to customise the output of the debuggingRoundTripper.

In this PR I am proposing to change the visibility of newDebuggingRoundTripper (and debugLevels) to allow client-go's API consumers (in my case a controller built with controller-runtime) to create debuggingRoundTripper with custom debugLevels.

lavalamp

I think I'm pretty OK with this since it is just making something public. But did we consider letting the user also pass a log alternative in? So it doesn't have to use the global one? I guess that could be a separate change if we wanted to do that.

staging/src/k8s.io/client-go/transport/round_trippers.go

…rs.go

atosatto · 2021-01-26T22:08:36Z

I think I'm pretty OK with this since it is just making something public. But did we consider letting the user also pass a log alternative in? So it doesn't have to use the global one? I guess that could be a separate change if we wanted to do that.

I did but I am afraid this would be a bigger change (perhaps with more contention) as it would require us to switch to the logr.Logger interface and default to klogr to guarantee backward compatibility with the current behaviour.

I am happy to give it a try in a separated PR as this would definitely help in the scenario where the API is used in controller-runtime's controllers.

lavalamp · 2021-01-26T22:31:00Z

OK. Let's just take this and wait for the structured logging folks to work their way through the system.

/lgtm
/approve

lavalamp · 2021-01-26T22:31:15Z

/ok-to-test

k8s-ci-robot · 2021-01-26T22:31:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atosatto, lavalamp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/client-go/OWNERS~~ [lavalamp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…rs_test.go

staging/src/k8s.io/client-go/go.mod

atosatto · 2021-01-27T00:57:56Z

/test pull-kubernetes-e2e-kind-ipv6

atosatto · 2021-01-27T01:30:07Z

@lavalamp it looks like the PR is now passing all the tests. Would you mind checking again whether it is good to be merged? Thanks!

fedebongio · 2021-02-02T21:06:19Z

/triage accepted

lavalamp · 2021-02-02T21:07:56Z

/lgtm

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 22, 2021

k8s-ci-robot requested review from deads2k and krousey January 22, 2021 20:38

lavalamp reviewed Jan 26, 2021

View reviewed changes

staging/src/k8s.io/client-go/transport/round_trippers.go Outdated Show resolved Hide resolved

Run gofmt against staging/src/k8s.io/client-go/transport/round_trippe…

9cb5580

…rs.go

k8s-ci-robot assigned lavalamp Jan 26, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 26, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2021

Run gofmt against staging/src/k8s.io/client-go/transport/round_trippe…

e87349f

…rs_test.go

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2021

Address golint warnings

675cefa

Update bazel BUILD file

bdd2da5

k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Jan 26, 2021

lavalamp reviewed Jan 26, 2021

View reviewed changes

staging/src/k8s.io/client-go/go.mod Outdated Show resolved Hide resolved

atosatto added 2 commits January 26, 2021 23:49

Remove unexpected updates to go.mod and go.sum

961fa25

Re-run gofmt against staging/src/k8s.io/client-go/transport/

1449af1

atosatto requested a review from lavalamp January 28, 2021 21:53

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 2, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2021

k8s-ci-robot merged commit 27104f1 into kubernetes:master Feb 2, 2021

k8s-ci-robot added this to the v1.21 milestone Feb 2, 2021

github-actions bot mentioned this pull request Feb 3, 2021

Week Ending January 31, 2021 dev-obs/actus#339

Open

atosatto mentioned this pull request Mar 7, 2024

REQUEST: New membership for atosatto kubernetes/org#4804

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client-go: export NewDebuggingRoundTripper function and DebugLevel #98324

client-go: export NewDebuggingRoundTripper function and DebugLevel #98324

atosatto commented Jan 22, 2021 •

edited

k8s-ci-robot commented Jan 22, 2021

leilajal commented Jan 26, 2021

atosatto commented Jan 26, 2021 •

edited

lavalamp left a comment

atosatto commented Jan 26, 2021

lavalamp commented Jan 26, 2021

lavalamp commented Jan 26, 2021

k8s-ci-robot commented Jan 26, 2021

atosatto commented Jan 27, 2021

atosatto commented Jan 27, 2021

fedebongio commented Feb 2, 2021

lavalamp commented Feb 2, 2021

client-go: export NewDebuggingRoundTripper function and DebugLevel #98324

client-go: export NewDebuggingRoundTripper function and DebugLevel #98324

Conversation

atosatto commented Jan 22, 2021 • edited

k8s-ci-robot commented Jan 22, 2021

leilajal commented Jan 26, 2021

atosatto commented Jan 26, 2021 • edited

lavalamp left a comment

Choose a reason for hiding this comment

atosatto commented Jan 26, 2021

lavalamp commented Jan 26, 2021

lavalamp commented Jan 26, 2021

k8s-ci-robot commented Jan 26, 2021

atosatto commented Jan 27, 2021

atosatto commented Jan 27, 2021

fedebongio commented Feb 2, 2021

lavalamp commented Feb 2, 2021

atosatto commented Jan 22, 2021 •

edited

atosatto commented Jan 26, 2021 •

edited