Merge pull request #98892 from ankeesler/exec-plugin-metrics

exec credential provider: add rest_client_exec_plugin_call_total metric

Kubernetes-commit: cd54b1931d015df7c1609043d81b1f8308f2187d

Godeps/Godeps.json

@@ -472,7 +472,7 @@
 		},
 		{
 			"ImportPath": "k8s.io/api",
-			"Rev": "c218b228ac09"
+			"Rev": "0d975ab4576f"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery",

go.mod

@@ -27,7 +27,7 @@ require (
 	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
 	golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d
 	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
-	k8s.io/api v0.0.0-20210304012009-c218b228ac09
+	k8s.io/api v0.0.0-20210304082812-0d975ab4576f
 	k8s.io/apimachinery v0.0.0-20210303224021-086982076e5b
 	k8s.io/klog/v2 v2.5.0
 	k8s.io/utils v0.0.0-20201110183641-67b214c5f920
@@ -35,6 +35,6 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20210304012009-c218b228ac09
+	k8s.io/api => k8s.io/api v0.0.0-20210304082812-0d975ab4576f
 	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210303224021-086982076e5b
 )

go.sum

@@ -427,7 +427,7 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20210304012009-c218b228ac09/go.mod h1:YspOqQmF4OXdACGAs03nGPxRrWe/nIKAS3Cwch9YyFk=
+k8s.io/api v0.0.0-20210304082812-0d975ab4576f/go.mod h1:YspOqQmF4OXdACGAs03nGPxRrWe/nIKAS3Cwch9YyFk=
 k8s.io/apimachinery v0.0.0-20210303224021-086982076e5b/go.mod h1:+s3G/nGQJY9oe1CFOXRrb9QkXTIEgTnFtF8GeKZIgOg=
 k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=

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)
 	}
 

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)
+	}
+}

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)
+	}
+}

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) {}