Standardize telemetry dependency policy by bmehta001 · Pull Request #32705 · microsoft/onnxruntime

bmehta001 · 2026-09-20T10:55:12Z

Description

Delegate Linux curl/mbedTLS construction to the 1DS SDK instead of maintaining a duplicate ORT builder; retain the static-package curl export fix.
Use system SQLite and libz on Apple (with process-safe SQLite lifecycle), minimal private SQLite and vendored zlib on other non-Windows source builds, and avoid unnecessary vcpkg SQLite/zlib packages on Apple.
Preserve Apple static-package system-library and CMake dependency metadata.
Merge current main and keep cpp_client_telemetry v3.10.267.1. The SDK now includes the upstream curl, threading, CA-path, and logging fixes, so the old compatibility patch is removed.

Motivation and Context

Keep one owner for the Linux 1DS transport and avoid shipping an extra process-global SQLite copy on Apple. The dependency work from microsoft/cpp_client_telemetry#1536 is included in the pinned SDK release; this PR carries only the ORT integration that remains necessary.

Validation

lintrunner cmake/deps.txt cmake/external/onnxruntime_external_deps.cmake cmake/CMakeLists.txt cmake/onnxruntime.cmake cmake/onnxruntime_common.cmake cmake/vcpkg-ports/cpp-client-telemetry/vcpkg.json onnxruntime/core/platform/posix/telemetry.cc — passed on Windows.
clang-format --dry-run --Werror onnxruntime/core/platform/posix/telemetry.cc — passed under WSL.
git diff origin/main --check — passed on Windows.
No full build or test suite run after the SDK update; platform CI is pending.

Delegate self-contained Linux curl/mbedTLS construction to 1DS, use Apple system SQLite/libz with host-owned process lifecycle, and use minimal private SQLite on other non-Windows platforms. Preserve static-package dependency targets and the released-SDK compatibility patch. Files changed: - cmake/CMakeLists.txt - cmake/deps.txt - cmake/external/onnxruntime_external_deps.cmake - cmake/external/telemetry_linux_http.cmake - cmake/onnxruntime_common.cmake - cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch - cmake/vcpkg-ports/cpp-client-telemetry/vcpkg.json - onnxruntime/core/platform/posix/telemetry.cc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

The delegated curl path loses required CA-path sanitization and static-package export normalization.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Restore runtime CA bundle selection for SDK-owned FETCH · New

What changed in this PR

Standardizes non-Windows telemetry dependencies across Linux and Apple platforms.

Changes:

Delegates Linux curl/mbedTLS construction to 1DS.
Uses system SQLite/zlib on Apple and private dependencies elsewhere.
Preserves static-package targets and adds pthread-safe mbedTLS configuration.

File	Description
`onnxruntime/core/platform/posix/telemetry.cc`	Avoids process-global SQLite shutdown on Apple.
`cmake/vcpkg-ports/cpp-client-telemetry/vcpkg.json`	Excludes SQLite/zlib packages on Apple.
`cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch`	Enables pthread-safe fetched mbedTLS.
`cmake/onnxruntime_common.cmake`	Links Apple system SQLite and zlib.
`cmake/external/telemetry_linux_http.cmake`	Removes the duplicate Linux dependency builder.
`cmake/external/onnxruntime_external_deps.cmake`	Selects platform-specific 1DS dependency providers.
`cmake/deps.txt`	Reclassifies curl and mbedTLS as transitive inventory entries.
`cmake/CMakeLists.txt`	Recreates Apple dependency targets for static packages.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Restore curl CA-path sanitization in the 3.10.240.1 compatibility patch so Linux packages do not embed build-host trust paths.\n\nFiles changed: cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d3afa07-bc40-4851-bc49-ccf0a37e6da0

Copilot

Copilot review overview

🟡 Changes recommended

The fetched curl target retains an unresolvable CURL::mbedtls dependency in exported static packages.

Review effort: Balanced
Findings: 1

Open (1)

Normalize CURL::mbedtls from libcurl_static link interface · New

Resolved since last review (1)

Restore runtime CA bundle selection for SDK-owned FETCH

Strip curl's directory-scoped CURL::mbedtls helper from the exported static target and link the bundled mbedtls target instead. This preserves self-contained ORT static packages after delegating curl construction to the SDK. Files changed: cmake/external/onnxruntime_external_deps.cmake. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d3afa07-bc40-4851-bc49-ccf0a37e6da0

Copilot

Copilot review overview

🔵 Needs a closer look

Apple static-framework metadata omits the newly required SQLite and zlib system libraries.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Normalize CURL::mbedtls from libcurl_static link interface

Previously missed (1)

In code that hasn't changed since last review

Apple static framework omits zlib and SQLite system libraries

cmake/onnxruntime_common.cmake:324

These link flags cover normal CMake linkage, but not the assembled Apple static framework. That path stitches only archive contents (cmake/onnxruntime.cmake:515-535), while its CocoaPods system-library requirements come from APPLE_SYSTEM_LIBRARIES, which is still empty (cmake/onnxruntime.cmake:124-133; tools/ci_build/github/apple/c/c.podspec.template:23-33). Now that 1DS uses system SQLite/zlib, static framework consumers can be left with unresolved sqlite3_*/zlib symbols. Add z and sqlite3 to the framework metadata when telemetry is enabled.

Static framework assembly cannot carry system SQLite and zlib archives; advertise both in CocoaPods framework metadata so consumers resolve telemetry symbols. Files changed: cmake/onnxruntime.cmake. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d3afa07-bc40-4851-bc49-ccf0a37e6da0

Copilot

Copilot review overview

🔵 Needs a closer look

The compatibility patch does not propagate mbedTLS threading configuration to fetched libcurl.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Propagate mbedTLS configuration macros to libcurl consumers

cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch:87

These configuration macros are private to the mbedTLS libraries, so fetched libcurl compiles against mbedTLS headers without the same threading configuration. The deleted ORT builder applied the macros to libcurl_static too, and the SDK's upstream implementation propagates them publicly. Use PUBLIC here (or also define them on libcurl) so producer and consumer see a consistent mbedTLS configuration.

The 3.10.240.1 compatibility patch now exports mbedTLS threading defines to fetched libcurl, matching the upstream SDK policy and avoiding inconsistent header configuration. Files changed: cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d3afa07-bc40-4851-bc49-ccf0a37e6da0

Copilot

Copilot review overview

🔵 Needs a closer look

Cross-platform telemetry packaging and Apple static-consumer behavior require final human validation.

Review effort: Balanced
Findings: None

Resolve the SDK pin and dependency conflicts in cmake/deps.txt and cmake/external/onnxruntime_external_deps.cmake. Remove the obsolete cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch while retaining SDK-owned Linux curl and the Apple SQLite/zlib packaging policy in cmake/CMakeLists.txt, cmake/onnxruntime.cmake, cmake/onnxruntime_common.cmake, cmake/vcpkg-ports/cpp-client-telemetry/vcpkg.json, and onnxruntime/core/platform/posix/telemetry.cc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4e6ddf54-85d1-4f61-b358-aa6b56895ddb

Copilot AI balanced review requested due to automatic review settings September 20, 2026 10:55

Copilot started reviewing on behalf of bmehta001 September 20, 2026 10:55 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

Comment thread cmake/external/onnxruntime_external_deps.cmake

bmehta001 requested a balanced review from Copilot September 29, 2026 04:58

Copilot started reviewing on behalf of bmehta001 September 29, 2026 04:59 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread cmake/patches/cpp_client_telemetry/cpp_client_telemetry.patch Outdated

bmehta001 requested a balanced review from Copilot September 29, 2026 05:08

Copilot started reviewing on behalf of bmehta001 September 29, 2026 05:08 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

bmehta001 requested a balanced review from Copilot September 29, 2026 05:15

Copilot started reviewing on behalf of bmehta001 September 29, 2026 05:16 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

bmehta001 requested a balanced review from Copilot September 29, 2026 05:22

Copilot started reviewing on behalf of bmehta001 September 29, 2026 05:23 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

bmehta001 self-assigned this Sep 30, 2026

bmehta001 enabled auto-merge (squash) September 30, 2026 21:57

Baiju Meswani (baijumeswani) approved these changes Sep 30, 2026

View reviewed changes

bmehta001 merged commit 3716f24 into main Oct 1, 2026
97 of 98 checks passed

bmehta001 deleted the bhamehta/standardize-telemetry-integration branch October 1, 2026 02:01

Navigation Menu

Standardize telemetry dependency policy - #32705

Standardize telemetry dependency policy#32705
bmehta001 merged 6 commits into
mainfrom
bhamehta/standardize-telemetry-integration

bmehta001 commented Sep 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bmehta001 commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bmehta001 commented Sep 20, 2026 •

edited

Loading