diff --git a/.gitignore b/.gitignore index 6bd57911ac8..afb4e67a1b8 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ /build /build_* /build-* +/tests/venv /docs/build /docs/publish diff --git a/.gitmodules b/.gitmodules index c05da0c9ff9..2fed57a519d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -168,3 +168,6 @@ [submodule "contrib/fmtlib"] path = contrib/fmtlib url = https://github.com/fmtlib/fmt.git +[submodule "contrib/sentry-native"] + path = contrib/sentry-native + url = https://github.com/getsentry/sentry-native.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 218b8499f4c..943bc6412b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -362,6 +362,7 @@ include (cmake/find/orc.cmake) include (cmake/find/avro.cmake) include (cmake/find/msgpack.cmake) include (cmake/find/cassandra.cmake) +include (cmake/find/sentry.cmake) find_contrib_lib(cityhash) find_contrib_lib(farmhash) diff --git a/base/daemon/BaseDaemon.cpp b/base/daemon/BaseDaemon.cpp index 33dee5d4a63..e7ccf84d7da 100644 --- a/base/daemon/BaseDaemon.cpp +++ b/base/daemon/BaseDaemon.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -288,7 +289,7 @@ private: std::stringstream bare_stacktrace; bare_stacktrace << "Stack trace:"; for (size_t i = stack_trace.getOffset(); i < stack_trace.getSize(); ++i) - bare_stacktrace << ' ' << stack_trace.getFrames()[i]; + bare_stacktrace << ' ' << stack_trace.getFramePointers()[i]; LOG_FATAL(log, bare_stacktrace.str()); } @@ -296,9 +297,20 @@ private: /// Write symbolized stack trace line by line for better grep-ability. stack_trace.toStringEveryLine([&](const std::string & s) { LOG_FATAL(log, s); }); + /// Send crash report to developers (if configured) + + #if defined(__ELF__) && !defined(__FreeBSD__) + const String & build_id_hex = DB::SymbolIndex::instance().getBuildIDHex(); + #else + String build_id_hex{}; + #endif + + SentryWriter::onFault(sig, info, context, stack_trace, build_id_hex); + /// When everything is done, we will try to send these error messages to client. if (thread_ptr) thread_ptr->onFatalError(); + } }; @@ -330,7 +342,7 @@ static void sanitizerDeathCallback() std::stringstream bare_stacktrace; bare_stacktrace << "Stack trace:"; for (size_t i = stack_trace.getOffset(); i < stack_trace.getSize(); ++i) - bare_stacktrace << ' ' << stack_trace.getFrames()[i]; + bare_stacktrace << ' ' << stack_trace.getFramePointers()[i]; LOG_FATAL(log, bare_stacktrace.str()); } @@ -529,6 +541,7 @@ void debugIncreaseOOMScore() {} void BaseDaemon::initialize(Application & self) { closeFDs(); + task_manager = std::make_unique(); ServerApplication::initialize(self); @@ -536,7 +549,6 @@ void BaseDaemon::initialize(Application & self) argsToConfig(argv(), config(), PRIO_APPLICATION - 100); bool is_daemon = config().getBool("application.runAsDaemon", false); - if (is_daemon) { /** When creating pid file and looking for config, will search for paths relative to the working path of the program when started. @@ -672,6 +684,7 @@ void BaseDaemon::initialize(Application & self) void BaseDaemon::initializeTerminationAndSignalProcessing() { + SentryWriter::initialize(config()); std::set_terminate(terminate_handler); /// We want to avoid SIGPIPE when working with sockets and pipes, and just handle return value/errno instead. diff --git a/base/daemon/CMakeLists.txt b/base/daemon/CMakeLists.txt index 5d9a37dc75e..04d2f059b39 100644 --- a/base/daemon/CMakeLists.txt +++ b/base/daemon/CMakeLists.txt @@ -1,7 +1,13 @@ add_library (daemon BaseDaemon.cpp GraphiteWriter.cpp + SentryWriter.cpp ) target_include_directories (daemon PUBLIC ..) target_link_libraries (daemon PUBLIC loggers PRIVATE clickhouse_common_io clickhouse_common_config common ${EXECINFO_LIBRARIES}) + +if (USE_SENTRY) + target_link_libraries (daemon PRIVATE curl) + target_link_libraries (daemon PRIVATE ${SENTRY_LIBRARY}) +endif () diff --git a/base/daemon/SentryWriter.cpp b/base/daemon/SentryWriter.cpp new file mode 100644 index 00000000000..ea93d09f9aa --- /dev/null +++ b/base/daemon/SentryWriter.cpp @@ -0,0 +1,250 @@ +#include + +#include +#include + +#include +#include +#include +#if !defined(ARCADIA_BUILD) +# include "Common/config_version.h" +# include +#endif + +#if USE_SENTRY +# include // Y_IGNORE +# include +# include +#endif + + +#if USE_SENTRY +namespace +{ + +bool initialized = false; +bool anonymize = false; + +void setExtras() +{ + + if (!anonymize) + { + sentry_set_extra("server_name", sentry_value_new_string(getFQDNOrHostName().c_str())); + } + sentry_set_tag("version", VERSION_STRING); + sentry_set_extra("version_githash", sentry_value_new_string(VERSION_GITHASH)); + sentry_set_extra("version_describe", sentry_value_new_string(VERSION_DESCRIBE)); + sentry_set_extra("version_integer", sentry_value_new_int32(VERSION_INTEGER)); + sentry_set_extra("version_revision", sentry_value_new_int32(VERSION_REVISION)); + sentry_set_extra("version_major", sentry_value_new_int32(VERSION_MAJOR)); + sentry_set_extra("version_minor", sentry_value_new_int32(VERSION_MINOR)); + sentry_set_extra("version_patch", sentry_value_new_int32(VERSION_PATCH)); +} + +void sentry_logger(sentry_level_t level, const char * message, va_list args) +{ + auto * logger = &Poco::Logger::get("SentryWriter"); + size_t size = 1024; + char buffer[size]; +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wformat-nonliteral" +#endif + if (vsnprintf(buffer, size, message, args) >= 0) + { +#ifdef __clang__ +#pragma clang diagnostic pop +#endif + switch (level) + { + case SENTRY_LEVEL_DEBUG: + logger->debug(buffer); + break; + case SENTRY_LEVEL_INFO: + logger->information(buffer); + break; + case SENTRY_LEVEL_WARNING: + logger->warning(buffer); + break; + case SENTRY_LEVEL_ERROR: + logger->error(buffer); + break; + case SENTRY_LEVEL_FATAL: + logger->fatal(buffer); + break; + } + } +} +} +#endif + +void SentryWriter::initialize(Poco::Util::LayeredConfiguration & config) +{ +#if USE_SENTRY + bool enabled = false; + bool debug = config.getBool("send_crash_reports.debug", false); + auto * logger = &Poco::Logger::get("SentryWriter"); + if (config.getBool("send_crash_reports.enabled", false)) + { + if (debug || (strlen(VERSION_OFFICIAL) > 0)) + { + enabled = true; + } + } + if (enabled) + { + const std::filesystem::path & default_tmp_path = std::filesystem::path(config.getString("tmp_path", Poco::Path::temp())) / "sentry"; + const std::string & endpoint + = config.getString("send_crash_reports.endpoint"); + const std::string & temp_folder_path + = config.getString("send_crash_reports.tmp_path", default_tmp_path); + Poco::File(temp_folder_path).createDirectories(); + + sentry_options_t * options = sentry_options_new(); /// will be freed by sentry_init or sentry_shutdown + sentry_options_set_release(options, VERSION_STRING_SHORT); + sentry_options_set_logger(options, &sentry_logger); + if (debug) + { + sentry_options_set_debug(options, 1); + } + sentry_options_set_dsn(options, endpoint.c_str()); + sentry_options_set_database_path(options, temp_folder_path.c_str()); + if (strstr(VERSION_DESCRIBE, "-stable") || strstr(VERSION_DESCRIBE, "-lts")) + { + sentry_options_set_environment(options, "prod"); + } + else + { + sentry_options_set_environment(options, "test"); + } + + const std::string & http_proxy = config.getString("send_crash_reports.http_proxy", ""); + if (!http_proxy.empty()) + { + sentry_options_set_http_proxy(options, http_proxy.c_str()); + } + + int init_status = sentry_init(options); + if (!init_status) + { + initialized = true; + anonymize = config.getBool("send_crash_reports.anonymize", false); + LOG_INFO( + logger, + "Sending crash reports is initialized with {} endpoint and {} temp folder{}", + endpoint, + temp_folder_path, + anonymize ? " (anonymized)" : ""); + } + else + { + LOG_WARNING(logger, "Sending crash reports failed to initialize with {} status", init_status); + } + } + else + { + LOG_INFO(logger, "Sending crash reports is disabled"); + } +#else + UNUSED(config); +#endif +} + +void SentryWriter::shutdown() +{ +#if USE_SENTRY + if (initialized) + { + sentry_shutdown(); + } +#endif +} + +void SentryWriter::onFault(int sig, const siginfo_t & info, const ucontext_t & context, const StackTrace & stack_trace, const String & build_id_hex) +{ +#if USE_SENTRY + auto * logger = &Poco::Logger::get("SentryWriter"); + if (initialized) + { + const std::string & error_message = signalToErrorMessage(sig, info, context); + sentry_value_t event = sentry_value_new_message_event(SENTRY_LEVEL_FATAL, "fault", error_message.c_str()); + sentry_set_tag("signal", strsignal(sig)); + sentry_set_extra("signal_number", sentry_value_new_int32(sig)); + if (!build_id_hex.empty()) + { + sentry_set_tag("build_id", build_id_hex.c_str()); + } + setExtras(); + + /// Prepare data for https://develop.sentry.dev/sdk/event-payloads/stacktrace/ + sentry_value_t sentry_frames = sentry_value_new_list(); + size_t stack_size = stack_trace.getSize(); + if (stack_size > 0) + { + ssize_t offset = stack_trace.getOffset(); + char instruction_addr[100]; + StackTrace::Frames frames; + StackTrace::symbolize(stack_trace.getFramePointers(), offset, stack_size, frames); + for (ssize_t i = stack_size - 1; i >= offset; --i) + { + const StackTrace::Frame & current_frame = frames[i]; + sentry_value_t sentry_frame = sentry_value_new_object(); + UInt64 frame_ptr = reinterpret_cast(current_frame.virtual_addr); + + if (std::snprintf(instruction_addr, sizeof(instruction_addr), "0x%" PRIx64, frame_ptr) >= 0) + { + sentry_value_set_by_key(sentry_frame, "instruction_addr", sentry_value_new_string(instruction_addr)); + } + + if (current_frame.symbol.has_value()) + { + sentry_value_set_by_key(sentry_frame, "function", sentry_value_new_string(current_frame.symbol.value().c_str())); + } + + if (current_frame.file.has_value()) + { + sentry_value_set_by_key(sentry_frame, "filename", sentry_value_new_string(current_frame.file.value().c_str())); + } + + if (current_frame.line.has_value()) + { + sentry_value_set_by_key(sentry_frame, "lineno", sentry_value_new_int32(current_frame.line.value())); + } + + sentry_value_append(sentry_frames, sentry_frame); + } + } + + /// Prepare data for https://develop.sentry.dev/sdk/event-payloads/threads/ + /// Stacktrace is filled only for a single thread that failed + sentry_value_t stacktrace = sentry_value_new_object(); + sentry_value_set_by_key(stacktrace, "frames", sentry_frames); + + sentry_value_t thread = sentry_value_new_object(); + sentry_value_set_by_key(thread, "stacktrace", stacktrace); + + sentry_value_t values = sentry_value_new_list(); + sentry_value_append(values, thread); + + sentry_value_t threads = sentry_value_new_object(); + sentry_value_set_by_key(threads, "values", values); + + sentry_value_set_by_key(event, "threads", threads); + + LOG_INFO(logger, "Sending crash report"); + sentry_capture_event(event); + shutdown(); + } + else + { + LOG_INFO(logger, "Not sending crash report"); + } +#else + UNUSED(sig); + UNUSED(info); + UNUSED(context); + UNUSED(stack_trace); + UNUSED(build_id_hex); +#endif +} diff --git a/base/daemon/SentryWriter.h b/base/daemon/SentryWriter.h new file mode 100644 index 00000000000..a7b255e72bf --- /dev/null +++ b/base/daemon/SentryWriter.h @@ -0,0 +1,33 @@ +#pragma once + +#include +#include + +#include + +#include + +/// \brief Sends crash reports to ClickHouse core developer team via https://sentry.io +/// +/// This feature can enabled with "send_crash_reports.enabled" server setting, +/// in this case reports are sent only for official ClickHouse builds. +/// +/// It is possible to send those reports to your own sentry account or account of consulting company you hired +/// by overriding "send_crash_reports.endpoint" setting. "send_crash_reports.debug" setting will allow to do that for +class SentryWriter +{ +public: + SentryWriter() = delete; + + static void initialize(Poco::Util::LayeredConfiguration & config); + static void shutdown(); + + /// Not signal safe and can't be called from a signal handler + static void onFault( + int sig, + const siginfo_t & info, + const ucontext_t & context, + const StackTrace & stack_trace, + const String & build_id_hex + ); +}; diff --git a/base/daemon/ya.make b/base/daemon/ya.make index 1c72af3ed53..125417adca5 100644 --- a/base/daemon/ya.make +++ b/base/daemon/ya.make @@ -9,6 +9,7 @@ PEERDIR( SRCS( BaseDaemon.cpp GraphiteWriter.cpp + SentryWriter.cpp ) END() diff --git a/cmake/find/sentry.cmake b/cmake/find/sentry.cmake new file mode 100644 index 00000000000..eadf071141e --- /dev/null +++ b/cmake/find/sentry.cmake @@ -0,0 +1,21 @@ +set (SENTRY_LIBRARY "sentry") +set (SENTRY_INCLUDE_DIR "${ClickHouse_SOURCE_DIR}/contrib/sentry-native/include") +if (NOT EXISTS "${SENTRY_INCLUDE_DIR}/sentry.h") + message (WARNING "submodule contrib/sentry-native is missing. to fix try run: \n git submodule update --init --recursive") + return() +endif () + +if (NOT OS_FREEBSD AND NOT SPLIT_SHARED_LIBRARIES AND NOT_UNBUNDLED AND NOT (OS_DARWIN AND COMPILER_CLANG)) + option (USE_SENTRY "Use Sentry" ON) + set (CURL_LIBRARY ${ClickHouse_SOURCE_DIR}/contrib/curl/lib) + set (CURL_INCLUDE_DIR ${ClickHouse_SOURCE_DIR}/contrib/curl/include) + set (SENTRY_TRANSPORT "curl" CACHE STRING "") + set (SENTRY_BACKEND "none" CACHE STRING "") + set (SENTRY_EXPORT_SYMBOLS OFF CACHE BOOL "") + set (SENTRY_LINK_PTHREAD OFF CACHE BOOL "") + set (SENTRY_PIC OFF CACHE BOOL "") + set (BUILD_SHARED_LIBS OFF) + message (STATUS "Using sentry=${USE_SENTRY}: ${SENTRY_LIBRARY}") + + include_directories("${SENTRY_INCLUDE_DIR}") +endif () diff --git a/cmake/version.cmake b/cmake/version.cmake index eea17f68c47..963f291c0f3 100644 --- a/cmake/version.cmake +++ b/cmake/version.cmake @@ -14,6 +14,7 @@ endif () set (VERSION_NAME "${PROJECT_NAME}") set (VERSION_FULL "${VERSION_NAME} ${VERSION_STRING}") set (VERSION_SO "${VERSION_STRING}") +set (VERSION_STRING_SHORT "${VERSION_MAJOR}.${VERSION_MINOR}") math (EXPR VERSION_INTEGER "${VERSION_PATCH} + ${VERSION_MINOR}*1000 + ${VERSION_MAJOR}*1000000") diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index b8029124712..f2222797bff 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -263,7 +263,7 @@ if (USE_INTERNAL_GRPC_LIBRARY) add_subdirectory(grpc-cmake) endif () -if (USE_INTERNAL_AWS_S3_LIBRARY) +if (USE_INTERNAL_AWS_S3_LIBRARY OR USE_SENTRY) set (save_CMAKE_C_FLAGS ${CMAKE_C_FLAGS}) set (save_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES}) set (save_CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES}) @@ -275,12 +275,18 @@ if (USE_INTERNAL_AWS_S3_LIBRARY) set (CMAKE_CMAKE_REQUIRED_INCLUDES ${save_CMAKE_REQUIRED_INCLUDES}) set (CMAKE_REQUIRED_FLAGS ${save_CMAKE_REQUIRED_FLAGS}) set (CMAKE_CMAKE_MODULE_PATH ${save_CMAKE_MODULE_PATH}) + + # The library is large - avoid bloat. + target_compile_options (curl PRIVATE -g0) +endif () + +if (USE_INTERNAL_AWS_S3_LIBRARY) add_subdirectory(aws-s3-cmake) # The library is large - avoid bloat. target_compile_options (aws_s3 PRIVATE -g0) target_compile_options (aws_s3_checksums PRIVATE -g0) - target_compile_options (curl PRIVATE -g0) + endif () if (USE_BASE64) @@ -300,5 +306,9 @@ if (USE_CASSANDRA) add_subdirectory (cassandra) endif() +if (USE_SENTRY) + add_subdirectory (sentry-native) +endif() + add_subdirectory (fmtlib-cmake) diff --git a/contrib/curl-cmake/CMakeLists.txt b/contrib/curl-cmake/CMakeLists.txt index d9805612ffe..d0f6a7773b0 100644 --- a/contrib/curl-cmake/CMakeLists.txt +++ b/contrib/curl-cmake/CMakeLists.txt @@ -1,4 +1,6 @@ set (CURL_DIR ${ClickHouse_SOURCE_DIR}/contrib/curl) +set (CURL_LIBRARY ${ClickHouse_SOURCE_DIR}/contrib/curl/lib) +set (CURL_INCLUDE_DIR ${ClickHouse_SOURCE_DIR}/contrib/curl/include) set (SRCS ${CURL_DIR}/lib/file.c diff --git a/contrib/sentry-native b/contrib/sentry-native new file mode 160000 index 00000000000..f91ed3f95b5 --- /dev/null +++ b/contrib/sentry-native @@ -0,0 +1 @@ +Subproject commit f91ed3f95b5653f247189d720ab00765b4899d6f diff --git a/docs/en/operations/server-configuration-parameters/settings.md b/docs/en/operations/server-configuration-parameters/settings.md index 5759357ca81..7641c4cef96 100644 --- a/docs/en/operations/server-configuration-parameters/settings.md +++ b/docs/en/operations/server-configuration-parameters/settings.md @@ -307,11 +307,11 @@ Logging settings. Keys: -- level – Logging level. Acceptable values: `trace`, `debug`, `information`, `warning`, `error`. -- log – The log file. Contains all the entries according to `level`. -- errorlog – Error log file. -- size – Size of the file. Applies to `log`and`errorlog`. Once the file reaches `size`, ClickHouse archives and renames it, and creates a new log file in its place. -- count – The number of archived log files that ClickHouse stores. +- `level` – Logging level. Acceptable values: `trace`, `debug`, `information`, `warning`, `error`. +- `log` – The log file. Contains all the entries according to `level`. +- `errorlog` – Error log file. +- `size` – Size of the file. Applies to `log`and`errorlog`. Once the file reaches `size`, ClickHouse archives and renames it, and creates a new log file in its place. +- `count` – The number of archived log files that ClickHouse stores. **Example** @@ -348,6 +348,30 @@ Keys: Default value: `LOG_USER` if `address` is specified, `LOG_DAEMON otherwise.` - format – Message format. Possible values: `bsd` and `syslog.` +## send_crash_reports {#server_configuration_parameters-logger} + +Settings for opt-in sending crash reports to the ClickHouse core developers team via [Sentry](https://sentry.io). +Enabling it, especially in pre-production environments, is greatly appreciated. + +The server will need an access to public Internet via IPv4 (at the time of writing IPv6 is not supported by Sentry) for this feature to be functioning properly. + +Keys: + +- `enabled` – Boolean flag to enable the feature. Set to `true` to allow sending crash reports. +- `endpoint` – Overrides the Sentry endpoint. +- `anonymize` - Avoid attaching the server hostname to crash report. +- `http_proxy` - Configure HTTP proxy for sending crash reports. +- `debug` - Sets the Sentry client into debug mode. +- `tmp_path` - Filesystem path for temporary crash report state. + +**Recommended way to use** + +``` xml + + true + +``` + ## macros {#macros} Parameter substitutions for replicated tables. diff --git a/programs/odbc-bridge/CMakeLists.txt b/programs/odbc-bridge/CMakeLists.txt index 628f9ee018a..4b63ed2596d 100644 --- a/programs/odbc-bridge/CMakeLists.txt +++ b/programs/odbc-bridge/CMakeLists.txt @@ -10,7 +10,6 @@ set (CLICKHOUSE_ODBC_BRIDGE_SOURCES PingHandler.cpp validateODBCConnectionString.cpp ) - set (CLICKHOUSE_ODBC_BRIDGE_LINK PRIVATE clickhouse_parsers diff --git a/programs/server/config.xml b/programs/server/config.xml index 81b77f9225b..ca2e6072cd2 100644 --- a/programs/server/config.xml +++ b/programs/server/config.xml @@ -45,6 +45,18 @@ --> + + + + + false + + false + + + https://6f33034cfe684dd7a3ab9875e57b1c8d@o388870.ingest.sentry.io/5226277 + + 8123 9000 diff --git a/src/Common/StackTrace.cpp b/src/Common/StackTrace.cpp index 6d0b6a0f7d2..9c6904b884c 100644 --- a/src/Common/StackTrace.cpp +++ b/src/Common/StackTrace.cpp @@ -1,12 +1,12 @@ #include +#include #include #include #include #include #include #include -#include #include #include @@ -176,13 +176,13 @@ static void * getCallerAddress(const ucontext_t & context) { #if defined(__x86_64__) /// Get the address at the time the signal was raised from the RIP (x86-64) -#if defined(__FreeBSD__) +# if defined(__FreeBSD__) return reinterpret_cast(context.uc_mcontext.mc_rip); -#elif defined(__APPLE__) +# elif defined(__APPLE__) return reinterpret_cast(context.uc_mcontext->__ss.__rip); -#else +# else return reinterpret_cast(context.uc_mcontext.gregs[REG_RIP]); -#endif +# endif #elif defined(__aarch64__) return reinterpret_cast(context.uc_mcontext.pc); #else @@ -190,6 +190,66 @@ static void * getCallerAddress(const ucontext_t & context) #endif } +void StackTrace::symbolize(const StackTrace::FramePointers & frame_pointers, size_t offset, size_t size, StackTrace::Frames & frames) +{ +#if defined(__ELF__) && !defined(__FreeBSD__) && !defined(ARCADIA_BUILD) + + const DB::SymbolIndex & symbol_index = DB::SymbolIndex::instance(); + std::unordered_map dwarfs; + + for (size_t i = 0; i < offset; ++i) + { + frames[i].virtual_addr = frame_pointers[i]; + } + + for (size_t i = offset; i < size; ++i) + { + StackTrace::Frame & current_frame = frames[i]; + current_frame.virtual_addr = frame_pointers[i]; + const auto * object = symbol_index.findObject(current_frame.virtual_addr); + uintptr_t virtual_offset = object ? uintptr_t(object->address_begin) : 0; + current_frame.physical_addr = reinterpret_cast(uintptr_t(current_frame.virtual_addr) - virtual_offset); + + if (object) + { + current_frame.object = object->name; + if (std::filesystem::exists(current_frame.object.value())) + { + auto dwarf_it = dwarfs.try_emplace(object->name, *object->elf).first; + + DB::Dwarf::LocationInfo location; + if (dwarf_it->second.findAddress(uintptr_t(current_frame.physical_addr), location, DB::Dwarf::LocationInfoMode::FAST)) + { + current_frame.file = location.file.toString(); + current_frame.line = location.line; + } + } + } + else + { + current_frame.object = "?"; + } + + const auto * symbol = symbol_index.findSymbol(current_frame.virtual_addr); + if (symbol) + { + int status = 0; + current_frame.symbol = demangle(symbol->name, status); + } + else + { + current_frame.symbol = "?"; + } + } +#else + for (size_t i = 0; i < size; ++i) + { + frames[i].virtual_addr = frame_pointers[i]; + } + UNUSED(offset); +#endif +} + StackTrace::StackTrace() { tryCapture(); @@ -203,16 +263,15 @@ StackTrace::StackTrace(const ucontext_t & signal_context) if (size == 0 && caller_address) { - frames[0] = caller_address; + frame_pointers[0] = caller_address; size = 1; } else { /// Skip excessive stack frames that we have created while finding stack trace. - for (size_t i = 0; i < size; ++i) { - if (frames[i] == caller_address) + if (frame_pointers[i] == caller_address) { offset = i; break; @@ -229,8 +288,8 @@ void StackTrace::tryCapture() { size = 0; #if USE_UNWIND - size = unw_backtrace(frames.data(), capacity); - __msan_unpoison(frames.data(), size * sizeof(frames[0])); + size = unw_backtrace(frame_pointers.data(), capacity); + __msan_unpoison(frame_pointers.data(), size * sizeof(frame_pointers[0])); #endif } @@ -244,13 +303,12 @@ size_t StackTrace::getOffset() const return offset; } -const StackTrace::Frames & StackTrace::getFrames() const +const StackTrace::FramePointers & StackTrace::getFramePointers() const { - return frames; + return frame_pointers; } - -static void toStringEveryLineImpl(const StackTrace::Frames & frames, size_t offset, size_t size, std::function callback) +static void toStringEveryLineImpl(const StackTrace::FramePointers & frame_pointers, size_t offset, size_t size, std::function callback) { if (size == 0) return callback(""); @@ -263,7 +321,7 @@ static void toStringEveryLineImpl(const StackTrace::Frames & frames, size_t offs for (size_t i = offset; i < size; ++i) { - const void * virtual_addr = frames[i]; + const void * virtual_addr = frame_pointers[i]; const auto * object = symbol_index.findObject(virtual_addr); uintptr_t virtual_offset = object ? uintptr_t(object->address_begin) : 0; const void * physical_addr = reinterpret_cast(uintptr_t(virtual_addr) - virtual_offset); @@ -302,7 +360,7 @@ static void toStringEveryLineImpl(const StackTrace::Frames & frames, size_t offs for (size_t i = offset; i < size; ++i) { - const void * addr = frames[i]; + const void * addr = frame_pointers[i]; out << i << ". " << addr; callback(out.str()); @@ -311,35 +369,36 @@ static void toStringEveryLineImpl(const StackTrace::Frames & frames, size_t offs #endif } -static std::string toStringImpl(const StackTrace::Frames & frames, size_t offset, size_t size) +static std::string toStringImpl(const StackTrace::FramePointers & frame_pointers, size_t offset, size_t size) { std::stringstream out; - toStringEveryLineImpl(frames, offset, size, [&](const std::string & str) { out << str << '\n'; }); + toStringEveryLineImpl(frame_pointers, offset, size, [&](const std::string & str) { out << str << '\n'; }); return out.str(); } void StackTrace::toStringEveryLine(std::function callback) const { - toStringEveryLineImpl(frames, offset, size, std::move(callback)); + toStringEveryLineImpl(frame_pointers, offset, size, std::move(callback)); } + std::string StackTrace::toString() const { /// Calculation of stack trace text is extremely slow. /// We use simple cache because otherwise the server could be overloaded by trash queries. static SimpleCache func_cached; - return func_cached(frames, offset, size); + return func_cached(frame_pointers, offset, size); } -std::string StackTrace::toString(void ** frames_, size_t offset, size_t size) +std::string StackTrace::toString(void ** frame_pointers_, size_t offset, size_t size) { - __msan_unpoison(frames_, size * sizeof(*frames_)); + __msan_unpoison(frame_pointers_, size * sizeof(*frame_pointers_)); - StackTrace::Frames frames_copy{}; + StackTrace::FramePointers frame_pointers_copy{}; for (size_t i = 0; i < size; ++i) - frames_copy[i] = frames_[i]; + frame_pointers_copy[i] = frame_pointers_[i]; static SimpleCache func_cached; - return func_cached(frames_copy, offset, size); + return func_cached(frame_pointers_copy, offset, size); } diff --git a/src/Common/StackTrace.h b/src/Common/StackTrace.h index 401c8344f2d..3ae4b964838 100644 --- a/src/Common/StackTrace.h +++ b/src/Common/StackTrace.h @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -23,8 +25,18 @@ struct NoCapture class StackTrace { public: + struct Frame + { + const void * virtual_addr = nullptr; + void * physical_addr = nullptr; + std::optional symbol; + std::optional object; + std::optional file; + std::optional line; + }; static constexpr size_t capacity = 32; - using Frames = std::array; + using FramePointers = std::array; + using Frames = std::array; /// Tries to capture stack trace StackTrace(); @@ -38,19 +50,19 @@ public: size_t getSize() const; size_t getOffset() const; - const Frames & getFrames() const; + const FramePointers & getFramePointers() const; std::string toString() const; - static std::string toString(void ** frames, size_t offset, size_t size); + static std::string toString(void ** frame_pointers, size_t offset, size_t size); + static void symbolize(const FramePointers & frame_pointers, size_t offset, size_t size, StackTrace::Frames & frames); void toStringEveryLine(std::function callback) const; - protected: void tryCapture(); size_t size = 0; size_t offset = 0; /// How many frames to skip while displaying. - Frames frames{}; + FramePointers frame_pointers{}; }; std::string signalToErrorMessage(int sig, const siginfo_t & info, const ucontext_t & context); diff --git a/src/Common/TraceCollector.cpp b/src/Common/TraceCollector.cpp index 7df06dc7892..f5bdfd2b826 100644 --- a/src/Common/TraceCollector.cpp +++ b/src/Common/TraceCollector.cpp @@ -81,7 +81,7 @@ void TraceCollector::collect(TraceType trace_type, const StackTrace & stack_trac size_t stack_trace_offset = stack_trace.getOffset(); writeIntBinary(UInt8(stack_trace_size - stack_trace_offset), out); for (size_t i = stack_trace_offset; i < stack_trace_size; ++i) - writePODBinary(stack_trace.getFrames()[i], out); + writePODBinary(stack_trace.getFramePointers()[i], out); writePODBinary(trace_type, out); writePODBinary(thread_id, out); diff --git a/src/Common/config.h.in b/src/Common/config.h.in index e6bc46256e0..ff4e5e8c6b3 100644 --- a/src/Common/config.h.in +++ b/src/Common/config.h.in @@ -10,5 +10,6 @@ #cmakedefine01 USE_UNWIND #cmakedefine01 USE_OPENCL #cmakedefine01 USE_CASSANDRA +#cmakedefine01 USE_SENTRY #cmakedefine01 USE_GRPC #cmakedefine01 CLICKHOUSE_SPLIT_BINARY diff --git a/src/Common/config_version.h.in b/src/Common/config_version.h.in index bc90e63e39c..c3c0c6df87b 100644 --- a/src/Common/config_version.h.in +++ b/src/Common/config_version.h.in @@ -20,6 +20,7 @@ #cmakedefine VERSION_MINOR @VERSION_MINOR@ #cmakedefine VERSION_PATCH @VERSION_PATCH@ #cmakedefine VERSION_STRING "@VERSION_STRING@" +#cmakedefine VERSION_STRING_SHORT "@VERSION_STRING_SHORT@" #cmakedefine VERSION_OFFICIAL "@VERSION_OFFICIAL@" #cmakedefine VERSION_FULL "@VERSION_FULL@" #cmakedefine VERSION_DESCRIBE "@VERSION_DESCRIBE@" diff --git a/src/Storages/System/StorageSystemStackTrace.cpp b/src/Storages/System/StorageSystemStackTrace.cpp index a8966ad0307..bdce70894d5 100644 --- a/src/Storages/System/StorageSystemStackTrace.cpp +++ b/src/Storages/System/StorageSystemStackTrace.cpp @@ -198,7 +198,7 @@ void StorageSystemStackTrace::fillData(MutableColumns & res_columns, const Conte Array arr; arr.reserve(stack_trace_size - stack_trace_offset); for (size_t i = stack_trace_offset; i < stack_trace_size; ++i) - arr.emplace_back(reinterpret_cast(stack_trace->getFrames()[i])); + arr.emplace_back(reinterpret_cast(stack_trace->getFramePointers()[i])); res_columns[0]->insert(tid); res_columns[1]->insertData(query_id_data, query_id_size); diff --git a/tests/integration/test_send_crash_reports/__init__.py b/tests/integration/test_send_crash_reports/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_send_crash_reports/configs/config_send_crash_reports.xml b/tests/integration/test_send_crash_reports/configs/config_send_crash_reports.xml new file mode 100644 index 00000000000..10f559b0054 --- /dev/null +++ b/tests/integration/test_send_crash_reports/configs/config_send_crash_reports.xml @@ -0,0 +1,8 @@ + + + + true + true + http://6f33034cfe684dd7a3ab9875e57b1c8d@localhost:9500/5226277 + + diff --git a/tests/integration/test_send_crash_reports/fake_sentry_server.py b/tests/integration/test_send_crash_reports/fake_sentry_server.py new file mode 100644 index 00000000000..74f0592504f --- /dev/null +++ b/tests/integration/test_send_crash_reports/fake_sentry_server.py @@ -0,0 +1,43 @@ +import BaseHTTPServer + +RESULT_PATH = '/result.txt' + +class SentryHandler(BaseHTTPServer.BaseHTTPRequestHandler): + def do_POST(self): + post_data = self.__read_and_decode_post_data() + with open(RESULT_PATH, 'w') as f: + if self.headers.get("content-type") != "application/x-sentry-envelope": + f.write("INCORRECT_CONTENT_TYPE") + elif self.headers.get("content-length") < 3000: + f.write("INCORRECT_CONTENT_LENGTH") + elif '"http://6f33034cfe684dd7a3ab9875e57b1c8d@localhost:9500/5226277"' not in post_data: + f.write('INCORRECT_POST_DATA') + else: + f.write("OK") + self.send_response(200) + + def __read_and_decode_post_data(self): + transfer_encoding = self.headers.get("transfer-Encoding") + decoded = "" + if transfer_encoding == "chunked": + while True: + s = self.rfile.readline() + chunk_length = int(s, 16) + if not chunk_length: + break + decoded += self.rfile.read(chunk_length) + self.rfile.readline() + else: + content_length = int(self.headers.get("content-length", 0)) + decoded = self.rfile.read(content_length) + return decoded + + +if __name__ == "__main__": + with open(RESULT_PATH, 'w') as f: + f.write("INITIAL_STATE") + httpd = BaseHTTPServer.HTTPServer(("localhost", 9500,), SentryHandler) + try: + httpd.serve_forever() + finally: + httpd.server_close() diff --git a/tests/integration/test_send_crash_reports/test.py b/tests/integration/test_send_crash_reports/test.py new file mode 100644 index 00000000000..ff4b55da99b --- /dev/null +++ b/tests/integration/test_send_crash_reports/test.py @@ -0,0 +1,44 @@ +import os +import time + +import pytest + +import helpers.cluster +import helpers.test_tools +import fake_sentry_server + + +SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) + + +@pytest.fixture(scope="module") +def started_node(): + cluster = helpers.cluster.ClickHouseCluster(__file__) + try: + node = cluster.add_instance("node", main_configs=[ + os.path.join(SCRIPT_DIR, "configs", "config_send_crash_reports.xml") + ]) + cluster.start() + yield node + finally: + cluster.shutdown() + + +def test_send_segfault(started_node,): + started_node.copy_file_to_container(os.path.join(SCRIPT_DIR, "fake_sentry_server.py"), "/fake_sentry_server.py") + started_node.exec_in_container(["bash", "-c", "python2 /fake_sentry_server.py"], detach=True, user="root") + time.sleep(0.5) + started_node.exec_in_container(["bash", "-c", "pkill -11 clickhouse"], user="root") + + result = None + for attempt in range(1, 6): + time.sleep(0.25 * attempt) + result = started_node.exec_in_container(['cat', fake_sentry_server.RESULT_PATH], user='root') + if result == 'OK': + break + elif result == 'INITIAL_STATE': + continue + elif result: + assert False, 'Unexpected state: ' + result + + assert result == 'OK', 'Crash report not sent' diff --git a/tests/queries/0_stateless/00600_replace_running_query.sh b/tests/queries/0_stateless/00600_replace_running_query.sh index 1331dd3c15b..75006cc56ce 100755 --- a/tests/queries/0_stateless/00600_replace_running_query.sh +++ b/tests/queries/0_stateless/00600_replace_running_query.sh @@ -36,5 +36,5 @@ wait ${CLICKHOUSE_CLIENT} --query_id=42 --query='SELECT 3, count() FROM system.numbers' 2>&1 | grep -cF 'was cancelled' & wait_for_query_to_start '42' ${CLICKHOUSE_CLIENT} --query_id=42 --replace_running_query=1 --replace_running_query_max_wait_ms=500 --query='SELECT 43' 2>&1 | grep -F "can't be stopped" > /dev/null -${CLICKHOUSE_CLIENT} --query_id=42 --replace_running_query=1 --query='SELECT 44' wait +${CLICKHOUSE_CLIENT} --query_id=42 --replace_running_query=1 --query='SELECT 44' diff --git a/utils/check-style/check-include b/utils/check-style/check-include index 211172979bd..35f94d6e706 100755 --- a/utils/check-style/check-include +++ b/utils/check-style/check-include @@ -59,6 +59,7 @@ inc="-I. \ -I./contrib/lz4/lib \ -I./contrib/hyperscan/src \ -I./contrib/simdjson/include \ +-I./contrib/sentry-native/include \ -I./src \ -I${BUILD_DIR}/src"