feat(sdk): add config driven sink selection#176
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
lan17
left a comment
There was a problem hiding this comment.
I like the direction of moving sink selection into a shared model, but I don't think this middle layer is ready to merge yet. The biggest issue is that the config surface is now shared between SDK and server, but the semantics still diverge. I also don't think the SDK should take ownership of caller-registered sinks on shutdown, and returning the best fanout result makes partial delivery failures too easy to miss.
lan17
left a comment
There was a problem hiding this comment.
Overall I think this is in good shape. The split between shared selection shape and runtime-specific semantics is the right direction, and moving the server onto its own sink env names fixes the biggest design problem from the earlier version. I left one follow-up comment on named-sink cleanup when switching back to default, but I don't think it should block this landing. Assuming this gets rebased cleanly on top of 175, I'm good with it.
Summary
Added config-driven observability sink selection across the SDK and server.
Introduced shared telemetry sink-selection primitives so observability can use the default backend, registered SDK sinks, or named custom sink factories.
Expanded tests and README examples to cover the new sink-selection behavior and lifecycle handling.
Fixed the SDK mypy issue in shutdown handling by wrapping sink lifecycle awaitables in a concrete coroutine before passing them to
asyncio.run().Scope
User-facing/API changes:
agent_control.init()now acceptsobservability_sink_nameandobservability_sink_configInternal changes:
telemetry/Out of scope:
Risk and Rollout
Risk level: medium
Rollback plan: Revert this branch to restore the previous default-only observability path. As a partial mitigation, deployments can keep
observability_sink_name=defaultto stay on the legacy behavior even with this code merged.Testing
make check— could not run in this environment becauseuvwas not available in the shellChecklist
Follow-up tasks:
make checkin a full local/CI environment withuvavailable