From 84b9fbbdefa3f0bcfb1c4787093aa7840079b7ce Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Thu, 29 Feb 2024 19:03:43 +0800 Subject: [PATCH 1/4] rename apiserver trace span to http server guidelines Signed-off-by: Ziqi Zhao --- .../apiserver/pkg/endpoints/filters/traces.go | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go index 1ecf59d4543..b6eafb1fab7 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go @@ -22,6 +22,7 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" "go.opentelemetry.io/otel/trace" + "k8s.io/apiserver/pkg/endpoints/request" tracing "k8s.io/component-base/tracing" ) @@ -32,6 +33,14 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { otelhttp.WithPropagators(tracing.Propagators()), otelhttp.WithPublicEndpoint(), otelhttp.WithTracerProvider(tp), + otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string { + ctx := r.Context() + info, exist := request.RequestInfoFrom(ctx) + if !exist { + return "KubernetesAPI" + } + return getSpanNameFromRequestInfo(info) + }), } wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Add the http.target attribute to the otelhttp span @@ -45,3 +54,22 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { // See https://github.com/open-telemetry/opentelemetry-go/tree/main/example/passthrough return otelhttp.NewHandler(wrappedHandler, "KubernetesAPI", opts...) } + +func getSpanNameFromRequestInfo(info *request.RequestInfo) string { + spanName := "/" + info.APIPrefix + if info.APIGroup != "" { + spanName += "/" + info.APIGroup + } + spanName += "/" + info.APIVersion + if info.Namespace != "" { + spanName += "/namespaces/{:namespace}" + } + spanName += "/" + info.Resource + if info.Name != "" { + spanName += "/" + "{:id}" + } + if info.Subresource != "" { + spanName += "/" + info.Subresource + } + return spanName +} From 02154293c76a0ea54293c82236c9025b96ea0125 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Tue, 12 Mar 2024 22:49:38 +0800 Subject: [PATCH 2/4] change the integration test Signed-off-by: Ziqi Zhao --- .../k8s.io/apiserver/pkg/endpoints/filters/traces.go | 6 +++++- test/integration/apiserver/tracing/tracing_test.go | 12 ++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go index b6eafb1fab7..b56dda2f87b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go @@ -56,6 +56,10 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { } func getSpanNameFromRequestInfo(info *request.RequestInfo) string { + if !info.IsResourceRequest { + return info.Path + } + spanName := "/" + info.APIPrefix if info.APIGroup != "" { spanName += "/" + info.APIGroup @@ -66,7 +70,7 @@ func getSpanNameFromRequestInfo(info *request.RequestInfo) string { } spanName += "/" + info.Resource if info.Name != "" { - spanName += "/" + "{:id}" + spanName += "/" + "{:name}" } if info.Subresource != "" { spanName += "/" + info.Subresource diff --git a/test/integration/apiserver/tracing/tracing_test.go b/test/integration/apiserver/tracing/tracing_test.go index c3cdc15bb5e..f7bca4798a8 100644 --- a/test/integration/apiserver/tracing/tracing_test.go +++ b/test/integration/apiserver/tracing/tracing_test.go @@ -309,7 +309,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -428,7 +428,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -518,7 +518,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -636,7 +636,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -780,7 +780,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -901,7 +901,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "KubernetesAPI", + name: "/api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") From 1aeb0ba314016f2a2cd94b0450ba097c2b165e5d Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Wed, 13 Mar 2024 13:25:36 +0800 Subject: [PATCH 3/4] add http method to span name Signed-off-by: Ziqi Zhao --- .../k8s.io/apiserver/pkg/endpoints/filters/traces.go | 8 ++++---- test/integration/apiserver/tracing/tracing_test.go | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go index b56dda2f87b..5a06d34f5b3 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go @@ -39,7 +39,7 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { if !exist { return "KubernetesAPI" } - return getSpanNameFromRequestInfo(info) + return getSpanNameFromRequestInfo(info, r) }), } wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -55,9 +55,9 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { return otelhttp.NewHandler(wrappedHandler, "KubernetesAPI", opts...) } -func getSpanNameFromRequestInfo(info *request.RequestInfo) string { +func getSpanNameFromRequestInfo(info *request.RequestInfo, r *http.Request) string { if !info.IsResourceRequest { - return info.Path + return r.Method + " " + info.Path } spanName := "/" + info.APIPrefix @@ -75,5 +75,5 @@ func getSpanNameFromRequestInfo(info *request.RequestInfo) string { if info.Subresource != "" { spanName += "/" + info.Subresource } - return spanName + return r.Method + " " + spanName } diff --git a/test/integration/apiserver/tracing/tracing_test.go b/test/integration/apiserver/tracing/tracing_test.go index f7bca4798a8..60b49c7ed15 100644 --- a/test/integration/apiserver/tracing/tracing_test.go +++ b/test/integration/apiserver/tracing/tracing_test.go @@ -309,7 +309,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes", + name: "POST /api/v1/nodes", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -428,7 +428,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes/{:name}", + name: "GET /api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -518,7 +518,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes", + name: "GET /api/v1/nodes", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -636,7 +636,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes/{:name}", + name: "PUT /api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -780,7 +780,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes/{:name}", + name: "PATCH /api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") @@ -901,7 +901,7 @@ endpoint: %s`, listener.Addr().String())), os.FileMode(0755)); err != nil { }, expectedTrace: []*spanExpectation{ { - name: "/api/v1/nodes/{:name}", + name: "DELETE /api/v1/nodes/{:name}", attributes: map[string]func(*commonv1.AnyValue) bool{ "http.user_agent": func(v *commonv1.AnyValue) bool { return strings.HasPrefix(v.GetStringValue(), "tracing.test") From 91af1145bf7b0e18a6b520a78875a1db6db29d96 Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Fri, 15 Mar 2024 09:42:42 +0800 Subject: [PATCH 4/4] fix for comments to ignore the request without request info Signed-off-by: Ziqi Zhao --- .../src/k8s.io/apiserver/pkg/endpoints/filters/traces.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go index 5a06d34f5b3..6e36ffec862 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go @@ -36,8 +36,8 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string { ctx := r.Context() info, exist := request.RequestInfoFrom(ctx) - if !exist { - return "KubernetesAPI" + if !exist || !info.IsResourceRequest { + return r.Method } return getSpanNameFromRequestInfo(info, r) }), @@ -56,10 +56,6 @@ func WithTracing(handler http.Handler, tp trace.TracerProvider) http.Handler { } func getSpanNameFromRequestInfo(info *request.RequestInfo, r *http.Request) string { - if !info.IsResourceRequest { - return r.Method + " " + info.Path - } - spanName := "/" + info.APIPrefix if info.APIGroup != "" { spanName += "/" + info.APIGroup