Skip to content

Add enableSessionTelemetry option to all SDKs#1214

Closed
Davsterl wants to merge 5 commits intogithub:mainfrom
Davsterl:dev/davsterl/enableSessionTelemetry
Closed

Add enableSessionTelemetry option to all SDKs#1214
Davsterl wants to merge 5 commits intogithub:mainfrom
Davsterl:dev/davsterl/enableSessionTelemetry

Conversation

@Davsterl
Copy link
Copy Markdown
Contributor

@Davsterl Davsterl commented May 6, 2026

Adds an optional enableSessionTelemetry field to SessionConfig and ResumeSessionConfig across Node.js, Python, Go, and .NET SDKs.

When omitted (default), session telemetry remains enabled — no extra parameter is needed. When set to false, internal session telemetry is disabled.

Includes unit tests for serialization, cloning, and wire forwarding in all four languages.

Adds an optional enableSessionTelemetry field to SessionConfig and
ResumeSessionConfig across Node.js, Python, Go, and .NET SDKs.

When omitted (default), session telemetry remains enabled — no extra
parameter is needed. When set to false, internal session telemetry
(Hydro/AppInsights) is disabled for that session.

Includes unit tests for serialization, cloning, and wire forwarding
in all four languages.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 6, 2026 18:08
@Davsterl Davsterl requested a review from a team as a code owner May 6, 2026 18:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new per-session configuration flag (enableSessionTelemetry) across the Node.js, Python, Go, and .NET SDKs to allow callers to disable internal session telemetry (while keeping the default behavior unchanged when omitted). This fits into the SDKs’ cross-language SessionConfig / ResumeSessionConfig surface area and ensures the option is serialized and forwarded over the JSON-RPC wire consistently.

Changes:

  • Introduces enableSessionTelemetry / enable_session_telemetry on SessionConfig and ResumeSessionConfig across Node.js, Python, Go, and .NET.
  • Forwards the value into session.create / session.resume payloads in each SDK.
  • Adds unit tests in each language to validate cloning/serialization and wire forwarding behavior.
Show a summary per file
File Description
python/test_client.py Adds forwarding tests for enableSessionTelemetry on session.create and session.resume.
python/copilot/session.py Extends SessionConfig / ResumeSessionConfig TypedDicts with enable_session_telemetry.
python/copilot/client.py Adds optional enable_session_telemetry parameters and forwards to enableSessionTelemetry on the wire.
nodejs/test/client.test.ts Adds request-forwarding tests for enableSessionTelemetry on create/resume.
nodejs/src/types.ts Extends SessionConfig and ResumeSessionConfig types to include enableSessionTelemetry (with docs).
nodejs/src/client.ts Forwards enableSessionTelemetry into session.create / session.resume request payloads.
go/types.go Adds EnableSessionTelemetry to configs and wire request structs with JSON tag.
go/client.go Forwards EnableSessionTelemetry from config into create/resume requests.
go/client_test.go Adds JSON serialization tests for presence/omission of enableSessionTelemetry.
dotnet/test/Unit/SerializationTests.cs Adds serialization tests ensuring enableSessionTelemetry is written as expected.
dotnet/test/Unit/CloneTests.cs Adds clone coverage for EnableSessionTelemetry (set + default/null).
dotnet/src/Types.cs Adds EnableSessionTelemetry to SessionConfig and ResumeSessionConfig and ensures it is cloned.
dotnet/src/Client.cs Adds EnableSessionTelemetry to internal create/resume request records and wires it from config.

Copilot's findings

  • Files reviewed: 13/13 changed files
  • Comments generated: 1

Comment thread nodejs/src/types.ts Outdated
Comment thread dotnet/src/Types.cs Outdated
Comment thread dotnet/test/Unit/CloneTests.cs
Davsterl and others added 2 commits May 6, 2026 11:22
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Enrich XML/JSDoc/comment docs across all SDKs: explain default behavior,
  clarify independence from OpenTelemetry config
- Add two .NET E2E tests validating wire serialization of enableSessionTelemetry
- Fix gofmt alignment in Go test (CI failure)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nickfyson
Copy link
Copy Markdown

nickfyson commented May 7, 2026

👋 Hello!

Is it likely to be possible to get this merged today, and then into a published version of the SDK? 😄

(Context – we're waiting on this functionality for our work on TS Autofind, and it's one of the blockers to starting our dark ship of the re-platforming onto CCA.)

Comment thread dotnet/test/E2E/ClientOptionsE2ETests.cs
…sables

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@stephentoub
Copy link
Copy Markdown
Collaborator

@Davsterl, could you make the docs clarifying change for the other languages as well? Then we can get this merged. Thanks.

Clarify that when a custom provider (BYOK) is configured, session
telemetry is always disabled regardless of the enableSessionTelemetry
setting. Updated Node.js, Go, and Python to match .NET docs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@stephentoub
Copy link
Copy Markdown
Collaborator

I'm not sure why the node tests are failing; it looks like infrastructure and not being able to get an auth token. I'm going to try submitting your commits in a new PR...

@stephentoub
Copy link
Copy Markdown
Collaborator

@Davsterl, I cherry-picked your commits into #1224, which is passing CI. I'm going to close this PR and go with that one, which also addresses the code review that highlighted because of things racing in CI, the Rust SDK was missing the new members.

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.

4 participants