feat: make traces better (#5412)

* feat: make traces better

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* error

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

hack/main.go

@@ -20,10 +20,23 @@ var (
 )
 
 type arg struct {
-	Type       reflect.Type
+	reflect.Type
 	IsVariadic bool
 }
 
+func (a arg) IsError() bool {
+	return goType(a.Type) == "error"
+}
+
+type ret struct {
+	reflect.Type
+	IsLast bool
+}
+
+func (r ret) IsError() bool {
+	return goType(r.Type) == "error"
+}
+
 type operation struct {
 	Method reflect.Method
 }
@@ -32,6 +45,10 @@ func (o operation) HasContext() bool {
 	return o.Method.Type.NumIn() > 0 && goType(o.Method.Type.In(0)) == "context.Context"
 }
 
+func (o operation) HasError() bool {
+	return o.Method.Type.NumIn() > 0 && goType(o.Method.Type.In(o.Method.Type.NumIn()-1)) == "error"
+}
+
 type resource struct {
 	Method     reflect.Method
 	Type       reflect.Type
@@ -65,10 +82,13 @@ func getIns(in reflect.Method) []reflect.Type {
 	return out
 }
 
-func getOuts(in reflect.Method) []reflect.Type {
+func getOuts(in reflect.Method) []ret {
-	var out []reflect.Type
+	var out []ret
 	for i := 0; i < in.Type.NumOut(); i++ {
-		out = append(out, in.Type.Out(i))
+		out = append(out, ret{
+			Type:   in.Type.Out(i),
+			IsLast: i == in.Type.NumOut()-1,
+		})
 	}
 	return out
 }
@@ -164,10 +184,11 @@ func parseImports(cs clientset, packages ...string) []string {
 					}
 				}
 				for _, i := range getOuts(o.Method) {
+					pkg := i.PkgPath()
 					if i.Kind() == reflect.Pointer {
-						i = i.Elem()
+						i.Elem().PkgPath()
 					}
-					if i.PkgPath() != "" {
+					if pkg != "" {
 						imports.Insert(i.PkgPath())
 					}
 				}
@@ -200,7 +221,7 @@ func executeTemplate(tpl string, cs clientset, folder string, packages ...string
 				}
 				return out
 			},
-			"Returns": func(in reflect.Method) []reflect.Type {
+			"Returns": func(in reflect.Method) []ret {
 				return getOuts(in)
 			},
 			"Pkg": func(in string) string {
@@ -223,6 +244,7 @@ func executeTemplate(tpl string, cs clientset, folder string, packages ...string
 			panic(fmt.Sprintf("Failed to create file %s", path.Join(folder, file)))
 		}
 		if err := tmpl.Execute(f, map[string]interface{}{
+			"Folder":    folder,
 			"Clientset": cs,
 			"Packages":  parseImports(cs, packages...),
 		}); err != nil {
@@ -359,6 +381,7 @@ package client
 {{- $restPkg := Pkg "k8s.io/client-go/rest" }}
 {{- $tracingPkg := Pkg "github.com/kyverno/kyverno/pkg/tracing" }}
 {{- $attributePkg := Pkg "go.opentelemetry.io/otel/attribute" }}
+{{- $codesPkg := Pkg "go.opentelemetry.io/otel/codes" }}
 
 import (
 	{{- range $package := .Packages }}
@@ -452,8 +475,8 @@ func (c *wrapped{{ $client.Method.Name }}{{ $resource.Method.Name }}) {{ $operat
 	{{- if $operation.HasContext }}
 	ctx, span := {{ $tracingPkg }}.StartSpan(
 		arg0,
-		"{{ $client.Method.Name }}/{{ $resource.Method.Name }}",
+		{{ Quote $.Folder }},
-		{{ Quote $operation.Method.Name }},
+		"KUBE {{ $client.Method.Name }}/{{ $resource.Method.Name }}/{{ $operation.Method.Name }}",
 		{{ $attributePkg }}.String("client", {{ Quote $client.Method.Name }}),
 		{{ $attributePkg }}.String("resource", {{ Quote $resource.Method.Name }}),
 		{{ $attributePkg }}.String("kind", {{ Quote $resource.Kind }}),
@@ -461,7 +484,7 @@ func (c *wrapped{{ $client.Method.Name }}{{ $resource.Method.Name }}) {{ $operat
 	defer span.End()
 	arg0 = ctx
 	{{- end }}
-	return c.inner.{{ $operation.Method.Name }}(
+	{{ range $i, $ret := Returns $operation.Method }}ret{{ $i }}{{ if not $ret.IsLast -}},{{- end }} {{ end }} := c.inner.{{ $operation.Method.Name }}(
 		{{- range $i, $arg := Args $operation.Method -}}
 		{{- if $arg.IsVariadic -}}
 		arg{{ $i }}...,
@@ -470,6 +493,19 @@ func (c *wrapped{{ $client.Method.Name }}{{ $resource.Method.Name }}) {{ $operat
 		{{- end -}}
 		{{- end -}}
 	)
+	{{- if $operation.HasContext }}
+	{{- range $i, $ret := Returns $operation.Method }}
+	{{- if $ret.IsError }}
+	if ret{{ $i }} != nil {
+		span.RecordError(ret{{ $i }})
+		span.SetStatus({{ $codesPkg }}.Ok, ret{{ $i }}.Error())
+	}
+	{{- end }}
+	{{- end }}
+	{{- end }}
+	return	{{ range $i, $ret := Returns $operation.Method -}}
+	ret{{ $i }}{{ if not $ret.IsLast -}},{{- end }}
+	{{- end }}
 }
 {{- end }}
 {{- end }}
@@ -503,6 +539,7 @@ func main() {
 		"context",
 		"github.com/kyverno/kyverno/pkg/tracing",
 		"go.opentelemetry.io/otel/attribute",
+		"go.opentelemetry.io/otel/codes",
 		"k8s.io/client-go/discovery",
 		"k8s.io/client-go/rest",
 	)
@@ -512,6 +549,7 @@ func main() {
 		"context",
 		"github.com/kyverno/kyverno/pkg/tracing",
 		"go.opentelemetry.io/otel/attribute",
+		"go.opentelemetry.io/otel/codes",
 		"k8s.io/client-go/discovery",
 		"k8s.io/client-go/rest",
 	)

pkg/webhooks/handlers/admission.go

@@ -1,7 +1,6 @@
 package handlers
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -10,15 +9,9 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/kyverno/kyverno/pkg/tracing"
-	"go.opentelemetry.io/otel/attribute"
 	admissionv1 "k8s.io/api/admission/v1"
 )
 
-type (
-	AdmissionHandler func(context.Context, logr.Logger, *admissionv1.AdmissionRequest, time.Time) *admissionv1.AdmissionResponse
-	HttpHandler      func(http.ResponseWriter, *http.Request)
-)
-
 func (h AdmissionHandler) WithAdmission(logger logr.Logger) HttpHandler {
 	return withAdmission(logger, h)
 }
@@ -65,13 +58,9 @@ func withAdmission(logger logr.Logger, inner AdmissionHandler) HttpHandler {
 		// start span from request context
 		ctx, span := tracing.StartSpan(
 			request.Context(),
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			string(admissionReview.Request.Operation),
+			fmt.Sprintf("ADMISSION %s %s", admissionReview.Request.Operation, admissionReview.Request.Kind),
-			attribute.String("kind", admissionReview.Request.Kind.Kind),
+			admissionRequestAttributes(admissionReview.Request)...,
-			attribute.String("namespace", admissionReview.Request.Namespace),
-			attribute.String("name", admissionReview.Request.Name),
-			attribute.String("operation", string(admissionReview.Request.Operation)),
-			attribute.String("uid", string(admissionReview.Request.UID)),
 		)
 		defer span.End()
 		adminssionResponse := inner(ctx, logger, admissionReview.Request, startTime)

pkg/webhooks/handlers/dump.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
@@ -107,13 +108,14 @@ func withDump(inner AdmissionHandler) AdmissionHandler {
 	return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 		return tracing.Span1(
 			ctx,
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			"dump",
+			fmt.Sprintf("DUMP %s %s", request.Operation, request.Kind),
 			func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse {
 				response := inner(ctx, logger, request, startTime)
 				dumpPayload(logger, request, response)
 				return response
 			},
+			trace.WithAttributes(admissionRequestAttributes(request)...),
 		)
 	}
 }

pkg/webhooks/handlers/filter.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -19,14 +20,15 @@ func withFilter(c config.Configuration, inner AdmissionHandler) AdmissionHandler
 	return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 		return tracing.Span1(
 			ctx,
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			"filter",
+			fmt.Sprintf("FILTER %s %s", request.Operation, request.Kind),
 			func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse {
 				if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) {
 					return nil
 				}
 				return inner(ctx, logger, request, startTime)
 			},
+			trace.WithAttributes(admissionRequestAttributes(request)...),
 		)
 	}
 }

pkg/webhooks/handlers/metrics.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -21,13 +22,14 @@ func withMetrics(metricsConfig *metrics.MetricsConfig, inner AdmissionHandler) A
 	return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 		return tracing.Span1(
 			ctx,
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			"metrics",
+			fmt.Sprintf("METRICS %s %s", request.Operation, request.Kind),
 			func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse {
 				defer admissionReviewDuration.Process(metricsConfig, request, int64(time.Since(startTime)))
 				admissionRequests.Process(metricsConfig, request)
 				return inner(ctx, logger, request, startTime)
 			},
+			trace.WithAttributes(admissionRequestAttributes(request)...),
 		)
 	}
 }

pkg/webhooks/handlers/protect.go

@@ -28,8 +28,8 @@ func withProtection(inner AdmissionHandler) AdmissionHandler {
 	return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 		return tracing.Span1(
 			ctx,
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			"protect",
+			fmt.Sprintf("PROTECT %s %s", request.Operation, request.Kind),
 			func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse {
 				newResource, oldResource, err := utils.ExtractResources(nil, request)
 				if err != nil {
@@ -47,6 +47,7 @@ func withProtection(inner AdmissionHandler) AdmissionHandler {
 				}
 				return inner(ctx, logger, request, startTime)
 			},
+			trace.WithAttributes(admissionRequestAttributes(request)...),
 		)
 	}
 }

pkg/webhooks/handlers/trace.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	"github.com/kyverno/kyverno/pkg/tracing"
@@ -17,8 +18,8 @@ func withTrace(inner HttpHandler) HttpHandler {
 	return func(writer http.ResponseWriter, request *http.Request) {
 		tracing.Span(
 			request.Context(),
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			request.URL.Path,
+			fmt.Sprintf("HTTP %s %s", request.Method, request.URL.Path),
 			func(ctx context.Context, span trace.Span) {
 				inner(writer, request.WithContext(ctx))
 			},

pkg/webhooks/handlers/types.go

@@ -0,0 +1,15 @@
+package handlers
+
+import (
+	"context"
+	"net/http"
+	"time"
+
+	"github.com/go-logr/logr"
+	admissionv1 "k8s.io/api/admission/v1"
+)
+
+type (
+	AdmissionHandler func(context.Context, logr.Logger, *admissionv1.AdmissionRequest, time.Time) *admissionv1.AdmissionResponse
+	HttpHandler      func(http.ResponseWriter, *http.Request)
+)

pkg/webhooks/handlers/utils.go

@@ -0,0 +1,16 @@
+package handlers
+
+import (
+	"go.opentelemetry.io/otel/attribute"
+	admissionv1 "k8s.io/api/admission/v1"
+)
+
+func admissionRequestAttributes(request *admissionv1.AdmissionRequest) []attribute.KeyValue {
+	return []attribute.KeyValue{
+		attribute.String("kind", request.Kind.Kind),
+		attribute.String("namespace", request.Namespace),
+		attribute.String("name", request.Name),
+		attribute.String("operation", string(request.Operation)),
+		attribute.String("uid", string(request.UID)),
+	}
+}

pkg/webhooks/handlers/verify.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -17,8 +18,8 @@ func Verify() AdmissionHandler {
 	return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 		return tracing.Span1(
 			ctx,
-			"admission_webhook_operations",
+			"webhooks/handlers",
-			"verify",
+			fmt.Sprintf("VERIFY %s %s", request.Operation, request.Kind),
 			func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse {
 				if request.Name != "kyverno-health" || request.Namespace != config.KyvernoNamespace() {
 					return admissionutils.ResponseSuccess()
@@ -31,6 +32,7 @@ func Verify() AdmissionHandler {
 				}
 				return admissionutils.MutationResponse(bytes)
 			},
+			trace.WithAttributes(admissionRequestAttributes(request)...),
 		)
 	}
 }