Skip to content

Commit 80a55e7

Browse files
fix(bq_driver): Fix log file output issue during Git-based test runs and stderr issue
1 parent 3f1fa96 commit 80a55e7

10 files changed

Lines changed: 59 additions & 35 deletions

File tree

ci/cloudbuild/dockerfiles/ubuntu-22.04-install.Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ ENV PATH=${CLOUD_SDK_LOCATION}/bin:${PATH}
261261
COPY ./gha/builds/lib/odbc.ini /opt/odbc-driver/odbc.ini
262262
COPY ./gha/builds/lib/odbcinst.ini /opt/odbc-driver/odbcinst.ini
263263
COPY ./gha/builds/lib/lsan.supp /opt/odbc-driver/lsan.supp
264+
COPY ./gha/builds/lib/google.googlebigqueryodbc.ini /opt/odbc-driver/google.googlebigqueryodbc.ini
264265

265266
# glibc 2.17 or later
266267
RUN echo 'Installing glibc...'

ci/dependencies/driver-manager-setup-google-driver.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export LD_LIBRARY_PATH=${LD_LIBRARY_PATH:-}:/usr/local/lib/
5050
export ODBCSYSINI=/opt/odbc-driver
5151
export ODBCINI=/opt/odbc-driver/odbc.ini
5252
export CPP_BIGQUERY_ODBC_TEST_SERVICE_ACCOUNT_AUTH_KEY=/opt/odbc-driver/connection/key.json
53-
53+
export GOOGLEBIGQUERYODBCINI=/opt/odbc-driver/google.googlebigqueryodbc.ini
5454
cd "$CPP_GOOGLE_BIGQUERY_ODBC_DRIVER_MANAGER_SETUP_CURR_DIR"
5555

5656
echo '**** ODBC Driver installation END****'
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[Driver]
2+
LogLevel=0
3+
LogPath=

ci/gha/builds/lib/system_dsn_setup_32.reg

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,5 @@ REGEDIT4
5555
"LogFile"="C:\\b\\trace.log"
5656

5757
[HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Google\ODBC Driver for Google BigQuery\Driver]
58-
"LogLevel"="1"
59-
"LogFile"="C:\\b\\trace.log"
58+
"LogLevel"="0"
59+
"LogFile"=

ci/gha/builds/lib/system_dsn_setup_64.reg

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,5 @@ REGEDIT4
5555
"LogFile"="C:\\b\\trace.log"
5656

5757
[HKEY_LOCAL_MACHINE\SOFTWARE\Google\ODBC Driver for Google BigQuery\Driver]
58-
"LogLevel"="1"
59-
"LogFile"="C:\\b\\trace.log"
58+
"LogLevel"="0"
59+
"LogFile"=

ci/gha/builds/macos-cmake.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ source module ci/gha/builds/lib/cmake.sh
2222

2323
cp ci/gha/builds/lib/odbc_osx.ini /Users/runner/work/connection/odbc-driver/odbc.ini
2424
cp ci/gha/builds/lib/odbcinst_osx.ini /Users/runner/work/connection/odbc-driver/odbcinst.ini
25+
cp ci/gha/builds/lib/google.googlebigqueryodbc.ini /Users/runner/work/connection/google.googlebigqueryodbc.ini
2526
export ODBCINI=/Users/runner/work/connection/odbc-driver/odbc.ini
2627
export ODBCINSTINI=/Users/runner/work/connection/odbc-driver/odbcinst.ini
28+
export GOOGLEBIGQUERYODBCINI=/Users/runner/work/connection/google.googlebigqueryodbc.ini
2729
export ODBC_TESTS_DSN="SampleDSNGoogleDriver"
2830
export CPP_BIGQUERY_ODBC_TEST_SERVICE_ACCOUNT_AUTH_KEY=/Users/runner/work/connection/key.json
2931

google/cloud/odbc/bq_driver/internal/trace_utils.cc

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ static std::once_flag absl_log_init_flag;
3333
std::shared_ptr<TraceOptions> TraceOptions::options_console_ = nullptr;
3434
std::shared_ptr<TraceOptions> TraceOptions::options_file_ = nullptr;
3535
std::mutex TraceOptions::mu_;
36-
static std::mutex sink_mutex; // Global for the file
3736

3837
#ifdef _WIN32
3938
constexpr char kPathSeparator = '\\';
@@ -54,10 +53,10 @@ FileLogSink::FileLogSink(std::shared_ptr<TraceOptions> opts)
5453
}
5554

5655
FileLogSink::~FileLogSink() {
57-
std::lock_guard<std::mutex> lock(sink_mutex);
58-
if (file_sink_ && file_sink_.get() == this) {
59-
absl::log_internal::RemoveLogSink(this);
60-
file_sink_ = nullptr;
56+
// Close the file pointer if it was opened
57+
if (fp_ != nullptr) {
58+
fclose(fp_);
59+
fp_ = nullptr;
6160
}
6261
}
6362
// Required for custom log formatting and writing to the driver's default log
@@ -152,9 +151,13 @@ std::string GetLogFileWithIndex(std::string const& log_path) {
152151

153152
void FileLogSink::InitializeFileLog(
154153
std::shared_ptr<TraceOptions> const& trace_opts) {
155-
std::lock_guard<std::mutex> lock(sink_mutex);
156154
if (file_sink_ || !trace_opts) return;
157155

156+
if (file_sink_) {
157+
absl::log_internal::RemoveLogSink(file_sink_.get());
158+
file_sink_ = nullptr;
159+
}
160+
158161
file_sink_ = std::make_unique<FileLogSink>(trace_opts);
159162
absl::log_internal::AddLogSink(file_sink_.get());
160163
}
@@ -170,9 +173,19 @@ bool CanWriteToFile(std::string const& log_file, std::size_t new_log_size,
170173
}
171174

172175
bool TraceOptions::InitializeLogging(bool is_trace_override) {
176+
// suppress all stderr output
177+
std::call_once(absl_log_init_flag, []() { absl::InitializeLog(); });
178+
absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfinity);
179+
173180
if (!kTraceOptsFile.Ok()) return false;
174181
auto const& trace_opts = kTraceOptsFile.GetValue();
175182

183+
// If logging is disabled, return false
184+
if (trace_opts->log_level <= 0) {
185+
trace_opts->logging_enabled = false;
186+
return false;
187+
}
188+
176189
// Logging already initialized and no override requested
177190
if (trace_opts->logging_enabled && !is_trace_override) {
178191
return true;
@@ -186,23 +199,14 @@ bool TraceOptions::InitializeLogging(bool is_trace_override) {
186199
FileLogSink::InitializeFileLog(trace_opts);
187200
return true;
188201
}
189-
// If logging is disabled, suppress all stderr output
190-
if (trace_opts->log_level <= 0) {
191-
absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfinity);
192-
return false;
193-
}
194202

195203
// Initialize Abseil logging and custom file sink
196-
std::call_once(absl_log_init_flag, []() { absl::InitializeLog(); });
197204
auto log_severity =
198205
GetAbslSeverity(static_cast<LogLevel>(trace_opts->log_level));
199206
absl::SetMinLogLevel(static_cast<absl::LogSeverityAtLeast>(log_severity));
200207

201208
FileLogSink::InitializeFileLog(trace_opts);
202209
trace_opts->logging_enabled = true;
203-
204-
// Disable Abseil's stderr logging
205-
absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfinity);
206210
return true;
207211
}
208212

google/cloud/odbc/bq_driver/internal/utils.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -480,14 +480,8 @@ std::string GetOdbcTraceConfigPath() {
480480
if (path) {
481481
return *path;
482482
}
483-
absl::optional<std::string> home = google::cloud::internal::GetEnv("HOME");
484-
if (home) {
485-
home = *home +
486-
"/GoogleODBCDriverForBigQuery/lib/google.googlebigqueryodbc.ini";
487-
return *home;
488-
}
489-
// Default to using ~ path directly if HOME is not set.
490-
return "~/GoogleODBCDriverForBigQuery/lib/google.googlebigqueryodbc.ini";
483+
// Default to using ~ path directly
484+
return "/etc/google.googlebigqueryodbc.ini";
491485
#else
492486
return k_trace_reg_path;
493487
#endif // _WIN32

google/cloud/odbc/bq_driver/internal/utils_test.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,14 +239,8 @@ TEST(GetPathToOdbcIni, GetEmptyPath) {
239239
}
240240

241241
TEST(GetOdbcTraceConfigPath, GetDefaultPath) {
242-
auto home = ::google::cloud::internal::GetEnv("HOME");
243-
google::cloud::odbc_bigquery_client_interface::UnsetEnv("HOME");
244-
245242
std::string actual = GetOdbcTraceConfigPath();
246-
EXPECT_EQ(actual,
247-
"~/GoogleODBCDriverForBigQuery/lib/google.googlebigqueryodbc.ini");
248-
249-
google::cloud::odbc_bigquery_client_interface::SetEnv("HOME", home);
243+
EXPECT_EQ(actual, "/etc/google.googlebigqueryodbc.ini");
250244
}
251245

252246
TEST(GetOdbcTraceConfigPath, GetGoogleODBCIniPath) {

google/cloud/odbc/integration_tests/odbc_driver_tests/connection_test.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,6 +1556,32 @@ TEST(SQLGetFunctionsInternal,
15561556
SQLGetFunctions(conn->hdbc, SQL_API_ODBC3_ALL_FUNCTIONS, nullptr));
15571557
}
15581558

1559+
TEST(ConnectionTest, CheckTraceLogFileExist) {
1560+
#ifdef _WIN32
1561+
std::string log_path = "C:\\b";
1562+
std::string log_file = log_path + "\\googleodbcdriverforbigquery_0.log";
1563+
#else
1564+
std::string log_path = "/tmp";
1565+
std::string log_file = log_path + "/googleodbcdriverforbigquery_0.log";
1566+
#endif // _WIN32
1567+
auto conn_str =
1568+
kDefaultConnectionString + ";LogPath=" + log_path + ";LogLevel=3";
1569+
1570+
auto conn = std::make_shared<ODBCHandles>();
1571+
EXPECT_EQ(Connect(conn_str, conn), SQL_SUCCESS);
1572+
EXPECT_EQ(Disconnect(conn), SQL_SUCCESS);
1573+
// check if the file exists
1574+
EXPECT_TRUE(std::filesystem::exists(log_file));
1575+
// Check that the file is not empty
1576+
std::ifstream file(log_file, std::ios::binary);
1577+
ASSERT_TRUE(file.is_open());
1578+
1579+
file.seekg(0, std::ios::end);
1580+
auto size = file.tellg();
1581+
EXPECT_GT(size, 0);
1582+
file.close();
1583+
}
1584+
15591585
#endif // BQ_DRIVER_INTEGRATION_TESTS
15601586

15611587
} // namespace google::cloud::odbc_tests

0 commit comments

Comments
 (0)