Skip to content

[llvm] Add CMake flag to compile out the telemetry framework #124850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

JDevlieghere
Copy link
Member

Add a CMake flag (LLVM_BUILD_TELEMETRY) to disable building the telemetry framework. The flag being enabled does not mean that telemetry is being collected, it merely means we're building the generic telemetry framework. Hence the flag is enabled by default.

Motivated by this Discourse thread: https://discourse.llvm.org/t/how-to-disable-building-llvm-clang-telemetry/84305

@tstellar
Copy link
Collaborator

Does this change need to go to the release//20.x branch?

@JDevlieghere
Copy link
Member Author

Does this change need to go to the release//20.x branch?

Yup, I'll nominate it when the PR is approved.

Add a CMake flag (LLVM_BUILD_TELEMETRY) to disable building the
telemetry framework. The flag being enabled does *not* mean that
telemetry is being collected, it merely means we're building the generic
telemetry framework. Hence the flag is enabled by default.

Motivated by this Discourse thread: https://discourse.llvm.org/t/how-to-disable-building-llvm-clang-telemetry/84305
Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like @oontvoo to confirm before this gets merged, if possible.

@oontvoo
Copy link
Member

oontvoo commented Jan 31, 2025

Quick question: why isn't using the existing option to disable the framework sufficient?

Otherwise, LGTM.

@JDevlieghere
Copy link
Member Author

Quick question: why isn't using the existing option to disable the framework sufficient?

By the existing option, I assume you're referring to the Telemetry::Config struct? I can't speak for the original author on Discourse, but I would assume it's because it's much easier to verify statically that you don't have any telemetry code when it has been compiled out (e.g. you can run strings on the binary) compared to dynamically where you have to prove that code is never called.

@JDevlieghere JDevlieghere merged commit bac62ee into llvm:main Feb 3, 2025
6 of 8 checks passed
@JDevlieghere JDevlieghere deleted the telemetry branch February 3, 2025 18:35
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 4, 2025

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2223

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/38/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-12124-38-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=38 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 7, 2025
…4850)

Add a CMake flag (LLVM_BUILD_TELEMETRY) to disable building the
telemetry framework. The flag being enabled does *not* mean that
telemetry is being collected, it merely means we're building the generic
telemetry framework. Hence the flag is enabled by default.

Motivated by this Discourse thread:
https://discourse.llvm.org/t/how-to-disable-building-llvm-clang-telemetry/84305

(cherry picked from commit bac62ee)
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…4850)

Add a CMake flag (LLVM_BUILD_TELEMETRY) to disable building the
telemetry framework. The flag being enabled does *not* mean that
telemetry is being collected, it merely means we're building the generic
telemetry framework. Hence the flag is enabled by default.

Motivated by this Discourse thread:
https://discourse.llvm.org/t/how-to-disable-building-llvm-clang-telemetry/84305
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.

7 participants