diff --git a/ci/cloudbuild/builds/demo-install.sh b/ci/cloudbuild/builds/demo-install.sh index e28011290d..40f1870f2a 100755 --- a/ci/cloudbuild/builds/demo-install.sh +++ b/ci/cloudbuild/builds/demo-install.sh @@ -22,6 +22,7 @@ source module ci/install-dependencies.sh source module ci/cloudbuild/builds/lib/cmake.sh source module ci/cloudbuild/builds/lib/unit-tests.sh source module ci/lib/io.sh +cp ci/gha/builds/lib/google.googlebigqueryodbc.ini /opt/google.googlebigqueryodbc.ini cmake_config_testing_details=( # -DCMAKE_TOOLCHAIN_FILE="${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" @@ -46,6 +47,7 @@ cmake --build cmake-out -- -j "$(nproc)" cmake --build cmake-out --target install ## [DONE packaging.md] +export GOOGLEBIGQUERYODBCINI=/opt/google.googlebigqueryodbc.ini mapfile -t ctest_args < <(ctest::common_args) # I am unable to upgrade coreutils on Centos 7. So, # `env -C cmake-out ctest "${ctest_args[@]}"` throws diff --git a/google/cloud/odbc/bq_driver/internal/driver_log_form.cc b/google/cloud/odbc/bq_driver/internal/driver_log_form.cc index be0f5d067e..f6ca5f9942 100644 --- a/google/cloud/odbc/bq_driver/internal/driver_log_form.cc +++ b/google/cloud/odbc/bq_driver/internal/driver_log_form.cc @@ -27,7 +27,7 @@ std::string const kLogError = "LOG_ERROR"; std::string const kLogInfo = "LOG_INFO"; std::string const kLogWarning = "LOG_WARNING"; std::string const kDefaultMaxFiles = "50"; -std::string const kDefaultMaxSize = "20"; +std::string const kDefaultMaxSize = "2000"; Section LogTraceDialog::last_log_saved_values_; std::string LogTraceDialog::log_level_ = kLogOff; std::string LogTraceDialog::log_file_path_; @@ -165,7 +165,7 @@ void LogTraceDialog::InitControls() { // Max File Size (MB) Label and Edit Box HWND h_max_size_label = - CreateLabel(parent_hwnd, "Max file size (MB):", KAxisX, KAxisY + 125, + CreateLabel(parent_hwnd, "Max file size (KB):", KAxisX, KAxisY + 125, KLabelWidth + 60, kLabelHeight, 0); SendMessage(h_max_size_label, WM_SETFONT, (WPARAM)h_font, TRUE); diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_tables.cc b/google/cloud/odbc/bq_driver/internal/odbc_sql_tables.cc index ad47659430..2737b22b0d 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_tables.cc +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_tables.cc @@ -443,11 +443,8 @@ StatusRecordOr GetResultSetForTables( }; // 3. Execute tasks using the generic utility - int max_threads = 8; std::shared_ptr trace_option = TraceOptions::GetTraceOption(); - if (trace_option != nullptr && trace_option->max_threads > 0) { - max_threads = trace_option->max_threads; - } + int max_threads = trace_option->max_threads; auto parallel_results_or = ExecuteParallelTasks( max_threads, tasks, parallel_func); diff --git a/google/cloud/odbc/bq_driver/internal/trace_utils.cc b/google/cloud/odbc/bq_driver/internal/trace_utils.cc index addfdc0456..299e58014b 100644 --- a/google/cloud/odbc/bq_driver/internal/trace_utils.cc +++ b/google/cloud/odbc/bq_driver/internal/trace_utils.cc @@ -24,7 +24,7 @@ using ::google::cloud::odbc_internal::StatusRecord; using ::google::cloud::odbc_internal::StatusRecordOr; namespace fs = std::filesystem; -constexpr int kCharBufSize1 = 1024; +constexpr int kKB = 1024; constexpr int kCharBufSize2 = 256; static std::once_flag absl_log_init_flag; @@ -54,6 +54,11 @@ FileLogSink::FileLogSink(std::shared_ptr opts) if (!fp_) { return; } + if (std::filesystem::exists(current_file_)) { + current_file_size_ = std::filesystem::file_size(current_file_); + } else { + current_file_size_ = 0; + } } } @@ -64,49 +69,36 @@ FileLogSink::~FileLogSink() { fp_ = nullptr; } } -// Required for custom log formatting and writing to the driver's default log -// file +// Required for writing to the driver's default log file void FileLogSink::Send(absl::LogEntry const& entry) { std::lock_guard lock(log_mutex_); // Logging disabled or never initialized - if (!fp_ || !opts_) { - return; - } + if (!fp_ || !opts_) return; - auto message = entry.text_message_with_prefix_and_newline(); - std::size_t new_log_size = message.size() + 1; - std::uintmax_t max_file_size_bytes = opts_->max_file_size * 1024 * 1024; - - if (!CanWriteToFile(current_file_, new_log_size, max_file_size_bytes)) { - if (fp_ != nullptr) { - fclose(fp_); - fp_ = nullptr; - } + auto& opts = *opts_; + auto formatted_msg = GetFormattedMsg(entry); + std::size_t new_log_size = formatted_msg.size(); + std::uintmax_t max_file_size_bytes = + opts.max_file_size * kKB; // file size is in KB + if (current_file_size_ + new_log_size >= max_file_size_bytes) { + fclose(fp_); + fp_ = nullptr; // Remove the oldest log file (if limit reached), then advance to the next // index. - ClearOldLogFiles(opts_->log_path, opts_->current_file_index, - opts_->max_file_count); - ++opts_->current_file_index; + ClearOldLogFiles(opts.log_path, opts.current_file_index, + opts.max_file_count); + ++opts.current_file_index; - current_file_ = GetLogFileWithIndex(opts_->log_path); + current_file_ = GetLogFileWithIndex(opts.log_path); fp_ = fopen(current_file_.c_str(), "a"); - } - std::string time_str = absl::FormatTime( - "%Y-%m-%d %H:%M:%S", entry.timestamp(), absl::LocalTimeZone()); + if (!fp_) return; - auto log_message = std::string(entry.text_message()); - std::string log_tag = absl::LogSeverityName(entry.log_severity()); - - absl::string_view full_path = entry.source_filename(); - size_t last_sep = full_path.find_last_of("/\\"); - absl::string_view file_name = (last_sep == absl::string_view::npos) - ? full_path - : full_path.substr(last_sep + 1); - - absl::FPrintF(fp_, "[%s] [%s] [%s:%d] %s\n", log_tag, time_str, file_name, - entry.source_line(), entry.text_message()); + current_file_size_ = 0; + } + absl::FPrintF(fp_, "%s", formatted_msg.c_str()); fflush(fp_); + current_file_size_ += new_log_size; } absl::LogSeverity GetAbslSeverity(LogLevel level) { @@ -122,10 +114,30 @@ absl::LogSeverity GetAbslSeverity(LogLevel level) { } } +// Formats log msg in format: [LOG_LEVEL] [TIME] [FILE:LINE] MSG +std::string GetFormattedMsg(absl::LogEntry const& entry) { + static absl::TimeZone const kTimeZone = absl::LocalTimeZone(); + std::string time_str = + absl::FormatTime("%Y-%m-%d %H:%M:%S", entry.timestamp(), kTimeZone); + + char const* log_tag = absl::LogSeverityName(entry.log_severity()); + absl::string_view full_path = entry.source_filename(); + + size_t pos = full_path.rfind('/'); + if (pos == absl::string_view::npos) pos = full_path.rfind('\\'); + + absl::string_view file_name = + pos == absl::string_view::npos ? full_path : full_path.substr(pos + 1); + + std::string msg = + absl::StrFormat("[%s] [%s] [%s:%d] %s\n", log_tag, time_str, file_name, + entry.source_line(), entry.text_message()); + return msg; +} + void ClearOldLogFiles(std::string const& base_dir, int next_index, int max_file_count) { - // No rotation needed if only 1 file allowed - if (max_file_count <= 1) return; + if (max_file_count < 1) return; // Oldest index that must be deleted int oldest_index = next_index - (max_file_count - 1); @@ -197,16 +209,6 @@ void FileLogSink::InitializeFileLog( absl::log_internal::AddLogSink(file_sink_.get()); } -bool CanWriteToFile(std::string const& log_file, std::size_t new_log_size, - std::uintmax_t max_file_size_bytes) { - std::ifstream file(log_file, std::ios::binary | std::ios::ate); - if (!file.is_open()) { - return true; - } - std::uintmax_t current_file_size = file.tellg(); - return (current_file_size + new_log_size) <= max_file_size_bytes; -} - bool TraceOptions::InitializeLogging(bool is_trace_override) { // suppress all stderr output std::call_once(absl_log_init_flag, []() { absl::InitializeLog(); }); @@ -269,11 +271,17 @@ TraceOptions::CreateTraceOptionsFile( trace_sections = odbc_section->second; } + std::lock_guard lk(mu_); + if (options_file_ == nullptr) { + // Cannot use std::make_shared because constructor is protected. + options_file_ = std::shared_ptr(new TraceOptions()); + } + std::string log_path; - int log_level = 0; - int log_file_count; - int log_file_size; - int max_threads = 8; // default max_threads + int log_level = options_file_->log_level; + int log_file_count = options_file_->max_file_count; + int log_file_size = options_file_->max_file_size; + int max_threads = options_file_->max_threads; // default max_threads for (auto const& s : trace_sections) { if (s.first == kLogLevel && !s.second.empty()) { log_level = std::strtol(s.second.c_str(), nullptr, 10); @@ -288,12 +296,6 @@ TraceOptions::CreateTraceOptionsFile( } } - std::lock_guard lk(mu_); - if (options_file_ == nullptr) { - // Cannot use std::make_shared because constructor is protected. - options_file_ = std::shared_ptr(new TraceOptions()); - } - if (log_level > 0) { options_file_->log_level = log_level; options_file_->log_path = log_path; diff --git a/google/cloud/odbc/bq_driver/internal/trace_utils.h b/google/cloud/odbc/bq_driver/internal/trace_utils.h index cd63c416c7..c97622486d 100644 --- a/google/cloud/odbc/bq_driver/internal/trace_utils.h +++ b/google/cloud/odbc/bq_driver/internal/trace_utils.h @@ -102,8 +102,8 @@ struct TraceOptions { bool logging_enabled; bool is_file_closed; int log_level{0}; - int max_file_count{50}; // max number of log files (50). - int max_file_size{20}; // max file size of a single file(20 MB) + int max_file_count{50}; // max number of log files (50). + int max_file_size{2000}; // max file size of a single file(2000 KB) int max_threads{8}; int current_file_index{0}; std::string log_path; @@ -144,6 +144,7 @@ class FileLogSink : public absl::LogSink { std::shared_ptr opts_; std::string current_file_; + std::size_t current_file_size_; std::mutex log_mutex_; FILE* fp_ = nullptr; }; @@ -163,10 +164,9 @@ void UpdateTraceOption(std::optional log_level, std::optional log_file_size, std::optional log_file_count); -bool CanWriteToFile(std::string const& log_file, std::size_t new_log_size, - std::uintmax_t max_file_size_bytes); - std::string GetLogFileWithIndex(std::string const& log_path); +std::string GetFormattedMsg(absl::LogEntry const& entry); + //////////////////////////////////////////////////////////////////// // Additional Helper methods for validating and formatting strings // based on parameter types. diff --git a/google/cloud/odbc/bq_driver/internal/trace_utils_test.cc b/google/cloud/odbc/bq_driver/internal/trace_utils_test.cc index ceadccdbb2..d75e3b3227 100644 --- a/google/cloud/odbc/bq_driver/internal/trace_utils_test.cc +++ b/google/cloud/odbc/bq_driver/internal/trace_utils_test.cc @@ -81,27 +81,6 @@ TEST(GetLogFileWithIndex, CustomLogPath) { EXPECT_EQ(actual, expected); } -TEST(CanWriteToFile, AllSecnarios) { - std::string file_name = "sample.log"; - - // File doesn't exist - EXPECT_TRUE(CanWriteToFile(file_name, 100, 1000)); - - // File exist and can store more data - CreateTestFile(file_name, 200); - EXPECT_TRUE(CanWriteToFile(file_name, 100, 500)); - - // File exists and writing would exceed limit - CreateTestFile(file_name, 900); - EXPECT_FALSE(CanWriteToFile(file_name, 100, 900)); - - // File exists and exactly fills the limit - CreateTestFile(file_name, 800); - EXPECT_TRUE(CanWriteToFile(file_name, 200, 1000)); - - DeleteTestFile(file_name); -} - TEST(ClearOldLogFiles, WhenMaxFileCountIsOne) { std::string dir = std::filesystem::temp_directory_path().string(); std::string file = dir + "/Tracetestingcountone_0.log"; diff --git a/google/cloud/odbc/bq_driver/internal/utils.cc b/google/cloud/odbc/bq_driver/internal/utils.cc index ddaaafff80..66bf24e467 100644 --- a/google/cloud/odbc/bq_driver/internal/utils.cc +++ b/google/cloud/odbc/bq_driver/internal/utils.cc @@ -214,11 +214,7 @@ StatusRecordOr> ParseConfig( 0, KEY_READ, &key_handle); if (status != ERROR_SUCCESS) { RegCloseKey(key_handle); - LOG(ERROR) - << "ParseConfig::RegOpenKeyEx:: Can't open registry key with path: " - << registry_key; - return StatusRecord{SQLStates::k_HY000(), - "Can't open registry key with path: " + registry_key}; + return std::make_shared(); } TCHAR subkey_name[kMaxKeyLength]; @@ -521,13 +517,9 @@ StatusRecordOr> ParseConfig( } } } - } else { - std::string msg = "Can't open file with path: "; - msg.append(file_path); - LOG(ERROR) << "ParseConfig::ifstream:: " << msg; - return StatusRecord{SQLStates::k_HY000(), msg}; + return std::make_shared(sections); } - return std::make_shared(sections); + return std::make_shared(); } #endif //_WIN32 diff --git a/google/cloud/odbc/bq_driver/internal/utils_test.cc b/google/cloud/odbc/bq_driver/internal/utils_test.cc index f1a14e3d2b..c06fa31f93 100644 --- a/google/cloud/odbc/bq_driver/internal/utils_test.cc +++ b/google/cloud/odbc/bq_driver/internal/utils_test.cc @@ -193,14 +193,10 @@ TEST(Parsing, ParseConfigIncorrectPath) { #else auto sections = ParseConfig("SOFTWARE\\WOW6432Node\\ODBC\\ODBC1.INI"); #endif - EXPECT_THAT(sections, - StatusRecordIs(SQLStates::k_HY000(), - HasSubstr("Can't open registry key with path"))); #else auto sections = ParseConfig("/invalid_file_name.ini"); - EXPECT_THAT(sections, StatusRecordIs(SQLStates::k_HY000(), - HasSubstr("Can't open file"))); #endif // _WIN32 + EXPECT_TRUE((*sections)->empty()); } #ifndef _WIN32 @@ -245,8 +241,15 @@ TEST(GetPathToOdbcIni, GetEmptyPath) { } TEST(GetOdbcTraceConfigPath, GetDefaultPath) { + auto home = ::google::cloud::internal::GetEnv("GOOGLEBIGQUERYODBCINI"); + // Need to remove the environment variable to use the default path + google::cloud::odbc_bigquery_client_interface::UnsetEnv( + "GOOGLEBIGQUERYODBCINI"); + std::string actual = GetOdbcTraceConfigPath(); EXPECT_EQ(actual, "/etc/google.googlebigqueryodbc.ini"); + google::cloud::odbc_bigquery_client_interface::SetEnv("GOOGLEBIGQUERYODBCINI", + home); } TEST(GetOdbcTraceConfigPath, GetGoogleODBCIniPath) {