[SDT-0001]: task-local instrument proposal

[kukushechkin]

This proposal adds a withInstrument(::) for task-local instrument scoping.

Motivation:

InstrumentationSystem.bootstrap(_:) installs an instrument once per process, so every withSpan/startSpan and every read of InstrumentationSystem.instrument resolves through that single instrument. That fits one tracer for the whole application, but it leaves no way to bind a different instrument for a region of work — most acutely in tests, where a second bootstrap crashes and parallel tests cannot each install their own in-memory tracer.

Modifications:

SDT-0001 proposal is added.

Result:

SDT-0001 proposal is ready for review.

[slashmo]

This sound like a great addition to me 👍 Perhaps another future direction could be a test trait that encapsulates the task-local tracer so we don't have to pay the indentation cost in every test function. Is this something you already considered, either for Tracing or Metrics/Logging?

[kukushechkin]

@slashmo yes, this seems to be a reasonable future direction. Similar in spirit to https://forums.swift.org/t/pitch-add-a-tasklocal-trait-to-swift-testing/87603

[czechboy0]

How does one get the task-local tracer, then? Do they have to dynamically cast the instrument? What should they do if the Instrument fails to cast to Tracer?

[slashmo]

How does one get the task-local tracer, then? Do they have to dynamically cast the instrument? What should they do if the Instrument fails to cast to Tracer?

InstrumentationSystem.tracer should still work for this case. Either the task-local instrument is a tracer and it's returned or it falls back to the no-op tracer.

[czechboy0]

I see, the language in the proposal seemed to imply that this is entirely opt-in, not that the existing API starts checking for the task local.

Don't get me wrong, I think this is the right solution, just explaining where my confusion came from.

Looks good!

[kukushechkin]

@czechboy0 opt-in in a sense InstrumentationSystem.tracer does not lookup anything task-local. If it find TaskLocalInstrument, recognizes it is a container (similar to the MultiplexInstrument) and asks it for the actual instrument. This is where task-local lookup happens inside TaskLocalInstrument. No TaskLocalInstrument == no task-local lookup.

[czechboy0]

Thanks for making me better understand the proposal now - unfortunately, I now don't think it's the right approach.

1. Not being able to call withInstrument from libraries is a non-starter. We must be able to cross Swift Concurrency/manual futures boundaries in libraries, and jump between manual propagation and task local propagation at will, upgrading ecosystem libraries as they see fit. This has been discussed at length in the motivation behind the approach taken in Swift Log and Swift Metrics. I don't think we should diverge here.
2. Installing a task local instrument prevents an important use case: installing a "crash immediately if accessed" instrument into the process bootstrap, and using task local and manual propagation exclusively in your entire application. This is crucial in enabling gradual migration away from process bootstraps, and again we deliberately made this possible in Swift Log/Metrics, and should enable that here as well.
3. If I'm not mistaken, since ServiceContext already has a task local, this introduces a second task local for tracing. Meaning that if you're crossing manual vs task local propagation, you need to manually manage not one, but two values. And if you forget one or the other, you're likely to get confusing results. Here I don't have a simple solution, but I think it requires more discussion if it's at all possible to have a combined task local that holds the instrument but also the service context, so that the workflow is just as easy as Log/Metrics, where you always just need to create a single scope for a single value, no two of them.

I think it's great you've explored this approach, but I believe we need to stay better aligned with Log/Metrics and not provide a worse experience here. If much of the motivation was driven out of avoiding a task local lookup when acquiring a Tracer, I think it's the wrong thing to optimize and instead we should optimize the long term shape of this API and make it easy to adopt and reason about.

[kukushechkin]

@czechboy0 withInstrument is an alternative to bootstrap, not a parallel mechanism. Both install into the same global slot. So there's no "disallow the global bootstrap but keep
task-local + manual" mode. Nothing on the read side chooses between "the bootstrapped instrument" and "a task-local one".

Libraries can call withInstrument — but it only does anything when the application is already using it. If the app chose an explicit bootstrap instead, a library withInstrument crashes, and the resolution is for the app to adopt withInstrument. What a library scopes is its own business, but in practice a library has no meaningful instrument to install.

Context stays with ServiceContext. Accumulating context down the stack is ServiceContext.withValue's job, unchanged by this proposal, and that's what libraries use.

[czechboy0]

withInstrument is an alternative to bootstrap, not a parallel mechanism

Right, but why? In Log/Metrics, you can use the with* functions in libraries just fine, and they don't interact with the process bootstrap'd backend. Why did we diverge here?

So there's no "disallow the global bootstrap but keep
task-local + manual" mode

In Log/Metrics, there is, and I think it's a crucial feature for gradual migration.

What might help - could you add more examples to the proposal about how to cross manual propagation and task local propagation, in both directions, in a library? Maybe I'm missing something 🙂

[kukushechkin]

@czechboy0 let's verify if I understand what kind of example you have in mind before adding to the proposal. Does this work?

// Application
// sets INSTRUMENT task-local
try await withInstrument(OTelTracer(configuration: config)) {
	let client = TracedClient(transport: transport)
	let server = TracedServer { request in
		try await client.send(HTTPRequest(method: .get, path: "/users"))
	}
	try await transport.serve(server.serve)
}

// Inbound library
public struct TracedServer {
	let handler: (HTTPRequest) async throws -> HTTPResponse
	
	public func serve(_ request: HTTPRequest) async throws -> HTTPResponse {
		var context = ServiceContext.topLevel
		// reads INSTRUMENT task-local
		InstrumentationSystem.instrument.extract(request.headers, into: &context, using: HTTPHeadersExtractor())
		
		// sets CONTEXT task-local (manual → task-local)
		return try await ServiceContext.withValue(context) {
			try await withSpan("\(request.method) \(request.path)", ofKind: .server) { _ in
				try await self.handler(request)
			}
		}
	}
}

// Outbound library
public struct TracedClient {
	let transport: any HTTPTransport
	
	public func send(_ request: HTTPRequest) async throws -> HTTPResponse {
		try await withSpan("\(request.method) \(request.path)", ofKind: .client) { span in
			var request = request
			// reads INSTRUMENT task-local
			InstrumentationSystem.instrument.inject(
				// reads CONTEXT task-local (task-local → manual)
				span.context,
				into: &request.headers,
				using: HTTPHeadersInjector()
			)
			return try await self.transport.send(request)
		}
	}
}

[czechboy0]

Not quite, I'm thinking a library that gets a Tracer passed explicitly, and calls out to another library in an async func, so needs to put that Tracer into a task local. And another example for the inverse, where the library has an async public method that reads out the task-local tracer, and passes it explicitly to code that isn't async.

And importantly - this behavior must work reliably for the library, it can't depend on the exact backend that the application bootstrapped.

The idea of hiding the task local in the backend was considered for Log/Metrics and ultimately rejected, so I'm not sure why we brought it back for tracing. The flaws are still there - the libraries suddenly can't reliably pass around telemetry handles unless the application bootstraps a specific backend, which causes undesirable coupling. The library should be able to write a unit test for its own passing around of telemetry handles and that correctness shouldn't depend on the application.

[kukushechkin]

I'm thinking a library that gets a Tracer passed explicitly, and calls out to another library in an async func

Right, yes.

Application (A) -> lib (B) with explicit Tracer API -> lib (C) without explicit Tracer API:

// (A) bootstraps everything
{
    withInstrument(someTracer) { // internally does the bootstrap
        ServiceContext.$current.withValue(someContext) {
            // scoped stuff is installed, but also passed explicitly
            try await B().run(InstrumentationSystem.tracer, ServiceContext.current)
        }
    }
}

// (B) with explicit Tracer API
struct B {
    func run(tracer: any Tracer, context: ServiceContext) async throws {
        try await tracer.withSpan("B.run", context: context) { _ in
            // No need to explicitly scope `tracer`
            try await C().run()
        }
    }
}

// (C) resolves the active tracer
struct C {
    func run() async throws {
        // uses InstrumentationSystem.tracer, which is one from .bootstrap() or .withInstrument()
        try await withSpan("C.run") { _ in … } 
    }
}

What if (A) never installed bootstrap or withInstrument? withSpan/.tracer turns into NoOp.
What if (B) actually installs .withInstrument? Well, this is discouraged, because library changes the global bootstrap, but nothing will crash and it is better than a library hijacking bootstrap — withInstrument installs the TaskLocalInstrument which is still NoOp outside of that scope.

(A') -> (C') -> (B')

(C') now relies on something being installed in the global bootstrap, either TaskLocal or not, it doesn't matter, there is no TaskLocal-specific APIs. Nothing installed? TaskLocalInstalled but no instrument binded? NoOp.

// (A') bootstraps everything
{
    withInstrument(someTracer) { // internally does the bootstrap
        ServiceContext.$current.withValue(someContext) {
            try await C().run()
        }
    }
}

// (C') resolves the active tracer
struct C {
    func run() async throws {
        // uses InstrumentationSystem.tracer, which is one from .bootstrap() or .withInstrument()
        try await withSpan("C.run") { span in
            B().run(InstrumentationSystem.tracer, span.context)
        } 
    }
}

// (B') with explicit Tracer API
struct B {
    func run(tracer: any Tracer, context: ServiceContext) async throws {
        try await tracer.withSpan("B.run", context: context) { _ in
            ...
        }
    }
}

Tests

Tests are the application. So tests do the .withInstrument Binding if necessary.

[czechboy0]

I understand the implications of the current design.

My point is that I don't think it's the right design for the various reasons I outlined. Mainly because it's completely diverging from Swift Log/Metrics for reasons I don't understand.

My question: why don't we do the same for Instrument/Tracer that we did for Logger and MetricsFactory?

[kukushechkin]

@czechboy0 I think this design is more aligned with the existing metrics and log design. Let me explain. Metrics read taskLocal ?? bootstrap on every metric creation, which is not a hot path (at least not recommended to be), increment() never touches task-local. Logger.current is an opt-in, off any existing hot path. Tracing's withSpan, on the contrary, is the hot path itself. I did benchmark and, as expected, it roughly doubles withSpan on a NoOp backend (indeed, probably <1% with a real exporter), and it's paid by every existing span, so keeping it opt-in and out of hot path would be similar to metrics and logs.

But we do not prevent ourselves from changing it later either. The withInstrument()/.instrument/.tracer API stays, with only implementation details changing to the always-checked-task-local slot later is a widening, non-breaking change. My lean is to ship the conservative version now and keep the slot as an easy follow-up if adoption shows everyone's on the task-local path.

But happy to get @ktoso in the discussion to hear his opinion here as well. I think we've been keeping this discussion in the PR rather than in the forums, I wonder if others missed it.

[czechboy0]

Thanks for more details, I still think this optimizes for the wrong thing, and sacrifices usability and how we want folks to use this API long term.

If perf is a huge issue, I'd much prefer we simply compile everything out like in Swift Log as an opt-in feature. But IMO the default should be high-convenience-fine-performance, and opt in should be top performance with reduced convenience.

That's what we did for Swift Log traits, and I think it's a good general rule of thumb.

Right now this proposal puts performance over everything else, and I don't think it's the right tradeoff.

[czechboy0]

I'm also still not sure how to, in a library, put an explicitly provided tracer into the task local. Something that we need to support for sure, and is easy to do in Log/Metrics.

[kukushechkin]

sacrifices usability and how we want folks to use this API long term

@czechboy0 Now I am going to ask for examples :) Where do you see API usability sacrifice? If we go with the "always-checked-task-local-slot", the withInstrument/.instrument/.tracer API will be exactly the same.

[kukushechkin]

I'm also still not sure how to, in a library, put an explicitly provided tracer into the task local.

withInstrument(tracer) {}:

• (1) if there is a task-local tracer bootstrapped, it will work, but doesn't make much sense — the app is using task-local, so it should've already wrapped explicit tracer pass with a scope (it did take the instrument from somewhere), similar to logger,
• (2) if there is nothing bootstrapped by the app, it will work, bootstrap the task-local returning NoOp outside of that scope,
• (3) if there is a non-task-local tracer bootstrapped — not allowed, crash.

Case (3) sounds bad and it is a diversion from "task-local-overrides any global bootstrap" used by metrics/log, but it is not incorrect. If application decided to .bootstrap a particular tracer, it is forced across all the dependencies and cannot be overridden by some dependency and using any of such dependency is prohibited. All the traces are emitted, nothing is lost. If an application decided to use scoped tracers (root withInstrument instead of an explicit bootstrap) — libraries are allowed to override them. Metrics/log approach does not have this "lock tracer" path even if application did global bootstrap. Tracer does not have "allow libraries do whatever regardless if application did bootstrap or root withInstrument" path.

[czechboy0]

I continue to think that the inability to reliably use, and be able to reason about, withInstrument inside libraries basically defeats the purpose of opening the door to task local propagation of tracers.

// Library
public func doStuff(logger: Logger, metrics: any MetricsFactory, tracing: any Tracer) {
  // eg in an unstructured task
  Task {
    try await withLogger(logger) {
      try await withMetricsFactory(metrics) {
        // A
      }
    }
  }
}

At point A:

• we know what Logger.current will have (logger), always safe to use, doesn't depend on process bootstrap
• we know what MetricsSystem.factory will have (metrics), always safe to use, doesn't depend on process bootstrap

How do we achieve the same for tracing? Something that works reliably in the library regardless of what the app did, the library can locally reason about?

The current withInstrument isn't it, because libraries can't actually adopt it unless they want to become the source of crashes, and holding this right requires coupling between libraries and apps. We don't have this problem for Log/Metrics, not sure why we'd create it here.

What ergonomics we're missing (mostly from my first post)

• ability to bootstrap a "crash if used" backend to allow motivated app devs to fully rely on task locals and explicit passing (possible in Log/Metrics, not Tracing)
• ability to locally reason inside a library about propagating your Tracer (works in Log/Metrics, not Tracing)
• the need to manage two task locals manually whenever crossing concurrency/manual boundaries (Log/Metrics each have one)

I have ideas on how to achieve all these while making Tracing behave like Log/Metrics, but don't want to hijack this thread by proposing an alternative approach 🙂

[czechboy0]

More broadly - I think this proposal needs to be considered in the context of the accepted and implemented task locals for Log/Metrics. So much so that the proposal here should only diverge from that approach if there's a very good reason and won't hurt library and application owners who implement all three telemetry signals.

Right now, this proposal seems to optimize for avoiding a task local lookup in withSpan, and works around that constraint. I don't think that's the right approach, however, and we should optimize for the ease of use of majority of users (for whom a task local lookup per span makes no measurable difference), and separately discuss other ways to make telemetry free for those who don't need it (like we did with traits in Log).

[ktoso]

Honza, you're continuing this discussion in the wrong place. Please move over to the review thread: https://forums.swift.org/t/proposal-sdt-0001-task-local-instrument/87826/4 Review threads are review threads because their point is to get involvement from the community in these discussions, and the PR discussion is likely completely missed out by folks interested in it.

[czechboy0]

Apologies. Will use the forums thread next time, on our other repos we welcome discussion both on the forums and the proposal PR, I didn't realize that wasn't the case in this repo.

Though - seems all the technical discussion happened here, not sure how to "move" a long back-and-forth now, or whether it's worth it. Vlad linked to this PR from there so hopefully nobody will miss it.