Address misc review points

benibus · May 22, 2024 · 85fde99 · 85fde99
1 parent f45f94e
commit 85fde99
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 20 deletions.
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -4694,12 +4694,6 @@ macro(build_opentelemetry)
        -DWITH_OTLP_GRPC=OFF
        # Disabled because it seemed to cause linking errors. May be worth a closer look.
        -DWITH_FUNC_TESTS=OFF
-       # These options are slated for removal in v1.14 and their features are deemed stable
-       # as of v1.13. However, setting their corresponding ENABLE_* macros in headers seems
-       # finicky - resulting in build failures or ABI-related runtime errors during HTTP
-       # client initialization. There may still be a solution, but we disable them for now.
-       -DWITH_OTLP_HTTP_SSL_PREVIEW=OFF
-       -DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=OFF
        "-DProtobuf_INCLUDE_DIR=${OPENTELEMETRY_PROTOBUF_INCLUDE_DIR}"
        "-DProtobuf_LIBRARY=${OPENTELEMETRY_PROTOBUF_INCLUDE_DIR}"
        "-DProtobuf_PROTOC_EXECUTABLE=${OPENTELEMETRY_PROTOC_EXECUTABLE}")

diff --git a/cpp/src/arrow/flight/sql/test_app_cli.cc b/cpp/src/arrow/flight/sql/test_app_cli.cc
@@ -108,11 +108,6 @@ class OtelScope {
  private:
   opentelemetry::trace::Scope scope_;
 };
-#else
-class OtelScope {
- public:
-  static Result<std::unique_ptr<OtelScope>> Make() { return nullptr; }
-};
 #endif
 
 Status PrintResultsForEndpoint(FlightSqlClient& client,
@@ -158,7 +153,9 @@ Status PrintResults(FlightSqlClient& client, const FlightCallOptions& call_optio
 }
 
 Status RunMain() {
+#ifdef ARROW_TELEMETRY
   ARROW_ASSIGN_OR_RAISE(auto otel_scope, OtelScope::Make());
+#endif
 
   ARROW_ASSIGN_OR_RAISE(auto location, Location::ForGrpcTcp(FLAGS_host, FLAGS_port));
   auto client_options = FlightClientOptions::Defaults();

diff --git a/cpp/src/arrow/flight/sql/test_server_cli.cc b/cpp/src/arrow/flight/sql/test_server_cli.cc
@@ -61,12 +61,12 @@ arrow::Status SetupOTel() {
 
   return arrow::Status::OK();
 }
-#else
-arrow::Status SetupOTel() { return arrow::Status::OK(); }
 #endif
 
 arrow::Status RunMain() {
+#ifdef ARROW_TELEMETRY
   ARROW_RETURN_NOT_OK(SetupOTel());
+#endif
 
   ARROW_ASSIGN_OR_RAISE(auto location,
                         arrow::flight::Location::ForGrpcTcp("0.0.0.0", FLAGS_port));

diff --git a/cpp/src/arrow/telemetry/logging.cc b/cpp/src/arrow/telemetry/logging.cc
@@ -104,17 +104,17 @@ class OtlpOStreamLogRecordExporter final : public otel::sdk::logs::LogRecordExpo
 otel::logs::Severity ToOtelSeverity(LogLevel level) {
   switch (level) {
     case LogLevel::ARROW_TRACE:
-      return opentelemetry::logs::Severity::kTrace;
+      return otel::logs::Severity::kTrace;
     case LogLevel::ARROW_DEBUG:
-      return opentelemetry::logs::Severity::kDebug;
+      return otel::logs::Severity::kDebug;
     case LogLevel::ARROW_INFO:
-      return opentelemetry::logs::Severity::kInfo;
+      return otel::logs::Severity::kInfo;
     case LogLevel::ARROW_WARNING:
-      return opentelemetry::logs::Severity::kWarn;
+      return otel::logs::Severity::kWarn;
     case LogLevel::ARROW_ERROR:
-      return opentelemetry::logs::Severity::kError;
+      return otel::logs::Severity::kError;
     case LogLevel::ARROW_FATAL:
-      return opentelemetry::logs::Severity::kFatal;
+      return otel::logs::Severity::kFatal;
   }
   return otel::logs::Severity::kInvalid;
 }
@@ -133,7 +133,6 @@ std::unique_ptr<otel::sdk::logs::LogRecordExporter> MakeExporter(
     } break;
     case ExporterKind::OTLP_HTTP: {
       namespace otlp = otel::exporter::otlp;
-      // TODO: Allow user configuration here?
       otlp::OtlpHttpLogRecordExporterOptions options{};
       return std::make_unique<otlp::OtlpHttpLogRecordExporter>(options);
     } break;