Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K8s trace context for APIServer is incorrect #124073

Open
CasperLiu opened this issue Mar 27, 2024 · 3 comments · May be fixed by #124079
Open

K8s trace context for APIServer is incorrect #124073

CasperLiu opened this issue Mar 27, 2024 · 3 comments · May be fixed by #124079
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@CasperLiu
Copy link

CasperLiu commented Mar 27, 2024

What happened?

In Kubernetes release 1.28, the tracing structure for the API server is depicted as follows:

image

Within the API server's code structure, the spans "List(recursive=true) etcd3" and "SerializeObject" are expected to be nested under the "List" span. However, "SerializeObject" appears parallel to "List", which is not the intended behavior.

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go

func ListResource(r rest.Lister, rw rest.Watcher, scope *RequestScope, forceWatch bool, minRequestTimeout time.Duration) http.HandlerFunc {
	return func(w http.ResponseWriter, req *http.Request) {
		ctx := req.Context()
		// For performance tracking purposes.

                 // CasperLiu: this span is for high level - "List"
		ctx, span := tracing.Start(ctx, "List", traceFields(req)...)

		namespace, err := scope.Namer.Namespace(req)
		if err != nil {
			scope.err(err, w, req)
			return
		}

                 // omit...
		span.AddEvent("About to List from storage")
                 // CasperLiu: span inside below function is for low level - "List(recursive=true) etcd3"
		result, err := r.List(ctx, &opts)
		if err != nil {
			scope.err(err, w, req)
			return
		}
		span.AddEvent("Listing from storage done")
		defer span.AddEvent("Writing http response done", attribute.Int("count", meta.LenList(result)))
                 // CasperLiu: span inside below function is for low level - "SerializeObject"
		transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
	}
}

What did you expect to happen?

Within the API server's code structure, the spans "List(recursive=true) etcd3" and "SerializeObject" are expected to be nested under the "List" span.

How can we reproduce it (as minimally and precisely as possible)?

  1. Configure the Kubernetes API server to enable tracing and direct the emitted spans to a backend.
  2. Illustrate the resulting trace flow.
  3. Invoke a quorum read LIST request for Kubernetes resources; within the trace flow, you may identify instances of incorrect trace context.

Anything else we need to know?

No response

Kubernetes version

kuberenetes version:
1.28.3

Cloud provider

Alibaba Cloud Kubernetes (ACK)

OS version

Unrelated to the OS version

Install tools

No need

Container runtime (CRI) and version (if applicable)

Unrelated to the CRI version

Related plugins (CNI, CSI, ...) and versions (if applicable)

Unrelated to the plugins version

@CasperLiu CasperLiu added the kind/bug Categorizes issue or PR as related to a bug. label Mar 27, 2024
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 27, 2024
@pranav-pandey0804
Copy link

/sig instrumentation

@k8s-ci-robot k8s-ci-robot added sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 27, 2024
@CasperLiu
Copy link
Author

CasperLiu commented Mar 27, 2024

I have made a code change to resolve this issue, PR is as blow:
#124079

@CasperLiu CasperLiu changed the title K8s trace context is incorrect K8s trace context for APIServer is incorrect Mar 27, 2024
@CasperLiu CasperLiu linked a pull request Mar 27, 2024 that will close this issue
@dgrisonnet
Copy link
Member

/assign @CasperLiu
/triage accepted

@k8s-ci-robot 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 Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants