Skip to content

Commit 9b96bdb

Browse files
authored
GH-47518: [C++][FlightRPC] Replace spdlogs with Arrow's Internal Logging (#47645)
### Rationale for this change Replace `spdlogs` with Arrow's Internal Logging. ### What changes are included in this PR? - removed `spdlogs` fetch and build - replaced odbc logging framework with Arrow's internal logging - Build fixes: - use C++ 20 - dsn window minor static cast fix ### Are these changes tested? - yes, on local Windows environment ### Are there any user-facing changes? No * GitHub Issue: #47518 Lead-authored-by: Alina (Xi) Li <[email protected]> Co-authored-by: Alina (Xi) Li <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent 19e3f90 commit 9b96bdb

File tree

12 files changed

+46
-347
lines changed

12 files changed

+46
-347
lines changed

cpp/src/arrow/flight/sql/odbc/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,9 @@
1717

1818
add_custom_target(arrow_flight_sql_odbc)
1919

20+
# Use C++ 20 for ODBC and its subdirectory
21+
# GH-44792: Arrow will switch to C++ 20
22+
set(CMAKE_CXX_STANDARD 20)
23+
2024
add_subdirectory(flight_sql)
2125
add_subdirectory(odbcabstraction)

cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc

Lines changed: 39 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,41 +18,31 @@
1818
#include "arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h"
1919
#include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h"
2020
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
21-
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h"
22-
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h"
21+
#include "arrow/util/io_util.h"
22+
#include "arrow/util/logging.h"
23+
#include "arrow/util/string.h"
2324

24-
#define DEFAULT_MAXIMUM_FILE_SIZE 16777216
25-
#define CONFIG_FILE_NAME "arrow-odbc.ini"
25+
using arrow::util::ArrowLogLevel;
2626

2727
namespace driver {
2828
namespace flight_sql {
29+
static constexpr const char* kODBCLogLevel = "ARROW_ODBC_LOG_LEVEL";
2930

3031
using odbcabstraction::Connection;
31-
using odbcabstraction::LogLevel;
3232
using odbcabstraction::OdbcVersion;
33-
using odbcabstraction::SPDLogger;
3433

35-
namespace {
36-
LogLevel ToLogLevel(int64_t level) {
37-
switch (level) {
38-
case 0:
39-
return LogLevel::LogLevel_TRACE;
40-
case 1:
41-
return LogLevel::LogLevel_DEBUG;
42-
case 2:
43-
return LogLevel::LogLevel_INFO;
44-
case 3:
45-
return LogLevel::LogLevel_WARN;
46-
case 4:
47-
return LogLevel::LogLevel_ERROR;
48-
default:
49-
return LogLevel::LogLevel_OFF;
50-
}
34+
FlightSqlDriver::FlightSqlDriver()
35+
: diagnostics_("Apache Arrow", "Flight SQL", OdbcVersion::V_3), version_("0.9.0.0") {
36+
RegisterLog();
5137
}
52-
} // namespace
5338

54-
FlightSqlDriver::FlightSqlDriver()
55-
: diagnostics_("Apache Arrow", "Flight SQL", OdbcVersion::V_3), version_("0.9.0.0") {}
39+
FlightSqlDriver::~FlightSqlDriver() {
40+
// Unregister log if logging is enabled
41+
if (arrow::internal::GetEnvVar(kODBCLogLevel).ValueOr("").empty()) {
42+
return;
43+
}
44+
arrow::util::ArrowLog::ShutDownArrowLog();
45+
}
5646

5747
std::shared_ptr<Connection> FlightSqlDriver::CreateConnection(OdbcVersion odbc_version) {
5848
return std::make_shared<FlightSqlConnection>(odbc_version, version_);
@@ -63,45 +53,36 @@ odbcabstraction::Diagnostics& FlightSqlDriver::GetDiagnostics() { return diagnos
6353
void FlightSqlDriver::SetVersion(std::string version) { version_ = std::move(version); }
6454

6555
void FlightSqlDriver::RegisterLog() {
66-
odbcabstraction::PropertyMap propertyMap;
67-
driver::odbcabstraction::ReadConfigFile(propertyMap, CONFIG_FILE_NAME);
68-
69-
auto log_enable_iterator = propertyMap.find(SPDLogger::LOG_ENABLED);
70-
auto log_enabled = log_enable_iterator != propertyMap.end()
71-
? odbcabstraction::AsBool(log_enable_iterator->second)
72-
: false;
73-
if (!log_enabled) {
56+
std::string log_level_str = arrow::internal::GetEnvVar(kODBCLogLevel)
57+
.Map(arrow::internal::AsciiToLower)
58+
.Map(arrow::internal::TrimString)
59+
.ValueOr("");
60+
if (log_level_str.empty()) {
7461
return;
7562
}
7663

77-
auto log_path_iterator = propertyMap.find(SPDLogger::LOG_PATH);
78-
auto log_path = log_path_iterator != propertyMap.end() ? log_path_iterator->second : "";
79-
if (log_path.empty()) {
80-
return;
64+
auto log_level = ArrowLogLevel::ARROW_DEBUG;
65+
66+
if (log_level_str == "fatal") {
67+
log_level = ArrowLogLevel::ARROW_FATAL;
68+
} else if (log_level_str == "error") {
69+
log_level = ArrowLogLevel::ARROW_ERROR;
70+
} else if (log_level_str == "warning") {
71+
log_level = ArrowLogLevel::ARROW_WARNING;
72+
} else if (log_level_str == "info") {
73+
log_level = ArrowLogLevel::ARROW_INFO;
74+
} else if (log_level_str == "debug") {
75+
log_level = ArrowLogLevel::ARROW_DEBUG;
76+
} else if (log_level_str == "trace") {
77+
log_level = ArrowLogLevel::ARROW_TRACE;
8178
}
8279

83-
auto log_level_iterator = propertyMap.find(SPDLogger::LOG_LEVEL);
84-
auto log_level = ToLogLevel(log_level_iterator != propertyMap.end()
85-
? std::stoi(log_level_iterator->second)
86-
: 1);
87-
if (log_level == odbcabstraction::LogLevel_OFF) {
88-
return;
80+
// Enable driver logging. Log files are not supported on Windows yet, since GLOG is not
81+
// tested fully on Windows.
82+
// Info log level is enabled by default.
83+
if (log_level != ArrowLogLevel::ARROW_INFO) {
84+
arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level);
8985
}
90-
91-
auto maximum_file_size_iterator = propertyMap.find(SPDLogger::MAXIMUM_FILE_SIZE);
92-
auto maximum_file_size = maximum_file_size_iterator != propertyMap.end()
93-
? std::stoi(maximum_file_size_iterator->second)
94-
: DEFAULT_MAXIMUM_FILE_SIZE;
95-
96-
auto maximum_file_quantity_iterator = propertyMap.find(SPDLogger::FILE_QUANTITY);
97-
auto maximum_file_quantity = maximum_file_quantity_iterator != propertyMap.end()
98-
? std::stoi(maximum_file_quantity_iterator->second)
99-
: 1;
100-
101-
std::unique_ptr<odbcabstraction::SPDLogger> logger(new odbcabstraction::SPDLogger());
102-
103-
logger->init(maximum_file_quantity, maximum_file_size, log_path, log_level);
104-
odbcabstraction::Logger::SetInstance(std::move(logger));
10586
}
10687

10788
} // namespace flight_sql

cpp/src/arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class FlightSqlDriver : public odbcabstraction::Driver {
3030

3131
public:
3232
FlightSqlDriver();
33+
~FlightSqlDriver();
3334

3435
std::shared_ptr<odbcabstraction::Connection> CreateConnection(
3536
odbcabstraction::OdbcVersion odbc_version) override;

cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ int DsnConfigurationWindow::CreateEncryptionSettingsGroup(int posX, int posY, in
280280
rightCheckPosX, rowPos - 2, 20, 2 * ROW_HEIGHT, "",
281281
ChildId::DISABLE_CERT_VERIFICATION_CHECKBOX, disableCertVerification);
282282

283-
rowPos += INTERVAL + static_cast<int>(1.5 * ROW_HEIGHT);
283+
rowPos += INTERVAL + static_cast<int>(1.5 * static_cast<int>(ROW_HEIGHT));
284284

285285
encryptionSettingsGroupBox =
286286
CreateGroupBox(posX, posY, sizeX, rowPos - posY, "Encryption settings",

cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ add_library(odbcabstraction
2525
include/odbcabstraction/diagnostics.h
2626
include/odbcabstraction/error_codes.h
2727
include/odbcabstraction/exceptions.h
28-
include/odbcabstraction/logger.h
2928
include/odbcabstraction/platform.h
30-
include/odbcabstraction/spd_logger.h
3129
include/odbcabstraction/types.h
3230
include/odbcabstraction/utils.h
3331
include/odbcabstraction/odbc_impl/attribute_utils.h
@@ -47,8 +45,6 @@ add_library(odbcabstraction
4745
diagnostics.cc
4846
encoding.cc
4947
exceptions.cc
50-
logger.cc
51-
spd_logger.cc
5248
utils.cc
5349
../../../../vendored/whereami/whereami.cc
5450
odbc_impl/odbc_connection.cc
@@ -65,20 +61,3 @@ set_target_properties(odbcabstraction
6561
${CMAKE_BINARY_DIR}/$<CONFIG>/lib
6662
RUNTIME_OUTPUT_DIRECTORY
6763
${CMAKE_BINARY_DIR}/$<CONFIG>/lib)
68-
69-
include(FetchContent)
70-
fetchcontent_declare(spdlog
71-
URL https://github.com/gabime/spdlog/archive/76fb40d95455f249bd70824ecfcae7a8f0930fa3.zip
72-
CONFIGURE_COMMAND
73-
""
74-
BUILD_COMMAND
75-
"")
76-
fetchcontent_getproperties(spdlog)
77-
if(NOT spdlog_POPULATED)
78-
fetchcontent_populate(spdlog)
79-
endif()
80-
81-
add_library(spdlog INTERFACE)
82-
target_include_directories(spdlog INTERFACE ${spdlog_SOURCE_DIR}/include)
83-
84-
target_link_libraries(odbcabstraction PUBLIC spdlog)

cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h

Lines changed: 0 additions & 67 deletions
This file was deleted.

cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h

Lines changed: 0 additions & 54 deletions
This file was deleted.

cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717

1818
#pragma once
1919

20-
#include <arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h>
21-
#include <arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spi/connection.h>
22-
#include <boost/algorithm/string.hpp>
2320
#include <string>
21+
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spi/connection.h"
2422

2523
namespace driver {
2624
namespace odbcabstraction {
@@ -51,8 +49,5 @@ boost::optional<bool> AsBool(const Connection::ConnPropertyMap& connPropertyMap,
5149
boost::optional<int32_t> AsInt32(int32_t min_value,
5250
const Connection::ConnPropertyMap& connPropertyMap,
5351
const std::string_view& property_name);
54-
55-
void ReadConfigFile(PropertyMap& properties, const std::string& configFileName);
56-
5752
} // namespace odbcabstraction
5853
} // namespace driver

cpp/src/arrow/flight/sql/odbc/odbcabstraction/logger.cc

Lines changed: 0 additions & 32 deletions
This file was deleted.

0 commit comments

Comments
 (0)