Add otel tracing instrumentation #506

sallyom · 2025-12-05T13:20:16Z

This PR satisfies the llm-d distributed tracing proposal llm-d/llm-d#119 implementation plan.

github-actions · 2025-12-05T13:20:27Z

🚨 Unsigned commits detected! Please sign your commits.

For instructions on how to set up GPG/SSH signing and verify your commits,
please see GitHub Documentation.

github-actions · 2025-12-29T01:38:40Z

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

sallyom · 2026-01-07T04:27:41Z

The GAIE entry request span PR is kubernetes-sigs/gateway-api-inference-extension#2057

wseaton · 2026-01-07T13:58:29Z

pkg/plugins/scorer/precise_prefix_cache.go

+	if request != nil && request.TargetModel != "" {
+		span.SetAttributes(attribute.String("gen_ai.request.model", request.TargetModel))
+	}
+	if request != nil && request.RequestId != "" {
+		span.SetAttributes(attribute.String("gen_ai.request.id", request.RequestId))
+	}
+


nit:

Suggested change

if request != nil && request.TargetModel != "" {

span.SetAttributes(attribute.String("gen_ai.request.model", request.TargetModel))

}

if request != nil && request.RequestId != "" {

span.SetAttributes(attribute.String("gen_ai.request.id", request.RequestId))

}

if request != nil {

if request.TargetModel != "" {

span.SetAttributes(attribute.String("gen_ai.request.model", request.TargetModel))

}

if request.RequestId != "" {

span.SetAttributes(attribute.String("gen_ai.request.id", request.RequestId))

}

}

updated, but I went with a check & handle nil/error early instead

Dockerfile.localdev.epp

zdtsw · 2026-01-07T13:27:03Z

Dockerfile.localdev.epp

+COPY llm-d-inference-scheduler/go.mod go.mod
+COPY llm-d-inference-scheduler/go.sum go.sum
+
+# Copy the go source
+COPY llm-d-inference-scheduler/cmd/ cmd/
+COPY llm-d-inference-scheduler/pkg/ pkg/
+COPY llm-d-inference-scheduler/test/ test/


are the "src" part correct here? or the intention is to build image from one level up

this Dockerfile I was mostly using in order to build & test from local sources- I'll clean it up!

I will add the localdev Dockerfile in a follow-up PR if others want it, but I've removed it from this PR

zdtsw · 2026-01-07T13:46:22Z

cmd/epp/main.go

+	tracer := telemetry.Tracer()
+	ctx, span := tracer.Start(ctx, "llm_d.epp.startup")
+	span.SetAttributes(
+		attribute.String("component", "llm-d-inference-scheduler"),


should we use "epp" than "llm-d-inference-scheduler?

zdtsw · 2026-01-07T13:50:42Z

cmd/pd-sidecar/main.go

+		attribute.String("component", "llm-d-pd-proxy"),
+		attribute.String("operation", "startup"),
+	)
+	defer span.End()


should be a "defer" on the "epp" as well?

zdtsw · 2026-01-07T13:56:26Z

pkg/telemetry/tracing.go

+	logger := log.FromContext(ctx)
+
+	// Get service name from environment, fallback to default
+	svcName := os.Getenv("OTEL_SERVICE_NAME")


seems svcName is not really needed?
either serviceName set to os.Getenv("OTEL_SERVICE_NAME") or use defaultServiceName

fixed this, ty!

zdtsw · 2026-01-07T14:12:16Z

pkg/sidecar/proxy/chat_completions.go

 func (s *Server) chatCompletionsHandler(w http.ResponseWriter, r *http.Request) {
+	requestStart := time.Now()
+	tracer := telemetry.Tracer()
+	ctx, span := tracer.Start(r.Context(), "llm_d.pd_proxy.request",


read the proposal part seem iit has decided to use llm_d.pd_proxy.

not for this PR, but should we change the folder to pkg/pd/proxy than pkg/sidecar/proxy for better naming?

Down the road we may have the sidecar proxy handle more than P/D disagg so I would suggest not renaming.

zdtsw · 2026-01-07T14:16:43Z

Dockerfile.localdev.epp

@@ -0,0 +1,110 @@
+# Build Stage: using Go 1.25 image


is this intended to check-in in this PR?

probably not - unless it's useful and people want it :)

I will add this local-build Dockerfile in a follow-up PR if people want it - I removed it from this PR

zdtsw · 2026-01-07T14:21:47Z

pkg/telemetry/tracing.go

+func stripScheme(endpoint string) string {
+	endpoint = strings.TrimPrefix(endpoint, "http://")
+	endpoint = strings.TrimPrefix(endpoint, "https://")
+	return endpoint
+}


looks like a common func, should it be moved to pkg/common/common.go ?

sallyom · 2026-01-17T01:12:07Z

The GAIE entry request span PR is kubernetes-sigs/gateway-api-inference-extension#2057

The GAIE PR merged!

zdtsw · 2026-01-17T10:43:30Z

cmd/epp/main.go

+			if err := shutdownTracing(ctx); err != nil {
+				ctrl.Log.Error(err, "Failed to shutdown tracing")
+			}
+		}


yeah, this is valid.
we could wrap up the code in a function.
and from main to call this function.

func main() { os.Exit(run()) } func run() int { // the current logic // move os.Exit(1) to return 1 }

then both defer and os.Exit(1) should work.

I agree but I'd rather leave that to a follow-up - since it's not related so much to tracing and I'm already out of my neighborhood here in this repo :) I'm trying to keep my changes as minimal as can be.

Add centralized telemetry package and custom spans following the llm-d distributed tracing proposal. Co-Authored-By: Claude <[email protected]> Signed-off-by: sallyom <[email protected]>

Signed-off-by: sallyom <[email protected]>

elevran · 2026-01-26T08:21:49Z

@sallyom happy to review and help get it merged. Could you please

ensure all commits are signed; and
fix linter error

elevran · 2026-01-27T08:43:39Z

cmd/epp/main.go

+		ctrl.Log.Error(err, "Failed to initialize tracing")
+	}
+
+	// Add startup span to verify tracing is working


Q: how does the below verify that tracing is working? Is there an expected error returned anywhere?

elevran · 2026-01-27T09:04:14Z

pkg/common/common.go

 	DataParallelPodHeader = "x-data-parallel-host-port"
 )
+
+// StripScheme removes http:// or https:// prefix from endpoint URL


nit: This could be improved with url.Parse() to better handle corner cases?

elevran · 2026-01-27T09:04:39Z

pkg/common/common_test.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2025 The llm-d Authors.


elevran · 2026-01-27T09:06:50Z

pkg/common/common_test.go

+func TestStripScheme(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string


nit: additional test cases that may make sense?

//192.168.1.1:80

HTTP://localhost:4317

:9090

elevran · 2026-01-27T09:09:27Z

pkg/plugins/pre-request/pd_prerequest.go

-func (p *PrefillHeaderHandler) PreRequest(_ context.Context, request *types.LLMRequest, schedulingResult *types.SchedulingResult) {
+func (p *PrefillHeaderHandler) PreRequest(ctx context.Context, request *types.LLMRequest, schedulingResult *types.SchedulingResult) {
+	tracer := telemetry.Tracer()
+	_, span := tracer.Start(ctx, "llm_d.epp.prerequest.pd_disaggregation",


nit: consider making consistent use of llm_d.epp (or llm-d.epp) in all span names (e.g., the "startup" span in main.go)?

elevran · 2026-01-27T09:10:51Z

pkg/plugins/pre-request/pd_prerequest.go

 	prefillProfileRunResult, exists := schedulingResult.ProfileResults[p.prefillProfile]
 	if !exists {
+		span.SetAttributes(
+			attribute.Bool("llm_d.epp.pd.disaggregation_enabled", false),


nit: it is enabled, just not used for this request

elevran · 2026-01-27T09:13:03Z

pkg/plugins/profile/pd_profile_handler.go

 	profileResults map[string]*types.ProfileRunResult) map[string]*framework.SchedulerProfile {
+	// Start tracing span for profile picking operation
+	tracer := telemetry.Tracer()
+	ctx, span := tracer.Start(ctx, "llm_d.epp.profile_handler.pick",


nit: do you want to span to explicitly state this is a P/D profile handler Pick() function? Missing P/D designation - all profile handler plugins have a Pick function...

elevran · 2026-01-27T09:17:17Z

pkg/sidecar/proxy/chat_completions.go

 func (s *Server) chatCompletionsHandler(w http.ResponseWriter, r *http.Request) {
+	requestStart := time.Now()
+	tracer := telemetry.Tracer()
+	ctx, span := tracer.Start(r.Context(), "llm_d.pd_proxy.request",


Down the road we may have the sidecar proxy handle more than P/D disagg so I would suggest not renaming.

github-project-automation bot added this to llm-d-inference-scheduler Dec 5, 2025

sallyom force-pushed the tracing branch from 662eba9 to 60d4150 Compare December 7, 2025 15:24

github-actions bot added the lifecycle/stale label Dec 29, 2025

github-actions bot added lifecycle/rotten and removed lifecycle/stale labels Jan 5, 2026

sallyom force-pushed the tracing branch 4 times, most recently from 9c56950 to c55c5a6 Compare January 5, 2026 20:14

github-actions bot removed the lifecycle/rotten label Jan 6, 2026

sallyom force-pushed the tracing branch 3 times, most recently from 884c096 to 8c4a03d Compare January 7, 2026 04:20

sallyom changed the title ~~[WIP] Tracing~~ Add otel tracing instrumentation Jan 7, 2026

sallyom force-pushed the tracing branch 7 times, most recently from 24dc5b9 to 15eb137 Compare January 7, 2026 05:44

wseaton reviewed Jan 7, 2026

View reviewed changes

zdtsw reviewed Jan 7, 2026

View reviewed changes

sallyom force-pushed the tracing branch from 15eb137 to ff87a59 Compare January 15, 2026 02:10

github-actions bot requested review from elevran and nilig January 15, 2026 02:11

sallyom force-pushed the tracing branch from ff87a59 to fca8980 Compare January 15, 2026 02:45

github-actions bot requested a review from kfswain January 15, 2026 02:45

sallyom force-pushed the tracing branch from fca8980 to ec31758 Compare January 17, 2026 01:08

github-actions bot requested a review from nirrozenbaum January 17, 2026 01:09

sallyom force-pushed the tracing branch from ec31758 to 06e9821 Compare January 17, 2026 01:24

github-actions bot requested a review from shmuelk January 17, 2026 01:24

zdtsw reviewed Jan 17, 2026

View reviewed changes

sallyom marked this pull request as ready for review January 20, 2026 15:51

sallyom force-pushed the tracing branch from 06e9821 to f7bcc9d Compare January 21, 2026 01:20

elevran added this to the v0.6 milestone Jan 22, 2026

sallyom and others added 3 commits January 23, 2026 14:40

Add opentelemetry tracing

d845ae8

Add centralized telemetry package and custom spans following the llm-d distributed tracing proposal. Co-Authored-By: Claude <[email protected]> Signed-off-by: sallyom <[email protected]>

update Dockerfile.sidecar

5b3bd44

Signed-off-by: sallyom <[email protected]>

tracing: remove extra success result

0612065

Signed-off-by: sallyom <[email protected]>

sallyom force-pushed the tracing branch from f7bcc9d to 0612065 Compare January 23, 2026 19:43

elevran reviewed Jan 27, 2026

View reviewed changes

Add otel tracing instrumentation #506

Are you sure you want to change the base?

Add otel tracing instrumentation #506

Uh oh!

Conversation

sallyom commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 5, 2025

Uh oh!

github-actions bot commented Dec 29, 2025

Uh oh!

sallyom commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sallyom commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elevran commented Jan 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

sallyom commented Dec 5, 2025 •

edited

Loading

sallyom commented Jan 17, 2026 •

edited

Loading