From 8aa3e0f684586e36f4b1876f883d98bcb96bfd6a Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Mon, 8 Feb 2021 15:20:15 -0500 Subject: [PATCH] exec credential provider: add rest_client_exec_plugin_call_total metric Signed-off-by: Andrew Keesler Kubernetes-commit: 31eec29b098f790cd96fd6d2441e68938f15363b --- plugin/pkg/client/auth/exec/exec.go | 4 +- plugin/pkg/client/auth/exec/metrics.go | 49 ++++++++++ plugin/pkg/client/auth/exec/metrics_test.go | 101 ++++++++++++++++++++ tools/metrics/metrics.go | 17 ++++ 4 files changed, 170 insertions(+), 1 deletion(-) diff --git a/plugin/pkg/client/auth/exec/exec.go b/plugin/pkg/client/auth/exec/exec.go index 43075bde..b23e57dd 100644 --- a/plugin/pkg/client/auth/exec/exec.go +++ b/plugin/pkg/client/auth/exec/exec.go @@ -401,7 +401,9 @@ func (a *Authenticator) refreshCredsLocked(r *clientauthentication.Response) err cmd.Stdin = a.stdin } - if err := cmd.Run(); err != nil { + err = cmd.Run() + incrementCallsMetric(err) + if err != nil { return a.wrapCmdRunErrorLocked(err) } diff --git a/plugin/pkg/client/auth/exec/metrics.go b/plugin/pkg/client/auth/exec/metrics.go index caf0cca3..3a2cc251 100644 --- a/plugin/pkg/client/auth/exec/metrics.go +++ b/plugin/pkg/client/auth/exec/metrics.go @@ -17,12 +17,39 @@ limitations under the License. package exec import ( + "errors" + "os/exec" + "reflect" "sync" "time" + "k8s.io/klog/v2" + "k8s.io/client-go/tools/metrics" ) +// The following constants shadow the special values used in the prometheus metrics implementation. +const ( + // noError indicates that the plugin process was successfully started and exited with an exit + // code of 0. + noError = "no_error" + // pluginExecutionError indicates that the plugin process was successfully started and then + // it returned a non-zero exit code. + pluginExecutionError = "plugin_execution_error" + // pluginNotFoundError indicates that we could not find the exec plugin. + pluginNotFoundError = "plugin_not_found_error" + // clientInternalError indicates that we attempted to start the plugin process, but failed + // for some reason. + clientInternalError = "client_internal_error" + + // successExitCode represents an exec plugin invocation that was successful. + successExitCode = 0 + // failureExitCode represents an exec plugin invocation that was not successful. This code is + // used in some failure modes (e.g., plugin not found, client internal error) so that someone + // can more easily monitor all unsuccessful invocations. + failureExitCode = 1 +) + type certificateExpirationTracker struct { mu sync.RWMutex m map[*Authenticator]time.Time @@ -58,3 +85,25 @@ func (c *certificateExpirationTracker) set(a *Authenticator, t time.Time) { c.metricSet(&earliest) } } + +// incrementCallsMetric increments a global metrics counter for the number of calls to an exec +// plugin, partitioned by exit code. The provided err should be the return value from +// exec.Cmd.Run(). +func incrementCallsMetric(err error) { + execExitError := &exec.ExitError{} + execError := &exec.Error{} + switch { + case err == nil: // Binary execution succeeded. + metrics.ExecPluginCalls.Increment(successExitCode, noError) + + case errors.As(err, &execExitError): // Binary execution failed (see "os/exec".Cmd.Run()). + metrics.ExecPluginCalls.Increment(execExitError.ExitCode(), pluginExecutionError) + + case errors.As(err, &execError): // Binary does not exist (see exec.Error). + metrics.ExecPluginCalls.Increment(failureExitCode, pluginNotFoundError) + + default: // We don't know about this error type. + klog.V(2).InfoS("unexpected exec plugin return error type", "type", reflect.TypeOf(err).String(), "err", err) + metrics.ExecPluginCalls.Increment(failureExitCode, clientInternalError) + } +} diff --git a/plugin/pkg/client/auth/exec/metrics_test.go b/plugin/pkg/client/auth/exec/metrics_test.go index dae90f5d..3c3c8f26 100644 --- a/plugin/pkg/client/auth/exec/metrics_test.go +++ b/plugin/pkg/client/auth/exec/metrics_test.go @@ -17,8 +17,14 @@ limitations under the License. package exec import ( + "fmt" "testing" "time" + + "github.com/google/go-cmp/cmp" + "k8s.io/client-go/pkg/apis/clientauthentication" + "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/client-go/tools/metrics" ) type mockExpiryGauge struct { @@ -94,3 +100,98 @@ func TestCertificateExpirationTracker(t *testing.T) { }) } } + +type mockCallsMetric struct { + exitCode int + errorType string +} + +type mockCallsMetricCounter struct { + calls []mockCallsMetric +} + +func (f *mockCallsMetricCounter) Increment(exitCode int, errorType string) { + f.calls = append(f.calls, mockCallsMetric{exitCode: exitCode, errorType: errorType}) +} + +func TestCallsMetric(t *testing.T) { + const ( + goodOutput = `{ + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1beta1", + "status": { + "token": "foo-bar" + } + }` + ) + + callsMetricCounter := &mockCallsMetricCounter{} + originalExecPluginCalls := metrics.ExecPluginCalls + t.Cleanup(func() { metrics.ExecPluginCalls = originalExecPluginCalls }) + metrics.ExecPluginCalls = callsMetricCounter + + exitCodes := []int{0, 1, 2, 0} + var wantCallsMetrics []mockCallsMetric + for _, exitCode := range exitCodes { + c := api.ExecConfig{ + Command: "./testdata/test-plugin.sh", + APIVersion: "client.authentication.k8s.io/v1beta1", + Env: []api.ExecEnvVar{ + {Name: "TEST_EXIT_CODE", Value: fmt.Sprintf("%d", exitCode)}, + {Name: "TEST_OUTPUT", Value: goodOutput}, + }, + } + + a, err := newAuthenticator(newCache(), &c, nil) + if err != nil { + t.Fatal(err) + } + + // Run refresh creds twice so that our test validates that the metrics are set correctly twice + // in a row with the same authenticator. + refreshCreds := func() { + if err := a.refreshCredsLocked(&clientauthentication.Response{}); (err == nil) != (exitCode == 0) { + if err != nil { + t.Fatalf("wanted no error, but got %q", err.Error()) + } else { + t.Fatal("wanted error, but got nil") + } + } + mockCallsMetric := mockCallsMetric{exitCode: exitCode, errorType: "no_error"} + if exitCode != 0 { + mockCallsMetric.errorType = "plugin_execution_error" + } + wantCallsMetrics = append(wantCallsMetrics, mockCallsMetric) + } + refreshCreds() + refreshCreds() + } + + // Run some iterations of the authenticator where the exec plugin fails to run to test special + // metric values. + refreshCreds := func(command string) { + c := api.ExecConfig{ + Command: "does not exist", + APIVersion: "client.authentication.k8s.io/v1beta1", + } + a, err := newAuthenticator(newCache(), &c, nil) + if err != nil { + t.Fatal(err) + } + if err := a.refreshCredsLocked(&clientauthentication.Response{}); err == nil { + t.Fatal("expected the authenticator to fail because the plugin does not exist") + } + wantCallsMetrics = append(wantCallsMetrics, mockCallsMetric{exitCode: 1, errorType: "plugin_not_found_error"}) + } + refreshCreds("does not exist without path slashes") + refreshCreds("./does/not/exist/with/relative/path") + refreshCreds("/does/not/exist/with/absolute/path") + + callsMetricComparer := cmp.Comparer(func(a, b mockCallsMetric) bool { + return a.exitCode == b.exitCode && a.errorType == b.errorType + }) + actuallCallsMetrics := callsMetricCounter.calls + if diff := cmp.Diff(wantCallsMetrics, actuallCallsMetrics, callsMetricComparer); diff != "" { + t.Fatalf("got unexpected metrics calls; -want, +got:\n%s", diff) + } +} diff --git a/tools/metrics/metrics.go b/tools/metrics/metrics.go index 5194026b..e3b8408d 100644 --- a/tools/metrics/metrics.go +++ b/tools/metrics/metrics.go @@ -46,6 +46,12 @@ type ResultMetric interface { Increment(code string, method string, host string) } +// CallsMetric counts calls that take place for a specific exec plugin. +type CallsMetric interface { + // Increment increments a counter per exitCode and callStatus. + Increment(exitCode int, callStatus string) +} + var ( // ClientCertExpiry is the expiry time of a client certificate ClientCertExpiry ExpiryMetric = noopExpiry{} @@ -57,6 +63,9 @@ var ( RateLimiterLatency LatencyMetric = noopLatency{} // RequestResult is the result metric that rest clients will update. RequestResult ResultMetric = noopResult{} + // ExecPluginCalls is the number of calls made to an exec plugin, partitioned by + // exit code and call status. + ExecPluginCalls CallsMetric = noopCalls{} ) // RegisterOpts contains all the metrics to register. Metrics may be nil. @@ -66,6 +75,7 @@ type RegisterOpts struct { RequestLatency LatencyMetric RateLimiterLatency LatencyMetric RequestResult ResultMetric + ExecPluginCalls CallsMetric } // Register registers metrics for the rest client to use. This can @@ -87,6 +97,9 @@ func Register(opts RegisterOpts) { if opts.RequestResult != nil { RequestResult = opts.RequestResult } + if opts.ExecPluginCalls != nil { + ExecPluginCalls = opts.ExecPluginCalls + } }) } @@ -105,3 +118,7 @@ func (noopLatency) Observe(string, url.URL, time.Duration) {} type noopResult struct{} func (noopResult) Increment(string, string, string) {} + +type noopCalls struct{} + +func (noopCalls) Increment(int, string) {}