Skip to content

feat(sdk): Add telemetry package to support sinks#164

Open
namrataghadi-galileo wants to merge 4 commits intomainfrom
feature/61573-add-telemetry-sink
Open

feat(sdk): Add telemetry package to support sinks#164
namrataghadi-galileo wants to merge 4 commits intomainfrom
feature/61573-add-telemetry-sink

Conversation

@namrataghadi-galileo
Copy link
Copy Markdown
Contributor

Summary

Added a new shared top-level telemetry/ package to hold common telemetry contracts and trace-context primitives used across AgentControl.

Refactored the SDK observability write path to use the shared ControlEventSink contract internally while preserving the existing default OSS behavior: queue-backed batching and delivery to /api/v1/observability/events.

Aligned telemetry packaging with the existing models/engine bundling pattern so telemetry is vendored into SDK and server builds rather than treated as a separate runtime-only dependency.

Scope

User-facing / API changes

  • No intended user-facing behavior changes.
  • No change to default OSS event delivery behavior.
  • No new public sink-selection API in this story.

Internal changes

  • Introduced shared telemetry types: ControlEventSink, SinkResult, BaseControlEventSink, and shared trace-context helpers.
  • Removed SDK-local ownership of trace-context internals and pointed SDK code/tests to the shared telemetry package.
  • Updated SDK observability internals so event writes flow through the shared sink contract.
  • Updated build/package wiring so telemetry is bundled like models and engine.

Out of scope

  • Server write-path refactor to the shared sink contract.
  • Config-driven sink selection.
  • OTEL sink implementation and OTEL settings.
  • Any changes to /observability/events server ingestion behavior.

Risk and Rollout

Risk level: Medium

Rollback plan:

  1. Revert the telemetry package introduction and SDK observability refactor.
  2. Restore the previous SDK-local telemetry module and direct batcher-based event writes.
  3. Remove telemetry vendoring from SDK/server build configuration.

Testing

  • Added or updated automated tests
  • Ran make check (or explained why not)

Focused validation is still pending locally; mypy surfaced follow-up packaging/type issues during development and we have been resolving those incrementally.

  • Manually verified behavior

Checklist

  • Linked issue/spec (if applicable)
  • Updated docs/examples for user-facing changes

No docs/example updates were needed because this story is internal-only and no repo examples referenced the old telemetry module path.

  • Included any required follow-up tasks

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 5, 2026

Codecov Report

❌ Patch coverage is 98.55072% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
sdks/python/src/agent_control/observability.py 96.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@lan17 lan17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

telemetry/ is now a standalone workspace package, but it is not wired into the repo's standard validation path. Please add it to the normal make lint, make typecheck, and make test/CI coverage so it can't drift outside the usual gates.

I also think it should have its own direct tests, even if they stay lightweight. Right now the package only gets incidental coverage through SDK/server tests, but the package itself owns behavior like trace-context validation and the sync/async sink contract. Those are simple enough to unit test directly in telemetry/, and package-local tests will make refactors safer than relying only on downstream coverage.

@namrataghadi-galileo
Copy link
Copy Markdown
Contributor Author

namrataghadi-galileo commented Apr 7, 2026

Build & CI Integration

Root validation now includes telemetry in make test, make lint, and make typecheck.
Added a package-local Makefile so those forwarded targets behave like the other workspace packages.

CI/code coverage now picks up telemetry coverage output, with Codecov path-fixing added for the new package.

  • ci.yml
  • codecov.yml

Tests & Validation

  • Added direct telemetry tests for trace-context validation and sink contracts.
  • Tightened provider validation so blank/whitespace IDs are rejected consistently.
  • Added async sink contracts plus tests so the package owns both sync and async behavior explicitly.

Copy link
Copy Markdown
Contributor

@lan17 lan17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The telemetry coverage gap is addressed. telemetry/ now has direct tests plus Makefile and CI wiring for test/lint/typecheck, which was the missing piece I called out earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants