improvements after review comments

This commit is contained in:
Ivan Blinkov 2020-06-16 15:56:28 +03:00
parent c30457a3ed
commit 0e77692a27
4 changed files with 29 additions and 18 deletions

View File

@ -14,6 +14,7 @@
#if USE_SENTRY
# include <sentry.h> // Y_IGNORE
# include <stdio.h>
# include <filesystem>
#endif
@ -31,7 +32,7 @@ void setExtras()
{
sentry_set_extra("server_name", sentry_value_new_string(getFQDNOrHostName().c_str()));
}
sentry_set_tag("version", VERSION_STRING_SHORT);
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));
@ -93,14 +94,15 @@ void SentryWriter::initialize(Poco::Util::LayeredConfiguration & config)
}
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", "https://6f33034cfe684dd7a3ab9875e57b1c8d@o388870.ingest.sentry.io/5226277");
const std::string & temp_folder_path
= config.getString("send_crash_reports.tmp_path", config.getString("tmp_path", Poco::Path::temp()) + "sentry/");
= config.getString("send_crash_reports.tmp_path", default_tmp_path);
Poco::File(temp_folder_path).createDirectories();
sentry_options_t * options = sentry_options_new();
sentry_options_set_release(options, VERSION_STRING);
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)
{
@ -128,17 +130,16 @@ void SentryWriter::initialize(Poco::Util::LayeredConfiguration & config)
{
initialized = true;
anonymize = config.getBool("send_crash_reports.anonymize", false);
const std::string& anonymize_status = anonymize ? " (anonymized)" : "";
LOG_INFO(
logger,
"Sending crash reports is initialized with {} endpoint and {} temp folder{}",
endpoint,
temp_folder_path,
anonymize_status);
anonymize ? " (anonymized)" : "");
}
else
{
LOG_WARNING(logger, "Sending crash reports failed to initialized with {} status", init_status);
LOG_WARNING(logger, "Sending crash reports failed to initialize with {} status", init_status);
}
}
else
@ -177,21 +178,20 @@ void SentryWriter::onFault(int sig, const siginfo_t & info, const ucontext_t & c
size_t stack_size = stack_trace.getSize();
if (stack_size > 0)
{
size_t offset = stack_trace.getOffset();
if (stack_size == 1)
{
offset = 1;
}
ssize_t offset = stack_trace.getOffset();
char instruction_addr[100];
StackTrace::Frames frames;
StackTrace::symbolize(stack_trace.getFramePointers().data(), offset, stack_size, frames);
for (size_t i = stack_size - 1; i >= offset; --i)
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<UInt64>(current_frame.virtual_addr);
std::snprintf(instruction_addr, sizeof(instruction_addr), "0x%" PRIx64, frame_ptr);
sentry_value_set_by_key(sentry_frame, "instruction_addr", sentry_value_new_string(instruction_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())
{
@ -213,6 +213,7 @@ void SentryWriter::onFault(int sig, const siginfo_t & info, const ucontext_t & c
}
/// 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);
@ -225,6 +226,7 @@ void SentryWriter::onFault(int sig, const siginfo_t & info, const ucontext_t & c
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");

View File

@ -7,7 +7,13 @@
#include <string>
/// Sends crash reports to ClickHouse core developer team via https://sentry.io
/// \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:
@ -15,6 +21,8 @@ public:
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,

View File

@ -353,7 +353,7 @@ Keys:
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 for this feature to be functioning properly.
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:

View File

@ -46,7 +46,8 @@
</logger>
<send_crash_reports>
<!-- Changing <enabled> to true allows sending crash reports to the ClickHouse core developers team -->
<!-- Changing <enabled> to true allows sending crash reports to -->
<!-- the ClickHouse core developers team via Sentry https://sentry.io -->
<!-- Doing so at least in pre-production environments is highly appreciated -->
<enabled>false</enabled>
<!-- Change <anonymize> to true if you don't feel comfortable attaching the server hostname to the crash report -->