Trace inbound Nexus HTTP requests by stephanos · Pull Request #11560 · temporalio/temporal

stephanos · 2026-08-13T20:27:13Z

What changed?

Wrapped the frontend Nexus dispatch routes with the shared OpenTelemetry HTTP handler.

Why?

Nexus HTTP requests need an inbound server span to connect the caller trace.

How did you test it?

github-actions · 2026-08-15T00:35:54Z

Claude finished @stephanos's task in 5m 51s —— View job

Reviewed 5 files, 4 findings.

small — The inbound Nexus completion callback route is left uninstrumented.

nexusCompletionHTTPHandler.RegisterRoutes (service/frontend/nexus_completion_http_handler.go:466-471) registers RouteCompletionCallback and PathCompletionCallbackNoIdentifier with bare HandlerFunc. Those are inbound Nexus HTTP requests too — an external handler POSTing an async operation completion back to Temporal — and #11559 already instrumented the outbound side of that same callback. As it stands, a completion arriving with a traceparent produces no server span, so the caller's trace stops at the frontend boundary for exactly the leg the callback tracing work was building toward.

If leaving them out is deliberate scoping, worth saying so in the PR description, since the title reads as covering all inbound Nexus requests.

Suggestion: wrap both completion routes with h.httpHandlerWrapper.Wrap(...) the same way the dispatch routes are, or note the follow-up.
· branch chrsmith/nexus-http-server-tracing

stephanos · 2026-08-15T01:05:44Z

 	)
 }
-
-func getDispatchByEndpointURL(address string, endpoint string) string {


Made this a method on the test env instead.

It looks like all the callers touched passed env.Namespace().String() as the first parameter. Is that always the case? And if so, can we simplify the method to just be dispatchByTaskQueueURL(...)? (And get the test environment's namespace from within the function, just to elide one of the parameters?)

Good observation! I'll make the simplification.

(there are still 2 callers that pass explicit namespaces, I'll leave the helper for that but default to the env's ns otherwise)

github-actions · 2026-08-15T01:22:35Z

Claude finished @stephanos's task in 6m 54s —— View job

Reviewed 8 files, 4 findings.

small — The inbound Nexus completion callback route is still uninstrumented.

nexusCompletionHTTPHandler.RegisterRoutes (service/frontend/nexus_completion_http_handler.go:465-473) registers RouteCompletionCallback and PathCompletionCallbackNoIdentifier with bare HandlerFunc. Those are inbound Nexus HTTP requests too — an external handler POSTing an async operation completion back to Temporal — and #11559 already instrumented the outbound side of that same callback. A completion arriving with a traceparent still produces no server span, so the caller's trace stops at the frontend boundary for exactly the leg the callback tracing work was building toward.

Re-raising because it is unaddressed, not to relitigate: if the scoping is deliberate, saying so in the PR description is enough, since the title reads as covering all inbound Nexus requests.

Suggestion: wrap both completion routes with h.httpHandlerWrapper.Wrap(...) the same way the dispatch routes now are, or note the follow-up.
· branch chrsmith/nexus-http-server-tracing

stephanos · 2026-08-15T01:23:00Z

-	exporter := tracetest.NewInMemoryExporter()
-	env := s.newTestEnv(exporter)
+// Verifies asynchronous start and cancellation connect real History client and Frontend server spans.
+func (s *NexusOTELSuite) TestOperation() {


Re-did these to be more end-to-end.

chrsmith

Lots of minor nitpicks and suggestions. Everything seems right, but there's enough moving parts that if we can think of ways to make the code easier to read/reason about it might be helpful down the line.

chrsmith · 2026-08-15T17:13:50Z

 }

+// HTTPHandlerWrapper wraps HTTP handlers with the service's tracing configuration.
+type HTTPHandlerWrapper func(http.Handler, string) http.Handler


Nit: I don't know if there is a better name, but HTTPHandlerWrapper sounds a little too generic. Like, it's a wrapper, sure. But the fact that it takes an http.Handler and a string seems odd. (Like, maybe "Named HTTP Handler" or something?)

If nothing comes to mind, feel free to keep it as-is. #namingishard

Yeah I can see what you mean; it's meant to mirror HTTPClientTransportWrapper. But that one is simpler/clearer. Ultimately it does two things: wrap the http.Handler and give the span a name. 🤔

Maybe?

HTTPClientTransportInstrumenter

HTTPServerHandlerInstrumenter

chrsmith · 2026-08-15T17:18:05Z

+		Handler(h.httpHandlerWrapper.Wrap(http.HandlerFunc(h.dispatchNexusTaskByNamespaceAndTaskQueue), strings.TrimPrefix(configs.DispatchNexusTaskByNamespaceAndTaskQueueAPIName, "/")))
 	r.PathPrefix("/" + commonnexus.RouteDispatchNexusTaskByEndpoint.Representation() + "/").
-		HandlerFunc(h.dispatchNexusTaskByEndpoint)
+		Handler(h.httpHandlerWrapper.Wrap(http.HandlerFunc(h.dispatchNexusTaskByEndpoint), strings.TrimPrefix(configs.DispatchNexusTaskByEndpointAPIName, "/")))


Nit: The AI is a fan of having really dense, complex express expressions on a single line. And I'd argue we should be fearless about introducing new local variables when it helps with readability.

e.g. if you think the following is a net-improvement to readability, consider the following:

dispatchTaskByEndpointHandlerFn := http.HandlerFunc(h.dispatchNexusTaskByEndpoint) dispatchEndpointName := strings.TrimPrefix(configs.DispatchNexusTaskByEndpointAPIName, "/") r.PathPrefix("/" + commonnexus.RouteDispatchNexusTaskByEndpoint.Representation() + "/"). Handler(h.httpHandlerWrapper.Wrap(dispatchTaskHandlerFn, dispatchEndpointName))

That's still pretty hard to reason about, IMHO. But at least it's a little easier to see since there aren't so many nested expressions.

I think I nudged it to be denser on that one actually; thinking that those routing expressions are best left compact/inline. But I can see how it's a lot. Let me extract some.

I added a register helper now.

chrsmith · 2026-08-15T17:20:07Z

 	)
 }
-
-func getDispatchByEndpointURL(address string, endpoint string) string {


It looks like all the callers touched passed env.Namespace().String() as the first parameter. Is that always the case? And if so, can we simplify the method to just be dispatchByTaskQueueURL(...)? (And get the test environment's namespace from within the function, just to elide one of the parameters?)

chrsmith · 2026-08-15T17:23:20Z

+		s.T(),
+		handlerEnv.dispatchByEndpointURL(handlerWorkerEndpoint.Id),
+	)
+	operationPathSuffix := "/" + service.Name + "/" + operation.Name()


Nit: Instead of repeated string concatenation, fmt.Sprintf("/%s/%s", ...) might be easier to read. (Or not.)

isn't the (less efficient) + approach more readable actually (no backtracking when reading)? 😬 🙈

Yeah, you are right. Sorry about that. That was just code review muscle memory 😅

I think a case could be made if there were more concatenations and what not. But yeah, the "/" + ... + "/" + ... is probably as clear as it could be.

chrsmith · 2026-08-15T17:26:03Z

-		RequestId:              tv.RequestID(),
-		ScheduleToCloseTimeout: durationpb.New(time.Minute),
+	requestHeaders := nexus.Header{
+		"traceparent": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",


I don't see us validating that this header was passed through anywhere. Do we want to have a test that the header is found unmodified on the handler-side? (i.e. it wasn't stripped by the Temporal x Nexus machineary, and made its way to the other side?) Or is that outside the scope of what we want to test here?

I looked at this more closely. requireExportedServerSpan verifies that this traceparent is extracted by the frontend. And nexus_api_test.go verifies that headers are forwarded already. I've added a comment to mention that.

chrsmith · 2026-08-15T17:30:16Z

+	)
+}
+
+func (s *NexusOTELSuite) requireExportedNexusHTTPSpanPair(


Can you please add a doc comment for what this is doing? From the logic it isn't immediately clear.

We're looping through the handlerSpans looking for a specific tag, and then making sure to find the child span in the client spans?

Maybe assertSpanPropagated? Or assertSpanParented? And maybe take in the parent span name as another parameter? That would make it a little clearer what's going on.

Alright, that method is gone now. I made some sweeping changes in the test to make the assertions much more declarative.

chrsmith · 2026-08-15T17:31:04Z

+	handlerExporter *tracetest.InMemoryExporter,
+	pathSuffix string,
+) {
+	s.Await(func(s *NexusOTELSuite) {


Nit: Maybe call s.T().Helper() before this? So that error messages would be attributed to the callsite, and not here?

## What changed? Wrapped the frontend Nexus dispatch routes with the shared OpenTelemetry HTTP handler. ## Why? Nexus HTTP requests need an inbound server span to connect the caller trace. ## How did you test it? - [x] built - [ ] run locally and tested manually - [ ] covered by existing tests - [x] added new unit test(s) - [ ] added new functional test(s)

stephanos mentioned this pull request Aug 13, 2026

Annotate Nexus spans #11561

Merged

5 tasks

stephanos force-pushed the chrsmith/nexus-http-server-tracing branch from cf4e7b1 to f1d6dd9 Compare August 13, 2026 22:47

github-actions Bot mentioned this pull request Aug 13, 2026

🦞 OpenClaw 生态日报 2026-08-14 junlinzhao327-oss/big_model_radar#343

Open

stephanos force-pushed the chrsmith/nexus-http-server-tracing branch from f1d6dd9 to bc7ea0c Compare August 14, 2026 19:44

stephanos force-pushed the chrsmith/nexus-http-server-tracing branch from bc7ea0c to 0e3d8cb Compare August 14, 2026 23:13

stephanos marked this pull request as ready for review August 15, 2026 00:35

stephanos requested a review from a team August 15, 2026 00:35

stephanos requested review from a team as code owners August 15, 2026 00:35