Skip to content

feat(sdk): [Enterprise Integration]: Add provider agnostic traceing#145

Merged
namrataghadi-galileo merged 5 commits into
mainfrom
feature/59789-add-provider-agnostic-tracing
Mar 27, 2026
Merged

feat(sdk): [Enterprise Integration]: Add provider agnostic traceing#145
namrataghadi-galileo merged 5 commits into
mainfrom
feature/59789-add-provider-agnostic-tracing

Conversation

@namrataghadi-galileo

Copy link
Copy Markdown
Contributor

Summary
Added a new provider-agnostic telemetry package to the AgentControl Python SDK for external trace context resolution and merged control event emission.
Updated tracing to consult a registered external trace context provider before falling back to OTEL context.
Exported the new telemetry APIs from the top-level agent_control package.
Added focused tests to ensure provider/sink failures do not affect existing behavior.

Scope

  • User-facing/API changes:

    • New SDK APIs:

      1. set_trace_context_provider(...)
      2. get_trace_context_from_provider()
      3. clear_trace_context_provider()
      4. set_control_event_sink(...)
      5. emit_control_events(...)
      6. clear_control_event_sink()
  • Internal changes:

    • Added:

      1. agent_control/telemetry/trace_context.py
      2. agent_control/telemetry/event_sink.py
      3. agent_control/telemetry/init.py
      4. Updated tracing.py to use external provider before OTEL fallback.
      5. Added tests for provider behavior, sink behavior, and tracing precedence.

Risk and Rollout
Risk level: low
Rollback plan:
Revert the new telemetry package and the small tracing/export changes in the SDK.
Since the change is additive and inactive unless a provider or sink is explicitly registered, rollback is straightforward.

Testing

  • Added or updated automated tests
  • Ran make check (or explained why not)
  • Ran targeted SDK tests only; did not run the full project check suite.
  • Manually verified behavior

Checklist

  • Linked issue/spec (if applicable)
  • Updated docs/examples for user-facing changes
  • Included any required follow-up tasks

@namrataghadi-galileo namrataghadi-galileo changed the title feat(sdk): Add provider agnostic traceing feat(sdk): [Enterprise Integration]: Add provider agnostic traceing Mar 23, 2026
@codecov

codecov Bot commented Mar 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@lan17

lan17 commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Why not emit these from the server?

@namrataghadi-galileo

Copy link
Copy Markdown
Contributor Author

@lan17 Keeping emission in the SDK gives us one merged, ordered batch and a single integration model for both logger and non-logger flows

@lan17

lan17 commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

@lan17 Keeping emission in the SDK gives us one merged, ordered batch and a single integration model for both logger and non-logger flows

What about other language sdks, like typescript, java, etc.?

If we can delegate this to agent control server then it makes sdk easier.

Could we just send additional metadata to agent control server when we send events to integrate with Galileo spans/traces?

@namrataghadi-galileo

Copy link
Copy Markdown
Contributor Author

Server-side emission is attractive for thinner SDKs, but it defeats the main purpose of the logger-based design: reusing Galileo Logger’s in-process trace buffering and flush. For logger integrations, the SDK must have the merged control events locally before flush. Other languages can still be supported through the thinner OTEL non-logger path until they need full logger integration.

@lan17

lan17 commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Server-side emission is attractive for thinner SDKs, but it defeats the main purpose of the logger-based design: reusing Galileo Logger’s in-process trace buffering and flush. For logger integrations, the SDK must have the merged control events locally before flush. Other languages can still be supported through the thinner OTEL non-logger path until they need full logger integration.

Should this be done outside of core OSS sdk, then?

This pattern conflicts with one @abhinav-galileo implemented where we emit events to the server (also via a buffer on SDK), so it maybe confusing to have both systems at same time.

@namrataghadi-galileo

namrataghadi-galileo commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

The current PR only adds provider-agnostic hooks to OSS. It does not move logger context into OSS or change the default buffered event-to-server flow. Logger context, span conversion, trace attachment, and flush integration still belong to the external Galileo integration layer, so OSS is not taking on a second concrete observability system.

Also, it would be confusing if both were active default systems. It is much less confusing if the existing buffered SDK-to-server flow remains the only default OSS behavior and the new sink/provider APIs are treated purely as optional extension points for external integrations.

@namrataghadi-galileo

Copy link
Copy Markdown
Contributor Author

I’m also waiting to hear back from both Davids on the RFC and get their perspective on the proposed logger-based and non-logger-based approaches. That said, the hook additions themselves are independent of any Galileo-specific observability or tracing design. They are generic enough to support other third-party observability systems as well. For example, if LangSmith is the external observability framework, these hooks could still be used to publish trace_id and span_id

@lan17 lan17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the attempt to keep the new telemetry surface generic, but there are still a few code-level gaps before this is ready to merge. I left inline comments on the sink wiring, provider validation, and the partial integration of the provider across SDK entry points.

Comment thread sdks/python/src/agent_control/telemetry/event_sink.py Outdated
Comment thread sdks/python/src/agent_control/telemetry/trace_context.py
Comment thread sdks/python/src/agent_control/tracing.py
@namrataghadi-galileo namrataghadi-galileo enabled auto-merge (squash) March 27, 2026 23:05
@namrataghadi-galileo namrataghadi-galileo merged commit f1ca27c into main Mar 27, 2026
6 checks passed
@namrataghadi-galileo namrataghadi-galileo deleted the feature/59789-add-provider-agnostic-tracing branch March 27, 2026 23:05
galileo-automation pushed a commit that referenced this pull request Apr 7, 2026
## [2.2.0](ts-sdk-v2.1.0...ts-sdk-v2.2.0) (2026-04-07)

### Features

* **evaluators:** add starts_with/ends_with mode to list evaluator ([#154](#154)) ([bf1f7d7](bf1f7d7))
* **sdk:** [Enterprise Integration]: Add provider agnostic traceing ([#145](#145)) ([f1ca27c](f1ca27c))
* **sdk:** Add telemetry package to support sinks ([#164](#164)) ([2186ba1](2186ba1))
* **sdk:** default merge events in SDK ([#155](#155)) ([5984a60](5984a60))
* **server,sdk, ui:** Control Templates ([#158](#158)) ([78bb538](78bb538))
* **server:** Override PG password in dockerfile ([#148](#148)) ([5d70c7d](5d70c7d))
* **server:** Remove container name for dev postgres ([92b2d13](92b2d13))
* **server:** Start local dev pg under docker compose project endign with dev ([88bee63](88bee63))
* **ui, server:** Intuitive JSON editing for Controls ([#151](#151)) ([8c23cef](8c23cef))
* **ui:** add full control JSON editing and create-from-JSON ([#147](#147)) ([e685ed0](e685ed0))

### Bug Fixes

* **docs:** add explicit shutdown to quickstart example ([#149](#149)) ([b76014f](b76014f))
* **sdk:** use sync shutdown flush fallback ([#150](#150)) ([90265ba](90265ba))
* **server:**  remove unused evaluator config store ([#152](#152)) ([dea2873](dea2873))
* **server:** Omit null fields in control JSON editor ([#157](#157)) ([0aa2f3c](0aa2f3c))
* **server:** Update docker-compose.dev.yml to use different container name ([14d4c87](14d4c87))
* **ui:** improve edit control ux, no layout shift, consistent spacing ([#122](#122)) ([76d67b9](76d67b9))
@galileo-automation

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

abhinav-galileo added a commit that referenced this pull request Apr 29, 2026
…g endpoints

The seven /control-bindings endpoints were migrated onto require_operation
in #204, but none supplied a context_builder. Upstream authorizers that
resolve the target's owning project (e.g., Galileo's
check_management_access) need (target_type, target_id) to make a
project-level decision; without them the upstream returns 400 and the
provider fails closed with 503.

Two builders, four endpoints wired:

- _binding_body_context — reads target_type/target_id from the request
  body. Wired on PUT "", PUT "/by-key", POST "/by-key:delete".
- _binding_list_context — reads target_type/target_id from query params
  when the GET list endpoint is target-scoped. Wired on GET "".

The header provider's behavior is unchanged because it ignores context.
Validated end-to-end against the live api PR #6350 + authz PR #145
stack: GET with target filter, PUT with owned target, foreign-target
404, no-auth 401 all behave correctly.

Out of scope (separate follow-up): the binding_id-based endpoints
(GET/PATCH/DELETE /{binding_id}) need a 2-phase auth — look up the
binding by namespace+id to discover its target, then auth-check with
target context. That's a deeper change to the require_operation contract
and is tracked separately.
abhinav-galileo added a commit that referenced this pull request Apr 29, 2026
…g endpoints

The seven /control-bindings endpoints were migrated onto require_operation
in #204, but none supplied a context_builder. Upstream authorizers that
resolve the target's owning project (e.g., Galileo's
check_management_access) need (target_type, target_id) to make a
project-level decision; without them the upstream returns 400 and the
provider fails closed with 503.

Two builders, four endpoints wired:

- _binding_body_context — reads target_type/target_id from the request
  body. Wired on PUT "", PUT "/by-key", POST "/by-key:delete".
- _binding_list_context — reads target_type/target_id from query params
  when the GET list endpoint is target-scoped. Wired on GET "".

The header provider's behavior is unchanged because it ignores context.
Validated end-to-end against the live api PR #6350 + authz PR #145
stack: GET with target filter, PUT with owned target, foreign-target
404, no-auth 401 all behave correctly.

Out of scope (separate follow-up): the binding_id-based endpoints
(GET/PATCH/DELETE /{binding_id}) need a 2-phase auth — look up the
binding by namespace+id to discover its target, then auth-check with
target context. That's a deeper change to the require_operation contract
and is tracked separately.
abhinav-galileo added a commit that referenced this pull request Apr 29, 2026
…g endpoints

The seven /control-bindings endpoints were migrated onto require_operation
in #204, but none supplied a context_builder. Upstream authorizers that
resolve the target's owning project (e.g., Galileo's
check_management_access) need (target_type, target_id) to make a
project-level decision; without them the upstream returns 400 and the
provider fails closed with 503.

Two builders, four endpoints wired:

- _binding_body_context — reads target_type/target_id from the request
  body. Wired on PUT "", PUT "/by-key", POST "/by-key:delete".
- _binding_list_context — reads target_type/target_id from query params
  when the GET list endpoint is target-scoped. Wired on GET "".

The header provider's behavior is unchanged because it ignores context.
Validated end-to-end against the live api PR #6350 + authz PR #145
stack: GET with target filter, PUT with owned target, foreign-target
404, no-auth 401 all behave correctly.

Out of scope (separate follow-up): the binding_id-based endpoints
(GET/PATCH/DELETE /{binding_id}) need a 2-phase auth — look up the
binding by namespace+id to discover its target, then auth-check with
target context. That's a deeper change to the require_operation contract
and is tracked separately.
abhinav-galileo added a commit that referenced this pull request Apr 29, 2026
…g endpoints

The seven /control-bindings endpoints were migrated onto require_operation
in #204, but none supplied a context_builder. Upstream authorizers that
resolve the target's owning project (e.g., Galileo's
check_management_access) need (target_type, target_id) to make a
project-level decision; without them the upstream returns 400 and the
provider fails closed with 503.

Two builders, four endpoints wired:

- _binding_body_context — reads target_type/target_id from the request
  body. Wired on PUT "", PUT "/by-key", POST "/by-key:delete".
- _binding_list_context — reads target_type/target_id from query params
  when the GET list endpoint is target-scoped. Wired on GET "".

The header provider's behavior is unchanged because it ignores context.
Validated end-to-end against the live api PR #6350 + authz PR #145
stack: GET with target filter, PUT with owned target, foreign-target
404, no-auth 401 all behave correctly.

Out of scope (separate follow-up): the binding_id-based endpoints
(GET/PATCH/DELETE /{binding_id}) need a 2-phase auth — look up the
binding by namespace+id to discover its target, then auth-check with
target context. That's a deeper change to the require_operation contract
and is tracked separately.
abhinav-galileo added a commit that referenced this pull request Apr 30, 2026
…g endpoints

The seven /control-bindings endpoints were migrated onto require_operation
in #204, but none supplied a context_builder. Upstream authorizers that
resolve the target's owning project (e.g., Galileo's
check_management_access) need (target_type, target_id) to make a
project-level decision; without them the upstream returns 400 and the
provider fails closed with 503.

Two builders, four endpoints wired:

- _binding_body_context — reads target_type/target_id from the request
  body. Wired on PUT "", PUT "/by-key", POST "/by-key:delete".
- _binding_list_context — reads target_type/target_id from query params
  when the GET list endpoint is target-scoped. Wired on GET "".

The header provider's behavior is unchanged because it ignores context.
Validated end-to-end against the live api PR #6350 + authz PR #145
stack: GET with target filter, PUT with owned target, foreign-target
404, no-auth 401 all behave correctly.

Out of scope (separate follow-up): the binding_id-based endpoints
(GET/PATCH/DELETE /{binding_id}) need a 2-phase auth — look up the
binding by namespace+id to discover its target, then auth-check with
target context. That's a deeper change to the require_operation contract
and is tracked separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants