[lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

Mon Feb 24 08:39:34 PST 2025

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Vy Nguyen (oontvoo)

<details>
<summary>Changes</summary>

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
  + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default.
  + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance.

---
Full diff: https://github.com/llvm/llvm-project/pull/128534.diff


11 Files Affected:

- (modified) lldb/source/Core/CMakeLists.txt (+1-3) 
- (modified) lldb/source/Core/Telemetry.cpp (+2-5) 
- (modified) lldb/unittests/Core/CMakeLists.txt (+3-3) 
- (modified) llvm/CMakeLists.txt (+2-1) 
- (modified) llvm/cmake/modules/LLVMConfig.cmake.in (+1) 
- (modified) llvm/include/llvm/Config/llvm-config.h.cmake (+2) 
- (modified) llvm/include/llvm/Telemetry/Telemetry.h (+10-2) 
- (modified) llvm/lib/CMakeLists.txt (+2-3) 
- (modified) llvm/unittests/CMakeLists.txt (+1-3) 
- (modified) llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn (+1) 
- (modified) utils/bazel/llvm_configs/llvm-config.h.cmake (+2) 


``````````diff

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
-    : m_config(std::move(config)) {}
+    : m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
     CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_BUILD_TELEMETRY@)
+set(LLVM_EANBLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
 
 if (NOT "@LLVM_PTHREAD_LIB@" STREQUAL "")
   set(LLVM_PTHREAD_LIB "@LLVM_PTHREAD_LIB@")
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 239f9dd3f38db..13db9935870f8 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -202,6 +202,8 @@
 #cmakedefine LLVM_HAS_LOGF128
 
 /* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
 #cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
 
 #endif
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h b/llvm/include/llvm/Telemetry/Telemetry.h
index 42319f3ef51f2..f60c4c762ca7b 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -64,11 +64,19 @@ class Serializer {
 /// This struct can be extended as needed to add additional configuration
 /// points specific to a vendor's implementation.
 struct Config {
-  virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+  static bool BuildTimeEnableTelemetry = true;
+#else
+  static bool BuildTimeEnableTelemetry = false;
+#endif
+  virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
 
   // If true, telemetry will be enabled.
   const bool EnableTelemetry;
-  Config(bool E) : EnableTelemetry(E) {}
+
+  // Telemetry can only be enabled if both the runtime and buildtime flag
+  // are set.
+  Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
 
   virtual std::optional<std::string> makeSessionId() { return std::nullopt; }
 };
diff --git a/llvm/lib/CMakeLists.txt b/llvm/lib/CMakeLists.txt
index d0a2bc9294381..37b6fcf1236e3 100644
--- a/llvm/lib/CMakeLists.txt
+++ b/llvm/lib/CMakeLists.txt
@@ -41,9 +41,8 @@ add_subdirectory(ProfileData)
 add_subdirectory(Passes)
 add_subdirectory(TargetParser)
 add_subdirectory(TextAPI)
-if (LLVM_BUILD_TELEMETRY)
-  add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
+
 add_subdirectory(ToolDrivers)
 add_subdirectory(XRay)
 if (LLVM_INCLUDE_TESTS)
diff --git a/llvm/unittests/CMakeLists.txt b/llvm/unittests/CMakeLists.txt
index 12e229b1c3498..81abce51b8939 100644
--- a/llvm/unittests/CMakeLists.txt
+++ b/llvm/unittests/CMakeLists.txt
@@ -63,9 +63,7 @@ add_subdirectory(Support)
 add_subdirectory(TableGen)
 add_subdirectory(Target)
 add_subdirectory(TargetParser)
-if (LLVM_BUILD_TELEMETRY)
-  add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
 add_subdirectory(Testing)
 add_subdirectory(TextAPI)
 add_subdirectory(Transforms)
diff --git a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
index 5a13545a15b13..8f6c431a713af 100644
--- a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -293,6 +293,7 @@ write_cmake_config("llvm-config") {
     "LLVM_BUILD_LLVM_DYLIB=",
     "LLVM_BUILD_SHARED_LIBS=",
     "LLVM_BUILD_TELEMETRY=",
+    "LLVM_ENABLE_TELEMETRY=",
     "LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
     "LLVM_ENABLE_DUMP=",
     "LLVM_ENABLE_HTTPLIB=",
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 239f9dd3f38db..315ddfb37362c 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -202,6 +202,8 @@
 #cmakedefine LLVM_HAS_LOGF128
 
 /* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
 #cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
 
 #endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/128534
