From 24f305d177a2f7a4d4f89c01f187546da126f505 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 12 Jun 2025 15:25:57 +0200 Subject: [PATCH 01/27] add sentry log API + send first logs --- examples/example.c | 14 ++++++ include/sentry.h | 8 +++ src/CMakeLists.txt | 2 + src/sentry_envelope.c | 28 +++++++++++ src/sentry_envelope.h | 6 +++ src/sentry_logs.c | 114 ++++++++++++++++++++++++++++++++++++++++++ src/sentry_logs.h | 48 ++++++++++++++++++ 7 files changed, 220 insertions(+) create mode 100644 src/sentry_logs.c create mode 100644 src/sentry_logs.h diff --git a/examples/example.c b/examples/example.c index 4e1790d5d..bcb3d519e 100644 --- a/examples/example.c +++ b/examples/example.c @@ -16,6 +16,8 @@ # undef NDEBUG #endif +#include "../src/sentry_options.h" + #include #ifdef SENTRY_PLATFORM_WINDOWS @@ -359,6 +361,18 @@ main(int argc, char **argv) sentry_init(options); + // TODO incorporate into test + if (options->enable_logs) { + sentry_logger_debug( + "We log it up %i percent, %s style\n", 100, "debug"); + sentry_logger_info("We log it up %i percent, %s style\n", 100, "info"); + sentry_logger_warn("We log it up %i percent, %s style\n", 100, "warn"); + sentry_logger_error( + "We log it up %i percent, %s style\n", 100, "error"); + sentry_logger_fatal( + "We log it up %i percent, %s style\n", 100, "fatal"); + } + if (!has_arg(argc, argv, "no-setup")) { sentry_set_transaction("test-transaction"); sentry_set_level(SENTRY_LEVEL_WARNING); diff --git a/include/sentry.h b/include/sentry.h index b5ceb617a..70ec092db 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1750,6 +1750,14 @@ SENTRY_EXPERIMENTAL_API void sentry_options_set_enable_logs( SENTRY_EXPERIMENTAL_API int sentry_options_get_enable_logs( const sentry_options_t *opts); +// TODO think about API; functions or MACROs? +SENTRY_EXPERIMENTAL_API void sentry_logger_trace(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_logger_debug(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_logger_info(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_logger_warn(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_logger_error(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_logger_fatal(const char *message, ...); + #ifdef SENTRY_PLATFORM_LINUX /** diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 700f79753..1eddcf4b3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -51,6 +51,8 @@ sentry_target_sources_cwd(sentry transports/sentry_disk_transport.h transports/sentry_function_transport.c unwinder/sentry_unwinder.c + sentry_logs.c + sentry_logs.h ) # generic platform / path / symbolizer diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index 137115c86..dc023d6f5 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -295,6 +295,34 @@ sentry__envelope_add_transaction( return item; } +sentry_envelope_item_t * +sentry__envelope_add_logs(sentry_envelope_t *envelope, sentry_value_t logs) +{ + // TODO ensure this starts out correctly; we get {dsn:...} as a header + // (but don't think we need it?) + sentry_envelope_item_t *item = envelope_add_item(envelope); + if (!item) { + return NULL; + } + + sentry_jsonwriter_t *jw = sentry__jsonwriter_new_sb(NULL); + if (!jw) { + return NULL; + } + + sentry__jsonwriter_write_value(jw, logs); + item->payload = sentry__jsonwriter_into_string(jw, &item->payload_len); + + sentry__envelope_item_set_header( + item, "type", sentry_value_new_string("log")); + sentry__envelope_item_set_header(item, "item_count", + sentry_value_new_int32(sentry_value_get_length(logs))); + sentry__envelope_item_set_header(item, "content_type", + sentry_value_new_string("application/vnd.sentry.items.log+json")); + + return item; +} + sentry_envelope_item_t * sentry__envelope_add_user_feedback( sentry_envelope_t *envelope, sentry_value_t user_feedback) diff --git a/src/sentry_envelope.h b/src/sentry_envelope.h index 842cbb475..4cfc7d90f 100644 --- a/src/sentry_envelope.h +++ b/src/sentry_envelope.h @@ -42,6 +42,12 @@ sentry_envelope_item_t *sentry__envelope_add_event( sentry_envelope_item_t *sentry__envelope_add_transaction( sentry_envelope_t *envelope, sentry_value_t transaction); +/** + * Add a list of logs to this envelope. + */ +sentry_envelope_item_t *sentry__envelope_add_logs( + sentry_envelope_t *envelope, sentry_value_t logs); + /** * Add a user feedback to this envelope. */ diff --git a/src/sentry_logs.c b/src/sentry_logs.c new file mode 100644 index 000000000..da7db3e98 --- /dev/null +++ b/src/sentry_logs.c @@ -0,0 +1,114 @@ +#include "sentry_logs.h" +#include "sentry_core.h" +#include "sentry_envelope.h" +#include "sentry_options.h" + +#include +#include + +#include +#include + +char * +log_level_as_string(sentry_log_level_t level) +{ + switch (level) { + case SENTRY_LOG_LEVEL_TRACE: + return "TRACE"; + case SENTRY_LOG_LEVEL_DEBUG: + return "DEBUG"; + case SENTRY_LOG_LEVEL_INFO: + return "INFO"; + case SENTRY_LOG_LEVEL_WARN: + return "WARN"; + case SENTRY_LOG_LEVEL_ERROR: + return "ERROR"; + case SENTRY_LOG_LEVEL_FATAL: + return "FATAL"; + } +} + +void +sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) +{ + SENTRY_WITH_OPTIONS (options) { + if (!options->enable_logs) + return; + } + SENTRY_INFOF("Logging level: %i\n", level); + vprintf(message, args); + // create log from message + sentry_value_t log = sentry_value_new_object(); + sentry_value_set_by_key(log, "body", sentry_value_new_string(message)); + sentry_value_set_by_key( + log, "level", sentry_value_new_string(log_level_as_string(level))); + // TODO add actual trace_id + sentry_value_set_by_key(log, "trace_id", + sentry_value_new_string("5b8efff798038103d269b633813fc60c")); + // timestamp in seconds + sentry_value_set_by_key(log, "timestamp", + sentry_value_new_double(sentry__usec_time() / 1000000.0)); + // TODO add log attributes + // https://develop.sentry.dev/sdk/telemetry/logs/#default-attributes + + // TODO split up the code below for batched log sending + // e.g. could we store logs on the scope? + sentry_value_t logs = sentry_value_new_object(); + sentry_value_t logs_list = sentry_value_new_list(); + sentry_value_append(logs_list, log); + sentry_value_set_by_key(logs, "items", logs_list); + // sending of the envelope + sentry_envelope_t *envelope = sentry__envelope_new(); + sentry__envelope_add_logs(envelope, logs); + sentry_envelope_write_to_file(envelope, "logs_envelope.json"); + SENTRY_WITH_OPTIONS (options) { + sentry__capture_envelope(options->transport, envelope); + } +} + +// TODO think about the structure below, is this how we want the API to +// function? +void +sentry_logger_debug(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_DEBUG, message, args); + va_end(args); +} + +void +sentry_logger_info(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_INFO, message, args); + va_end(args); +} + +void +sentry_logger_warn(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_WARN, message, args); + va_end(args); +} + +void +sentry_logger_error(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_ERROR, message, args); + va_end(args); +} + +void +sentry_logger_fatal(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_FATAL, message, args); + va_end(args); +} diff --git a/src/sentry_logs.h b/src/sentry_logs.h new file mode 100644 index 000000000..f8bbc4147 --- /dev/null +++ b/src/sentry_logs.h @@ -0,0 +1,48 @@ +#ifndef SENTRY_LOGS_H_INCLUDED +#define SENTRY_LOGS_H_INCLUDED + +#include "sentry_boot.h" + +/** + * Sentry levels for events and breadcrumbs. + * TODO should these differ from sentry_level_e? (has no `trace` level) + */ +typedef enum sentry_log_level_e { + SENTRY_LOG_LEVEL_TRACE, + SENTRY_LOG_LEVEL_DEBUG, + SENTRY_LOG_LEVEL_INFO, + SENTRY_LOG_LEVEL_WARN, + SENTRY_LOG_LEVEL_ERROR, + SENTRY_LOG_LEVEL_FATAL +} sentry_log_level_t; + +void sentry__logs_log( + sentry_log_level_t level, const char *message, va_list args); + +// TODO again, think about API (functions vs Macros) +void sentry_logger_trace(const char *message, ...); +void sentry_logger_debug(const char *message, ...); +void sentry_logger_info(const char *message, ...); +void sentry_logger_warn(const char *message, ...); +void sentry_logger_error(const char *message, ...); +void sentry_logger_fatal(const char *message, ...); + +#define SENTRY_LOGGER_TRACE(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_TRACE, message, __VA_ARGS__) + +#define SENTRY_LOGGER_DEBUG(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_DEBUG, message, __VA_ARGS__) + +#define SENTRY_LOGGER_INFO(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_INFO, message, __VA_ARGS__) + +#define SENTRY_LOGGER_WARN(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_WARN, message, __VA_ARGS__) + +#define SENTRY_LOGGER_ERROR(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_ERROR, message, __VA_ARGS__) + +#define SENTRY_LOGGER_FATAL(message, ...) \ + sentry__logs_log(SENTRY_LOG_LEVEL_FATAL, message, __VA_ARGS__) + +#endif From a895bb8e71ebdd81af99055ee2c653333311ffe5 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 12 Jun 2025 16:13:42 +0200 Subject: [PATCH 02/27] fix log_level_as_string --- src/sentry_logs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index da7db3e98..6a6986d37 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -25,6 +25,8 @@ log_level_as_string(sentry_log_level_t level) return "ERROR"; case SENTRY_LOG_LEVEL_FATAL: return "FATAL"; + default: + return "UNKNOWN"; } } From 168e51eb3c312286eb355eeb9aeacc598ed37f8b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 17 Jun 2025 15:14:10 +0200 Subject: [PATCH 03/27] attach attributes to logs --- src/sentry_logs.c | 113 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 18 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 6a6986d37..8cb8098fa 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -2,6 +2,7 @@ #include "sentry_core.h" #include "sentry_envelope.h" #include "sentry_options.h" +#include "sentry_scope.h" #include #include @@ -14,22 +15,108 @@ log_level_as_string(sentry_log_level_t level) { switch (level) { case SENTRY_LOG_LEVEL_TRACE: - return "TRACE"; + return "trace"; case SENTRY_LOG_LEVEL_DEBUG: - return "DEBUG"; + return "debug"; case SENTRY_LOG_LEVEL_INFO: - return "INFO"; + return "info"; case SENTRY_LOG_LEVEL_WARN: - return "WARN"; + return "warn"; case SENTRY_LOG_LEVEL_ERROR: - return "ERROR"; + return "error"; case SENTRY_LOG_LEVEL_FATAL: - return "FATAL"; + return "fatal"; default: return "UNKNOWN"; } } +static sentry_value_t +construct_log(sentry_log_level_t level, const char *message, va_list args) +{ + sentry_value_t log = sentry_value_new_object(); + sentry_value_t attributes = sentry_value_new_object(); + + sentry_value_set_by_key(log, "body", sentry_value_new_string(message)); + sentry_value_set_by_key( + log, "level", sentry_value_new_string(log_level_as_string(level))); + + // timestamp in seconds + sentry_value_set_by_key(log, "timestamp", + sentry_value_new_double(sentry__usec_time() / 1000000.0)); + + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_set_by_key(log, "trace_id", + sentry__value_clone(sentry_value_get_by_key( + sentry_value_get_by_key(scope->propagation_context, "trace"), + "trace_id"))); + + sentry_value_t parent_span_id = sentry_value_new_object(); + if (scope->transaction_object) { + sentry_value_set_by_key(parent_span_id, "value", + sentry__value_clone(sentry_value_get_by_key( + scope->transaction_object->inner, "span_id"))); + } + + if (scope->span) { + sentry_value_set_by_key(parent_span_id, "value", + sentry__value_clone( + sentry_value_get_by_key(scope->span->inner, "span_id"))); + } + sentry_value_set_by_key( + parent_span_id, "type", sentry_value_new_string("string")); + // TODO should we only add if either exists? + sentry_value_set_by_key( + attributes, "sentry.trace.parent_span_id", parent_span_id); + } + + SENTRY_WITH_OPTIONS (options) { + if (options->environment) { + sentry_value_t environment = sentry_value_new_object(); + sentry_value_set_by_key(environment, "value", + sentry_value_new_string(options->environment)); + sentry_value_set_by_key( + environment, "type", sentry_value_new_string("string")); + sentry_value_set_by_key( + attributes, "sentry.environment", environment); + } + if (options->release) { + sentry_value_t release = sentry_value_new_object(); + sentry_value_set_by_key( + release, "value", sentry_value_new_string(options->release)); + sentry_value_set_by_key( + release, "type", sentry_value_new_string("string")); + sentry_value_set_by_key(attributes, "sentry.release", release); + } + } + + sentry_value_t sdk_name = sentry_value_new_object(); + sentry_value_set_by_key( + sdk_name, "value", sentry_value_new_string("sentry.native")); + sentry_value_set_by_key( + sdk_name, "type", sentry_value_new_string("string")); + sentry_value_set_by_key(attributes, "sentry.sdk.name", sdk_name); + + sentry_value_t sdk_version = sentry_value_new_object(); + sentry_value_set_by_key( + sdk_version, "value", sentry_value_new_string(sentry_sdk_version())); + sentry_value_set_by_key( + sdk_version, "type", sentry_value_new_string("string")); + sentry_value_set_by_key(attributes, "sentry.sdk.name", sdk_version); + + sentry_value_t message_template = sentry_value_new_object(); + sentry_value_set_by_key( + message_template, "value", sentry_value_new_string(message)); + sentry_value_set_by_key( + message_template, "type", sentry_value_new_string("string")); + sentry_value_set_by_key( + attributes, "sentry.message.template", message_template); + + sentry_value_set_by_key(log, "attributes", attributes); + + return log; +} + void sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) { @@ -40,18 +127,7 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) SENTRY_INFOF("Logging level: %i\n", level); vprintf(message, args); // create log from message - sentry_value_t log = sentry_value_new_object(); - sentry_value_set_by_key(log, "body", sentry_value_new_string(message)); - sentry_value_set_by_key( - log, "level", sentry_value_new_string(log_level_as_string(level))); - // TODO add actual trace_id - sentry_value_set_by_key(log, "trace_id", - sentry_value_new_string("5b8efff798038103d269b633813fc60c")); - // timestamp in seconds - sentry_value_set_by_key(log, "timestamp", - sentry_value_new_double(sentry__usec_time() / 1000000.0)); - // TODO add log attributes - // https://develop.sentry.dev/sdk/telemetry/logs/#default-attributes + sentry_value_t log = construct_log(level, message, args); // TODO split up the code below for batched log sending // e.g. could we store logs on the scope? @@ -62,6 +138,7 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) // sending of the envelope sentry_envelope_t *envelope = sentry__envelope_new(); sentry__envelope_add_logs(envelope, logs); + // TODO remove debug write to file below sentry_envelope_write_to_file(envelope, "logs_envelope.json"); SENTRY_WITH_OPTIONS (options) { sentry__capture_envelope(options->transport, envelope); From 07b1deee172c12f5dde9a03a86d3bce4d04ef332 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 17 Jun 2025 17:43:01 +0200 Subject: [PATCH 04/27] attach formatted message + args --- src/sentry_logs.c | 170 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 169 insertions(+), 1 deletion(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 8cb8098fa..97409a101 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -31,13 +31,169 @@ log_level_as_string(sentry_log_level_t level) } } +static void +populate_message_parameters( + const char *message, va_list args, sentry_value_t attributes) +{ + if (!message || sentry_value_is_null(attributes)) { + return; + } + + const char *fmt_ptr = message; + int param_index = 0; + va_list args_copy; + va_copy(args_copy, args); + + while (*fmt_ptr) { + // Find the next format specifier + if (*fmt_ptr == '%') { + fmt_ptr++; // Skip the '%' + + // Skip flags, width, and precision + while (*fmt_ptr + && (*fmt_ptr == '-' || *fmt_ptr == '+' || *fmt_ptr == ' ' + || *fmt_ptr == '#' || *fmt_ptr == '0')) { + fmt_ptr++; + } + while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { + fmt_ptr++; + } + if (*fmt_ptr == '.') { + fmt_ptr++; + while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { + fmt_ptr++; + } + } + + // Skip length modifiers + while (*fmt_ptr + && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L' + || *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) { + fmt_ptr++; + } + + if (*fmt_ptr == '%') { + // Escaped '%', not a format specifier + fmt_ptr++; + continue; + } + + // Get the conversion specifier + char conversion = *fmt_ptr; + if (conversion) { + char key[64]; + snprintf(key, sizeof(key), "sentry.message.parameter.%d", + param_index); + + sentry_value_t param_obj = sentry_value_new_object(); + + switch (conversion) { + case 'd': + case 'i': { + int val = va_arg(args_copy, int); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_int32(val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("integer")); + break; + } + case 'u': + case 'x': + case 'X': + case 'o': { + unsigned int val = va_arg(args_copy, unsigned int); + sentry_value_set_by_key(param_obj, "value", + sentry_value_new_int32((int32_t)val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("integer")); + break; + } + case 'f': + case 'F': + case 'e': + case 'E': + case 'g': + case 'G': { + double val = va_arg(args_copy, double); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_double(val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("double")); + break; + } + case 'c': { + int val = va_arg(args_copy, int); + char str[2] = { (char)val, '\0' }; + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(str)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + case 's': { + const char *val = va_arg(args_copy, const char *); + if (val) { + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(val)); + } else { + sentry_value_set_by_key(param_obj, "value", + sentry_value_new_string("(null)")); + } + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + case 'p': { + void *val = va_arg(args_copy, void *); + char ptr_str[32]; + snprintf(ptr_str, sizeof(ptr_str), "%p", val); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(ptr_str)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + default: + // Unknown format specifier, skip the argument + (void)va_arg(args_copy, void *); + sentry_value_set_by_key(param_obj, "value", + sentry_value_new_string("(unknown)")); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + + sentry_value_set_by_key(attributes, key, param_obj); + param_index++; + fmt_ptr++; + } + } else { + fmt_ptr++; + } + } + + va_end(args_copy); +} + static sentry_value_t construct_log(sentry_log_level_t level, const char *message, va_list args) { sentry_value_t log = sentry_value_new_object(); sentry_value_t attributes = sentry_value_new_object(); - sentry_value_set_by_key(log, "body", sentry_value_new_string(message)); + va_list args_copy; + va_copy(args_copy, args); + int size = vsnprintf(NULL, 0, message, args_copy) + 1; + va_end(args_copy); + char *fmt_message = sentry_malloc(size); + if (!fmt_message) { + return sentry_value_new_null(); + } + + vsnprintf(fmt_message, size, message, args); + + sentry_value_set_by_key(log, "body", sentry_value_new_string(fmt_message)); + sentry_free(fmt_message); sentry_value_set_by_key( log, "level", sentry_value_new_string(log_level_as_string(level))); @@ -112,6 +268,9 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_set_by_key( attributes, "sentry.message.template", message_template); + // Parse variadic arguments and add them to attributes + populate_message_parameters(message, args, attributes); + sentry_value_set_by_key(log, "attributes", attributes); return log; @@ -147,6 +306,15 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) // TODO think about the structure below, is this how we want the API to // function? +void +sentry_logger_trace(const char *message, ...) +{ + va_list args; + va_start(args, message); + sentry__logs_log(SENTRY_LOG_LEVEL_TRACE, message, args); + va_end(args); +} + void sentry_logger_debug(const char *message, ...) { From cdd2ab34c385dadc358c4267008d0b44581ff46f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 17 Jun 2025 17:51:08 +0200 Subject: [PATCH 05/27] add to example --- examples/example.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/examples/example.c b/examples/example.c index bcb3d519e..eac2d6313 100644 --- a/examples/example.c +++ b/examples/example.c @@ -363,14 +363,26 @@ main(int argc, char **argv) // TODO incorporate into test if (options->enable_logs) { - sentry_logger_debug( - "We log it up %i percent, %s style\n", 100, "debug"); - sentry_logger_info("We log it up %i percent, %s style\n", 100, "info"); - sentry_logger_warn("We log it up %i percent, %s style\n", 100, "warn"); - sentry_logger_error( - "We log it up %i percent, %s style\n", 100, "error"); - sentry_logger_fatal( - "We log it up %i percent, %s style\n", 100, "fatal"); + sentry_logger_trace( + "We log it up %i percent, %s style\n", 100, "trace"); + // sentry_logger_debug( + // "We log it up %i percent, %s style\n", 100, "debug"); + // sentry_logger_info("We log it up %i percent, %s style\n", 100, + // "info"); sentry_logger_warn("We log it up %i percent, %s style\n", + // 100, "warn"); sentry_logger_error( + // "We log it up %i percent, %s style\n", 100, "error"); + // sentry_logger_fatal( + // "We log it up %i percent, %s style\n", 100, "fatal"); + + // Test the logger with various parameter types + sentry_logger_info( + "API call to %s completed in %d ms with %f success rate", + "/api/products", 2500, 0.95); + + sentry_logger_warn("Processing %d items, found %u errors, pointer: %p", + 100, 5u, (void *)0x12345678); + + sentry_logger_error("Character '%c' is invalid", 'X'); } if (!has_arg(argc, argv, "no-setup")) { @@ -580,6 +592,9 @@ main(int argc, char **argv) SENTRY_LEVEL_INFO, "my-logger", "Hello World!"); sentry_capture_event(event); } + if (options->enable_logs) { + sentry_logger_debug("logging after scoped transaction event"); + } sentry_transaction_finish(tx); } From d4834d09d7b8fdf86d91e98676b7155721f988c6 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 18 Jun 2025 13:29:40 +0200 Subject: [PATCH 06/27] add more attributes --- src/sentry_envelope.c | 2 - src/sentry_logs.c | 92 ++++++++++++++++++++++++++----------------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index dc023d6f5..5e79c8424 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -298,8 +298,6 @@ sentry__envelope_add_transaction( sentry_envelope_item_t * sentry__envelope_add_logs(sentry_envelope_t *envelope, sentry_value_t logs) { - // TODO ensure this starts out correctly; we get {dsn:...} as a header - // (but don't think we need it?) sentry_envelope_item_t *item = envelope_add_item(envelope); if (!item) { return NULL; diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 97409a101..5a4dd9471 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -2,6 +2,7 @@ #include "sentry_core.h" #include "sentry_envelope.h" #include "sentry_options.h" +#include "sentry_os.h" #include "sentry_scope.h" #include @@ -33,7 +34,7 @@ log_level_as_string(sentry_log_level_t level) static void populate_message_parameters( - const char *message, va_list args, sentry_value_t attributes) + sentry_value_t attributes, const char *message, va_list args) { if (!message || sentry_value_is_null(attributes)) { return; @@ -175,6 +176,16 @@ populate_message_parameters( va_end(args_copy); } +static void +add_attribute(sentry_value_t attributes, sentry_value_t value, const char *type, + const char *name) +{ + sentry_value_t param_obj = sentry_value_new_object(); + sentry_value_set_by_key(param_obj, "value", value); + sentry_value_set_by_key(param_obj, "type", sentry_value_new_string(type)); + sentry_value_set_by_key(attributes, name, param_obj); +} + static sentry_value_t construct_log(sentry_log_level_t level, const char *message, va_list args) { @@ -224,52 +235,59 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) // TODO should we only add if either exists? sentry_value_set_by_key( attributes, "sentry.trace.parent_span_id", parent_span_id); + + if (!sentry_value_is_null(scope->user)) { + sentry_value_t user_id = sentry_value_get_by_key(scope->user, "id"); + if (!sentry_value_is_null(user_id)) { + add_attribute(attributes, user_id, "string", "user.id"); + } + sentry_value_t user_username + = sentry_value_get_by_key(scope->user, "username"); + if (!sentry_value_is_null(user_username)) { + add_attribute(attributes, user_username, "string", "user.name"); + } + sentry_value_t user_email + = sentry_value_get_by_key(scope->user, "email"); + if (!sentry_value_is_null(user_email)) { + add_attribute(attributes, user_email, "string", "user.email"); + } + } + sentry_value_t os_context = sentry__get_os_context(); + if (!sentry_value_is_null(os_context)) { + sentry_value_t os_name + = sentry_value_get_by_key(os_context, "name"); + sentry_value_t os_version + = sentry_value_get_by_key(os_context, "version"); + if (!sentry_value_is_null(os_name)) { + add_attribute(attributes, os_name, "string", "os.name"); + } + if (!sentry_value_is_null(os_version)) { + add_attribute(attributes, os_version, "string", "os.version"); + } + } } SENTRY_WITH_OPTIONS (options) { if (options->environment) { - sentry_value_t environment = sentry_value_new_object(); - sentry_value_set_by_key(environment, "value", - sentry_value_new_string(options->environment)); - sentry_value_set_by_key( - environment, "type", sentry_value_new_string("string")); - sentry_value_set_by_key( - attributes, "sentry.environment", environment); + add_attribute(attributes, + sentry_value_new_string(options->environment), "string", + "sentry.environment"); } if (options->release) { - sentry_value_t release = sentry_value_new_object(); - sentry_value_set_by_key( - release, "value", sentry_value_new_string(options->release)); - sentry_value_set_by_key( - release, "type", sentry_value_new_string("string")); - sentry_value_set_by_key(attributes, "sentry.release", release); + add_attribute(attributes, sentry_value_new_string(options->release), + "string", "sentry.release"); } } - sentry_value_t sdk_name = sentry_value_new_object(); - sentry_value_set_by_key( - sdk_name, "value", sentry_value_new_string("sentry.native")); - sentry_value_set_by_key( - sdk_name, "type", sentry_value_new_string("string")); - sentry_value_set_by_key(attributes, "sentry.sdk.name", sdk_name); - - sentry_value_t sdk_version = sentry_value_new_object(); - sentry_value_set_by_key( - sdk_version, "value", sentry_value_new_string(sentry_sdk_version())); - sentry_value_set_by_key( - sdk_version, "type", sentry_value_new_string("string")); - sentry_value_set_by_key(attributes, "sentry.sdk.name", sdk_version); - - sentry_value_t message_template = sentry_value_new_object(); - sentry_value_set_by_key( - message_template, "value", sentry_value_new_string(message)); - sentry_value_set_by_key( - message_template, "type", sentry_value_new_string("string")); - sentry_value_set_by_key( - attributes, "sentry.message.template", message_template); + add_attribute(attributes, sentry_value_new_string("sentry.native"), + "string", "sentry.sdk.name"); + add_attribute(attributes, sentry_value_new_string(sentry_sdk_version()), + "string", "sentry.sdk.version"); + add_attribute(attributes, sentry_value_new_string(message), "string", + "sentry.message.template"); // Parse variadic arguments and add them to attributes - populate_message_parameters(message, args, attributes); + populate_message_parameters(attributes, message, args); sentry_value_set_by_key(log, "attributes", attributes); @@ -295,6 +313,8 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_append(logs_list, log); sentry_value_set_by_key(logs, "items", logs_list); // sending of the envelope + // TODO ensure envelope starts out correctly; we get {dsn:...} as a header + // (but not sure we need it?) -> we could add bool to envelope creation sentry_envelope_t *envelope = sentry__envelope_new(); sentry__envelope_add_logs(envelope, logs); // TODO remove debug write to file below From d1444e09df067d19b9c2b6a2341b9197fba49a56 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 18 Jun 2025 16:48:38 +0200 Subject: [PATCH 07/27] cleanup --- examples/example.c | 28 +++++++++++++--------------- include/sentry.h | 13 ++++++------- src/sentry_logs.c | 27 ++++++++++++++++----------- src/sentry_logs.h | 26 -------------------------- 4 files changed, 35 insertions(+), 59 deletions(-) diff --git a/examples/example.c b/examples/example.c index eac2d6313..8a0484c69 100644 --- a/examples/example.c +++ b/examples/example.c @@ -360,29 +360,27 @@ main(int argc, char **argv) } sentry_init(options); - + sentry_value_t new_user + = sentry_value_new_user("42", "marvin", "hitch@hiker.com", NULL); + sentry_set_user(new_user); // TODO incorporate into test if (options->enable_logs) { - sentry_logger_trace( - "We log it up %i percent, %s style\n", 100, "trace"); - // sentry_logger_debug( - // "We log it up %i percent, %s style\n", 100, "debug"); - // sentry_logger_info("We log it up %i percent, %s style\n", 100, - // "info"); sentry_logger_warn("We log it up %i percent, %s style\n", - // 100, "warn"); sentry_logger_error( - // "We log it up %i percent, %s style\n", 100, "error"); - // sentry_logger_fatal( - // "We log it up %i percent, %s style\n", 100, "fatal"); + sentry_log_trace("We log it up %i percent, %s style\n", 100, "trace"); + sentry_log_debug("We log it up %i percent, %s style\n", 100, "debug"); + sentry_log_info("We log it up %i percent, %s style\n", 100, "info"); + sentry_log_warn("We log it up %i percent, %s style\n", 100, "warn"); + sentry_log_error("We log it up %i percent, %s style\n", 100, "error"); + sentry_log_fatal("We log it up %i percent, %s style\n", 100, "fatal"); // Test the logger with various parameter types - sentry_logger_info( + sentry_log_info( "API call to %s completed in %d ms with %f success rate", "/api/products", 2500, 0.95); - sentry_logger_warn("Processing %d items, found %u errors, pointer: %p", + sentry_log_warn("Processing %d items, found %u errors, pointer: %p", 100, 5u, (void *)0x12345678); - sentry_logger_error("Character '%c' is invalid", 'X'); + sentry_log_error("Character '%c' is invalid", 'X'); } if (!has_arg(argc, argv, "no-setup")) { @@ -593,7 +591,7 @@ main(int argc, char **argv) sentry_capture_event(event); } if (options->enable_logs) { - sentry_logger_debug("logging after scoped transaction event"); + sentry_log_debug("logging after scoped transaction event"); } sentry_transaction_finish(tx); diff --git a/include/sentry.h b/include/sentry.h index 70ec092db..60d9f59c8 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1750,13 +1750,12 @@ SENTRY_EXPERIMENTAL_API void sentry_options_set_enable_logs( SENTRY_EXPERIMENTAL_API int sentry_options_get_enable_logs( const sentry_options_t *opts); -// TODO think about API; functions or MACROs? -SENTRY_EXPERIMENTAL_API void sentry_logger_trace(const char *message, ...); -SENTRY_EXPERIMENTAL_API void sentry_logger_debug(const char *message, ...); -SENTRY_EXPERIMENTAL_API void sentry_logger_info(const char *message, ...); -SENTRY_EXPERIMENTAL_API void sentry_logger_warn(const char *message, ...); -SENTRY_EXPERIMENTAL_API void sentry_logger_error(const char *message, ...); -SENTRY_EXPERIMENTAL_API void sentry_logger_fatal(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_trace(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_debug(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_info(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_warn(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_error(const char *message, ...); +SENTRY_EXPERIMENTAL_API void sentry_log_fatal(const char *message, ...); #ifdef SENTRY_PLATFORM_LINUX diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 5a4dd9471..12cca66e1 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -11,6 +11,10 @@ #include #include +// TODO discuss replacing this with sentry_level_t +// -> issue is 'warn' vs 'warning' +// https://develop.sentry.dev/sdk/data-model/event-payloads/#optional-attributes +// https://develop.sentry.dev/sdk/telemetry/logs/#log-severity-level char * log_level_as_string(sentry_log_level_t level) { @@ -232,9 +236,12 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) } sentry_value_set_by_key( parent_span_id, "type", sentry_value_new_string("string")); - // TODO should we only add if either exists? - sentry_value_set_by_key( - attributes, "sentry.trace.parent_span_id", parent_span_id); + if (scope->transaction_object || scope->span) { + sentry_value_set_by_key( + attributes, "sentry.trace.parent_span_id", parent_span_id); + } else { + sentry_value_decref(parent_span_id); + } if (!sentry_value_is_null(scope->user)) { sentry_value_t user_id = sentry_value_get_by_key(scope->user, "id"); @@ -324,10 +331,8 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) } } -// TODO think about the structure below, is this how we want the API to -// function? void -sentry_logger_trace(const char *message, ...) +sentry_log_trace(const char *message, ...) { va_list args; va_start(args, message); @@ -336,7 +341,7 @@ sentry_logger_trace(const char *message, ...) } void -sentry_logger_debug(const char *message, ...) +sentry_log_debug(const char *message, ...) { va_list args; va_start(args, message); @@ -345,7 +350,7 @@ sentry_logger_debug(const char *message, ...) } void -sentry_logger_info(const char *message, ...) +sentry_log_info(const char *message, ...) { va_list args; va_start(args, message); @@ -354,7 +359,7 @@ sentry_logger_info(const char *message, ...) } void -sentry_logger_warn(const char *message, ...) +sentry_log_warn(const char *message, ...) { va_list args; va_start(args, message); @@ -363,7 +368,7 @@ sentry_logger_warn(const char *message, ...) } void -sentry_logger_error(const char *message, ...) +sentry_log_error(const char *message, ...) { va_list args; va_start(args, message); @@ -372,7 +377,7 @@ sentry_logger_error(const char *message, ...) } void -sentry_logger_fatal(const char *message, ...) +sentry_log_fatal(const char *message, ...) { va_list args; va_start(args, message); diff --git a/src/sentry_logs.h b/src/sentry_logs.h index f8bbc4147..b87fc9370 100644 --- a/src/sentry_logs.h +++ b/src/sentry_logs.h @@ -19,30 +19,4 @@ typedef enum sentry_log_level_e { void sentry__logs_log( sentry_log_level_t level, const char *message, va_list args); -// TODO again, think about API (functions vs Macros) -void sentry_logger_trace(const char *message, ...); -void sentry_logger_debug(const char *message, ...); -void sentry_logger_info(const char *message, ...); -void sentry_logger_warn(const char *message, ...); -void sentry_logger_error(const char *message, ...); -void sentry_logger_fatal(const char *message, ...); - -#define SENTRY_LOGGER_TRACE(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_TRACE, message, __VA_ARGS__) - -#define SENTRY_LOGGER_DEBUG(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_DEBUG, message, __VA_ARGS__) - -#define SENTRY_LOGGER_INFO(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_INFO, message, __VA_ARGS__) - -#define SENTRY_LOGGER_WARN(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_WARN, message, __VA_ARGS__) - -#define SENTRY_LOGGER_ERROR(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_ERROR, message, __VA_ARGS__) - -#define SENTRY_LOGGER_FATAL(message, ...) \ - sentry__logs_log(SENTRY_LOG_LEVEL_FATAL, message, __VA_ARGS__) - #endif From 8a3ae2aa27217143f0eb5eb95c3b69922e096b33 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:12:25 +0200 Subject: [PATCH 08/27] windows warning-as-error --- src/sentry_envelope.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index 5e79c8424..da67ae2de 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -314,7 +314,7 @@ sentry__envelope_add_logs(sentry_envelope_t *envelope, sentry_value_t logs) sentry__envelope_item_set_header( item, "type", sentry_value_new_string("log")); sentry__envelope_item_set_header(item, "item_count", - sentry_value_new_int32(sentry_value_get_length(logs))); + sentry_value_new_int32((int32_t)sentry_value_get_length(logs))); sentry__envelope_item_set_header(item, "content_type", sentry_value_new_string("application/vnd.sentry.items.log+json")); From 8b869d7a5b30996e226631d726da38501eb3eb48 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 20 Jun 2025 12:27:33 +0200 Subject: [PATCH 09/27] windows warning-as-error v2 --- src/sentry_logs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 12cca66e1..d60be2587 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -15,7 +15,7 @@ // -> issue is 'warn' vs 'warning' // https://develop.sentry.dev/sdk/data-model/event-payloads/#optional-attributes // https://develop.sentry.dev/sdk/telemetry/logs/#log-severity-level -char * +static char * log_level_as_string(sentry_log_level_t level) { switch (level) { @@ -198,8 +198,9 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) va_list args_copy; va_copy(args_copy, args); - int size = vsnprintf(NULL, 0, message, args_copy) + 1; + int len = vsnprintf(NULL, 0, message, args_copy) + 1; va_end(args_copy); + size_t size = (size_t)len; char *fmt_message = sentry_malloc(size); if (!fmt_message) { return sentry_value_new_null(); @@ -214,7 +215,7 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) // timestamp in seconds sentry_value_set_by_key(log, "timestamp", - sentry_value_new_double(sentry__usec_time() / 1000000.0)); + sentry_value_new_double((double)sentry__usec_time() / 1000000.0)); SENTRY_WITH_SCOPE_MUT (scope) { sentry_value_set_by_key(log, "trace_id", From 74a823dfb70aee884836b612fde2579bce62017f Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 20 Jun 2025 13:21:00 +0200 Subject: [PATCH 10/27] windows warning-as-error v2 (final) --- src/sentry_logs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index d60be2587..c70f7e8a2 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -214,8 +214,9 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) log, "level", sentry_value_new_string(log_level_as_string(level))); // timestamp in seconds + uint64_t usec_time = sentry__usec_time(); sentry_value_set_by_key(log, "timestamp", - sentry_value_new_double((double)sentry__usec_time() / 1000000.0)); + sentry_value_new_double((double)usec_time / 1000000.0)); SENTRY_WITH_SCOPE_MUT (scope) { sentry_value_set_by_key(log, "trace_id", From ebc5ae3b5bf58ecfd6a0522169cdbc007a70a732 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 23 Jun 2025 17:05:36 +0200 Subject: [PATCH 11/27] add unit tests for initial logs --- examples/example.c | 10 ++-- src/sentry_logs.c | 4 -- tests/unit/CMakeLists.txt | 1 + tests/unit/test_logs.c | 100 ++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 3 ++ 5 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_logs.c diff --git a/examples/example.c b/examples/example.c index 8a0484c69..9eb235e91 100644 --- a/examples/example.c +++ b/examples/example.c @@ -16,8 +16,6 @@ # undef NDEBUG #endif -#include "../src/sentry_options.h" - #include #ifdef SENTRY_PLATFORM_WINDOWS @@ -360,11 +358,9 @@ main(int argc, char **argv) } sentry_init(options); - sentry_value_t new_user - = sentry_value_new_user("42", "marvin", "hitch@hiker.com", NULL); - sentry_set_user(new_user); + // TODO incorporate into test - if (options->enable_logs) { + if (sentry_options_get_enable_logs(options)) { sentry_log_trace("We log it up %i percent, %s style\n", 100, "trace"); sentry_log_debug("We log it up %i percent, %s style\n", 100, "debug"); sentry_log_info("We log it up %i percent, %s style\n", 100, "info"); @@ -590,7 +586,7 @@ main(int argc, char **argv) SENTRY_LEVEL_INFO, "my-logger", "Hello World!"); sentry_capture_event(event); } - if (options->enable_logs) { + if (sentry_options_get_enable_logs(options)) { sentry_log_debug("logging after scoped transaction event"); } diff --git a/src/sentry_logs.c b/src/sentry_logs.c index c70f7e8a2..ac8fb97dc 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -310,8 +310,6 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) if (!options->enable_logs) return; } - SENTRY_INFOF("Logging level: %i\n", level); - vprintf(message, args); // create log from message sentry_value_t log = construct_log(level, message, args); @@ -322,8 +320,6 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_append(logs_list, log); sentry_value_set_by_key(logs, "items", logs_list); // sending of the envelope - // TODO ensure envelope starts out correctly; we get {dsn:...} as a header - // (but not sure we need it?) -> we could add bool to envelope creation sentry_envelope_t *envelope = sentry__envelope_new(); sentry__envelope_add_logs(envelope, logs); // TODO remove debug write to file below diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 23038f408..877d6b7a3 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -28,6 +28,7 @@ add_executable(sentry_test_unit test_fuzzfailures.c test_info.c test_logger.c + test_logs.c test_modulefinder.c test_mpack.c test_options.c diff --git a/tests/unit/test_logs.c b/tests/unit/test_logs.c new file mode 100644 index 000000000..4ad2b4049 --- /dev/null +++ b/tests/unit/test_logs.c @@ -0,0 +1,100 @@ +#include "sentry_logs.h" +#include "sentry_testsupport.h" + +#include "sentry_envelope.h" + +// TODO these tests will need updating after batching is implemented + +static void +validate_logs_envelope(sentry_envelope_t *envelope, void *data) +{ + uint64_t *called = data; + *called += 1; + + // Verify we have at least one envelope item + TEST_CHECK(sentry__envelope_get_item_count(envelope) > 0); + + // Get the first item and check it's a logs item + const sentry_envelope_item_t *item = sentry__envelope_get_item(envelope, 0); + sentry_value_t type_header = sentry__envelope_item_get_header(item, "type"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(type_header), "log"); + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(basic_logging_functionality) +{ + uint64_t called_transport = 0; + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_enable_logs(options, true); + + sentry_transport_t *transport + = sentry_transport_new(validate_logs_envelope); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_init(options); + + // These should not crash and should respect the enable_logs option + sentry_log_trace("Trace message"); + sentry_log_debug("Debug message"); + sentry_log_info("Info message"); + sentry_log_warn("Warning message"); + sentry_log_error("Error message"); + sentry_log_fatal("Fatal message"); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called_transport, 6); +} + +SENTRY_TEST(logs_disabled_by_default) +{ + uint64_t called_transport = 0; + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + sentry_transport_t *transport + = sentry_transport_new(validate_logs_envelope); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + // Don't explicitly enable logs - they should be disabled by default + sentry_init(options); + + sentry_log_info("This should not be sent"); + + sentry_close(); + + // Transport should not be called since logs are disabled + TEST_CHECK_INT_EQUAL(called_transport, 0); +} + +SENTRY_TEST(formatted_log_messages) +{ + uint64_t called_transport = 0; + + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_enable_logs(options, true); + + sentry_transport_t *transport + = sentry_transport_new(validate_logs_envelope); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_init(options); + + // Test format specifiers + sentry_log_info("String: %s, Integer: %d, Float: %.2f", "test", 42, 3.14); + sentry_log_warn("Character: %c, Hex: 0x%x", 'A', 255); + sentry_log_error("Pointer: %p", (void *)0x1234); + + sentry_close(); + + // Transport should be called three times + TEST_CHECK_INT_EQUAL(called_transport, 3); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 048f554c5..8fba36755 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -11,6 +11,7 @@ XX(basic_http_request_preparation_for_event_with_attachment) XX(basic_http_request_preparation_for_minidump) XX(basic_http_request_preparation_for_transaction) XX(basic_http_request_preparation_for_user_feedback) +XX(basic_logging_functionality) XX(basic_spans) XX(basic_tracing_context) XX(basic_transaction) @@ -51,6 +52,7 @@ XX(dsn_without_project_id_is_invalid) XX(dsn_without_url_scheme_is_invalid) XX(empty_transport) XX(exception_without_type_or_value_still_valid) +XX(formatted_log_messages) XX(fuzz_json) XX(init_failure) XX(internal_uuid_api) @@ -58,6 +60,7 @@ XX(invalid_dsn) XX(invalid_proxy) XX(iso_time) XX(lazy_attachments) +XX(logs_disabled_by_default) XX(message_with_null_text_is_valid) XX(module_addr) XX(module_finder) From 9b42629a5de7bb4f791261918c7d627c945e671d Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 14:18:15 +0200 Subject: [PATCH 12/27] memleak attempted fix --- src/sentry_logs.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index ac8fb97dc..1142a46be 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -196,17 +196,22 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_t log = sentry_value_new_object(); sentry_value_t attributes = sentry_value_new_object(); - va_list args_copy; - va_copy(args_copy, args); - int len = vsnprintf(NULL, 0, message, args_copy) + 1; - va_end(args_copy); + va_list args_copy_1, args_copy_2, args_copy_3; + va_copy(args_copy_1, args); + va_copy(args_copy_2, args); + va_copy(args_copy_3, args); + int len = vsnprintf(NULL, 0, message, args_copy_1) + 1; + va_end(args_copy_1); size_t size = (size_t)len; char *fmt_message = sentry_malloc(size); if (!fmt_message) { + va_end(args_copy_2); + va_end(args_copy_3); return sentry_value_new_null(); } - vsnprintf(fmt_message, size, message, args); + vsnprintf(fmt_message, size, message, args_copy_2); + va_end(args_copy_2); sentry_value_set_by_key(log, "body", sentry_value_new_string(fmt_message)); sentry_free(fmt_message); @@ -263,10 +268,10 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) } sentry_value_t os_context = sentry__get_os_context(); if (!sentry_value_is_null(os_context)) { - sentry_value_t os_name - = sentry_value_get_by_key(os_context, "name"); - sentry_value_t os_version - = sentry_value_get_by_key(os_context, "version"); + sentry_value_t os_name = sentry__value_clone( + sentry_value_get_by_key(os_context, "name")); + sentry_value_t os_version = sentry__value_clone( + sentry_value_get_by_key(os_context, "version")); if (!sentry_value_is_null(os_name)) { add_attribute(attributes, os_name, "string", "os.name"); } @@ -274,6 +279,7 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) add_attribute(attributes, os_version, "string", "os.version"); } } + sentry_value_decref(os_context); } SENTRY_WITH_OPTIONS (options) { @@ -296,7 +302,8 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) "sentry.message.template"); // Parse variadic arguments and add them to attributes - populate_message_parameters(attributes, message, args); + populate_message_parameters(attributes, message, args_copy_3); + va_end(args_copy_3); sentry_value_set_by_key(log, "attributes", attributes); @@ -327,6 +334,8 @@ sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) SENTRY_WITH_OPTIONS (options) { sentry__capture_envelope(options->transport, envelope); } + // For now, free the logs object since envelope doesn't take ownership + sentry_value_decref(logs); } void From be24e762fb756d73a6a8719fa7fe4b1f21c00d3e Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 14:56:10 +0200 Subject: [PATCH 13/27] memleak attempted fix 2 --- src/sentry_logs.c | 61 ++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 1142a46be..75fffa7a9 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -191,7 +191,8 @@ add_attribute(sentry_value_t attributes, sentry_value_t value, const char *type, } static sentry_value_t -construct_log(sentry_log_level_t level, const char *message, va_list args) +construct_log(const sentry_options_t *options, sentry_log_level_t level, + const char *message, va_list args) { sentry_value_t log = sentry_value_new_object(); sentry_value_t attributes = sentry_value_new_object(); @@ -282,16 +283,13 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_decref(os_context); } - SENTRY_WITH_OPTIONS (options) { - if (options->environment) { - add_attribute(attributes, - sentry_value_new_string(options->environment), "string", - "sentry.environment"); - } - if (options->release) { - add_attribute(attributes, sentry_value_new_string(options->release), - "string", "sentry.release"); - } + if (options->environment) { + add_attribute(attributes, sentry_value_new_string(options->environment), + "string", "sentry.environment"); + } + if (options->release) { + add_attribute(attributes, sentry_value_new_string(options->release), + "string", "sentry.release"); } add_attribute(attributes, sentry_value_new_string("sentry.native"), @@ -314,28 +312,27 @@ void sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) { SENTRY_WITH_OPTIONS (options) { - if (!options->enable_logs) - return; - } - // create log from message - sentry_value_t log = construct_log(level, message, args); - - // TODO split up the code below for batched log sending - // e.g. could we store logs on the scope? - sentry_value_t logs = sentry_value_new_object(); - sentry_value_t logs_list = sentry_value_new_list(); - sentry_value_append(logs_list, log); - sentry_value_set_by_key(logs, "items", logs_list); - // sending of the envelope - sentry_envelope_t *envelope = sentry__envelope_new(); - sentry__envelope_add_logs(envelope, logs); - // TODO remove debug write to file below - sentry_envelope_write_to_file(envelope, "logs_envelope.json"); - SENTRY_WITH_OPTIONS (options) { - sentry__capture_envelope(options->transport, envelope); + if (options->enable_logs) { + // create log from message + sentry_value_t log = construct_log(options, level, message, args); + + // TODO split up the code below for batched log sending + // e.g. could we store logs on the scope? + sentry_value_t logs = sentry_value_new_object(); + sentry_value_t logs_list = sentry_value_new_list(); + sentry_value_append(logs_list, log); + sentry_value_set_by_key(logs, "items", logs_list); + // sending of the envelope + sentry_envelope_t *envelope = sentry__envelope_new(); + sentry__envelope_add_logs(envelope, logs); + // TODO remove debug write to file below + // sentry_envelope_write_to_file(envelope, "logs_envelope.json"); + sentry__capture_envelope(options->transport, envelope); + // For now, free the logs object since envelope doesn't take + // ownership + sentry_value_decref(logs); + } } - // For now, free the logs object since envelope doesn't take ownership - sentry_value_decref(logs); } void From ca7519893760fb78fa89418387d00b8f5a95a010 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 15:29:25 +0200 Subject: [PATCH 14/27] cleanup --- src/sentry_logs.c | 62 ++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 75fffa7a9..cbf9d3f8e 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -191,8 +191,7 @@ add_attribute(sentry_value_t attributes, sentry_value_t value, const char *type, } static sentry_value_t -construct_log(const sentry_options_t *options, sentry_log_level_t level, - const char *message, va_list args) +construct_log(sentry_log_level_t level, const char *message, va_list args) { sentry_value_t log = sentry_value_new_object(); sentry_value_t attributes = sentry_value_new_object(); @@ -282,14 +281,16 @@ construct_log(const sentry_options_t *options, sentry_log_level_t level, } sentry_value_decref(os_context); } - - if (options->environment) { - add_attribute(attributes, sentry_value_new_string(options->environment), - "string", "sentry.environment"); - } - if (options->release) { - add_attribute(attributes, sentry_value_new_string(options->release), - "string", "sentry.release"); + SENTRY_WITH_OPTIONS (options) { + if (options->environment) { + add_attribute(attributes, + sentry_value_new_string(options->environment), "string", + "sentry.environment"); + } + if (options->release) { + add_attribute(attributes, sentry_value_new_string(options->release), + "string", "sentry.release"); + } } add_attribute(attributes, sentry_value_new_string("sentry.native"), @@ -311,27 +312,32 @@ construct_log(const sentry_options_t *options, sentry_log_level_t level, void sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) { + bool enable_logs = false; SENTRY_WITH_OPTIONS (options) { - if (options->enable_logs) { - // create log from message - sentry_value_t log = construct_log(options, level, message, args); - - // TODO split up the code below for batched log sending - // e.g. could we store logs on the scope? - sentry_value_t logs = sentry_value_new_object(); - sentry_value_t logs_list = sentry_value_new_list(); - sentry_value_append(logs_list, log); - sentry_value_set_by_key(logs, "items", logs_list); - // sending of the envelope - sentry_envelope_t *envelope = sentry__envelope_new(); - sentry__envelope_add_logs(envelope, logs); - // TODO remove debug write to file below - // sentry_envelope_write_to_file(envelope, "logs_envelope.json"); + if (options->enable_logs) + enable_logs = true; + } + if (enable_logs) { + // create log from message + sentry_value_t log = construct_log(level, message, args); + + // TODO split up the code below for batched log sending + // e.g. could we store logs on the scope? + sentry_value_t logs = sentry_value_new_object(); + sentry_value_t logs_list = sentry_value_new_list(); + sentry_value_append(logs_list, log); + sentry_value_set_by_key(logs, "items", logs_list); + // sending of the envelope + sentry_envelope_t *envelope = sentry__envelope_new(); + sentry__envelope_add_logs(envelope, logs); + // TODO remove debug write to file below + // sentry_envelope_write_to_file(envelope, "logs_envelope.json"); + SENTRY_WITH_OPTIONS (options) { sentry__capture_envelope(options->transport, envelope); - // For now, free the logs object since envelope doesn't take - // ownership - sentry_value_decref(logs); } + // For now, free the logs object since envelope doesn't take + // ownership + sentry_value_decref(logs); } } From d57fcf3860ff3d11d57ef7d611f6f5f4c4d2265c Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 16:07:07 +0200 Subject: [PATCH 15/27] use `sentry_level_t` instead of new log level enum --- include/sentry.h | 1 + src/sentry_logs.c | 46 +++++++++++++++++++--------------------------- src/sentry_logs.h | 16 +--------------- src/sentry_value.c | 2 ++ 4 files changed, 23 insertions(+), 42 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 60d9f59c8..482e14751 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -370,6 +370,7 @@ SENTRY_API char *sentry_value_to_json(sentry_value_t value); * Sentry levels for events and breadcrumbs. */ typedef enum sentry_level_e { + SENTRY_LEVEL_TRACE = -2, SENTRY_LEVEL_DEBUG = -1, SENTRY_LEVEL_INFO = 0, SENTRY_LEVEL_WARNING = 1, diff --git a/src/sentry_logs.c b/src/sentry_logs.c index cbf9d3f8e..a323a7afd 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -5,34 +5,26 @@ #include "sentry_os.h" #include "sentry_scope.h" -#include +#include #include -#include -#include - -// TODO discuss replacing this with sentry_level_t -// -> issue is 'warn' vs 'warning' -// https://develop.sentry.dev/sdk/data-model/event-payloads/#optional-attributes -// https://develop.sentry.dev/sdk/telemetry/logs/#log-severity-level -static char * -log_level_as_string(sentry_log_level_t level) +static const char * +level_as_string(sentry_level_t level) { switch (level) { - case SENTRY_LOG_LEVEL_TRACE: + case SENTRY_LEVEL_TRACE: return "trace"; - case SENTRY_LOG_LEVEL_DEBUG: + case SENTRY_LEVEL_DEBUG: return "debug"; - case SENTRY_LOG_LEVEL_INFO: - return "info"; - case SENTRY_LOG_LEVEL_WARN: + case SENTRY_LEVEL_WARNING: return "warn"; - case SENTRY_LOG_LEVEL_ERROR: + case SENTRY_LEVEL_ERROR: return "error"; - case SENTRY_LOG_LEVEL_FATAL: + case SENTRY_LEVEL_FATAL: return "fatal"; + case SENTRY_LEVEL_INFO: default: - return "UNKNOWN"; + return "unknown"; } } @@ -191,7 +183,7 @@ add_attribute(sentry_value_t attributes, sentry_value_t value, const char *type, } static sentry_value_t -construct_log(sentry_log_level_t level, const char *message, va_list args) +construct_log(sentry_level_t level, const char *message, va_list args) { sentry_value_t log = sentry_value_new_object(); sentry_value_t attributes = sentry_value_new_object(); @@ -216,7 +208,7 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) sentry_value_set_by_key(log, "body", sentry_value_new_string(fmt_message)); sentry_free(fmt_message); sentry_value_set_by_key( - log, "level", sentry_value_new_string(log_level_as_string(level))); + log, "level", sentry_value_new_string(level_as_string(level))); // timestamp in seconds uint64_t usec_time = sentry__usec_time(); @@ -310,7 +302,7 @@ construct_log(sentry_log_level_t level, const char *message, va_list args) } void -sentry__logs_log(sentry_log_level_t level, const char *message, va_list args) +sentry__logs_log(sentry_level_t level, const char *message, va_list args) { bool enable_logs = false; SENTRY_WITH_OPTIONS (options) { @@ -346,7 +338,7 @@ sentry_log_trace(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_TRACE, message, args); + sentry__logs_log(SENTRY_LEVEL_TRACE, message, args); va_end(args); } @@ -355,7 +347,7 @@ sentry_log_debug(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_DEBUG, message, args); + sentry__logs_log(SENTRY_LEVEL_DEBUG, message, args); va_end(args); } @@ -364,7 +356,7 @@ sentry_log_info(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_INFO, message, args); + sentry__logs_log(SENTRY_LEVEL_INFO, message, args); va_end(args); } @@ -373,7 +365,7 @@ sentry_log_warn(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_WARN, message, args); + sentry__logs_log(SENTRY_LEVEL_WARNING, message, args); va_end(args); } @@ -382,7 +374,7 @@ sentry_log_error(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_ERROR, message, args); + sentry__logs_log(SENTRY_LEVEL_ERROR, message, args); va_end(args); } @@ -391,6 +383,6 @@ sentry_log_fatal(const char *message, ...) { va_list args; va_start(args, message); - sentry__logs_log(SENTRY_LOG_LEVEL_FATAL, message, args); + sentry__logs_log(SENTRY_LEVEL_FATAL, message, args); va_end(args); } diff --git a/src/sentry_logs.h b/src/sentry_logs.h index b87fc9370..5fa6a4a77 100644 --- a/src/sentry_logs.h +++ b/src/sentry_logs.h @@ -3,20 +3,6 @@ #include "sentry_boot.h" -/** - * Sentry levels for events and breadcrumbs. - * TODO should these differ from sentry_level_e? (has no `trace` level) - */ -typedef enum sentry_log_level_e { - SENTRY_LOG_LEVEL_TRACE, - SENTRY_LOG_LEVEL_DEBUG, - SENTRY_LOG_LEVEL_INFO, - SENTRY_LOG_LEVEL_WARN, - SENTRY_LOG_LEVEL_ERROR, - SENTRY_LOG_LEVEL_FATAL -} sentry_log_level_t; - -void sentry__logs_log( - sentry_log_level_t level, const char *message, va_list args); +void sentry__logs_log(sentry_level_t level, const char *message, va_list args); #endif diff --git a/src/sentry_value.c b/src/sentry_value.c index 9e573fe6f..f47b5249b 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -106,6 +106,8 @@ static const char * level_as_string(sentry_level_t level) { switch (level) { + case SENTRY_LEVEL_TRACE: + return "trace"; case SENTRY_LEVEL_DEBUG: return "debug"; case SENTRY_LEVEL_WARNING: From 37ece60ae70e60a4d31b841a3b5449092ca828d2 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 16:43:11 +0200 Subject: [PATCH 16/27] add SENTRY_LEVEL_TRACE to sentry_logger --- src/sentry_logger.c | 2 ++ src/sentry_logger.h | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/sentry_logger.c b/src/sentry_logger.c index 98b965d7a..e8f701450 100644 --- a/src/sentry_logger.c +++ b/src/sentry_logger.c @@ -69,6 +69,8 @@ const char * sentry__logger_describe(sentry_level_t level) { switch (level) { + case SENTRY_LEVEL_TRACE: + return "TRACE "; case SENTRY_LEVEL_DEBUG: return "DEBUG "; case SENTRY_LEVEL_INFO: diff --git a/src/sentry_logger.h b/src/sentry_logger.h index af21b016d..14e4d0451 100644 --- a/src/sentry_logger.h +++ b/src/sentry_logger.h @@ -18,6 +18,11 @@ const char *sentry__logger_describe(sentry_level_t level); void sentry__logger_log(sentry_level_t level, const char *message, ...); +#define SENTRY_TRACEF(message, ...) \ + sentry__logger_log(SENTRY_LEVEL_TRACE, message, __VA_ARGS__) + +#define SENTRY_TRACE(message) sentry__logger_log(SENTRY_LEVEL_TRACE, message) + #define SENTRY_DEBUGF(message, ...) \ sentry__logger_log(SENTRY_LEVEL_DEBUG, message, __VA_ARGS__) From e75f60cf52b922a65521b92d3163e4b4ed97d629 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 24 Jun 2025 17:12:37 +0200 Subject: [PATCH 17/27] quick anti-brownout fix - see https://github.com/getsentry/sentry-native/pull/1274 --- .github/workflows/ci.yml | 2 +- src/sentry_core.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 74ae91cbf..a8b13fc9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,7 +121,7 @@ jobs: SYSTEM_VERSION_COMPAT: 0 RUN_ANALYZER: asan,llvm-cov - name: Windows (old VS, 32-bit) - os: windows-2019 + os: windows-2022 TEST_X86: 1 - name: Windows (latest) os: windows-latest diff --git a/src/sentry_core.c b/src/sentry_core.c index 24d7a94bf..0bcfd2472 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -125,6 +125,7 @@ sentry_init(sentry_options_t *options) sentry_close(); sentry_logger_t logger = { NULL, NULL, SENTRY_LEVEL_DEBUG }; + if (options->debug) { logger = options->logger; } From b37ea8d5d32cdb3298614f2974f9be6ddae09a3d Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Wed, 25 Jun 2025 13:12:37 +0200 Subject: [PATCH 18/27] fix missing SENTRY_LEVEL_INFO string return --- src/sentry_logs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index a323a7afd..18e0d154b 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -16,13 +16,14 @@ level_as_string(sentry_level_t level) return "trace"; case SENTRY_LEVEL_DEBUG: return "debug"; + case SENTRY_LEVEL_INFO: + return "info"; case SENTRY_LEVEL_WARNING: return "warn"; case SENTRY_LEVEL_ERROR: return "error"; case SENTRY_LEVEL_FATAL: return "fatal"; - case SENTRY_LEVEL_INFO: default: return "unknown"; } From 08e84721e314d7dc2ddabd0f44d2cdb6ac888591 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 26 Jun 2025 13:59:21 +0200 Subject: [PATCH 19/27] fix logger level check + add test --- src/sentry_logger.c | 3 +- tests/unit/test_logger.c | 66 ++++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 1 + 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/sentry_logger.c b/src/sentry_logger.c index e8f701450..642c9433c 100644 --- a/src/sentry_logger.c +++ b/src/sentry_logger.c @@ -89,8 +89,7 @@ sentry__logger_describe(sentry_level_t level) void sentry__logger_log(sentry_level_t level, const char *message, ...) { - if (g_logger.logger_level != SENTRY_LEVEL_DEBUG - && level < g_logger.logger_level) { + if (level < g_logger.logger_level) { return; } sentry_logger_t logger = g_logger; diff --git a/tests/unit/test_logger.c b/tests/unit/test_logger.c index 89e50553e..ff0a784a2 100644 --- a/tests/unit/test_logger.c +++ b/tests/unit/test_logger.c @@ -51,3 +51,69 @@ SENTRY_TEST(custom_logger) sentry_close(); } } + +static void +test_log_level( + sentry_level_t level, const char *message, va_list args, void *_data) +{ + (void)level; + (void)message; + (void)args; + + logger_test_t *data = _data; + if (data->assert_now) { + data->called += 1; + } +} + +SENTRY_TEST(logger_level) +{ + // Test structure: for each level, test that only messages >= that level are + // logged + const struct { + sentry_level_t level; + int expected_count; // How many of the 5 test messages should be logged + } test_cases[] = { + { SENTRY_LEVEL_TRACE, + 5 }, // All messages: TRACE, DEBUG, INFO, WARN, ERROR + { SENTRY_LEVEL_DEBUG, + 4 }, // DEBUG, INFO, WARN, ERROR (TRACE filtered out) + { SENTRY_LEVEL_INFO, 3 }, // INFO, WARN, ERROR + { SENTRY_LEVEL_WARNING, 2 }, // WARN, ERROR + { SENTRY_LEVEL_ERROR, 1 }, // ERROR only + }; + + for (size_t i = 0; i < 5; i++) { // for each of the 5 logger levels + logger_test_t data = { 0, false }; + + { + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_debug(options, true); + sentry_options_set_logger_level(options, test_cases[i].level); + sentry_options_set_logger(options, test_log_level, &data); + + sentry_init(options); + + data.assert_now = true; + // Test all 5 levels in order from most to least verbose + SENTRY_TRACE("Logging Trace"); // level -2 + SENTRY_DEBUG("Logging Debug"); // level -1 + SENTRY_INFO("Logging Info"); // level 0 + SENTRY_WARN("Logging Warning"); // level 1 + SENTRY_ERROR("Logging Error"); // level 2 + + data.assert_now = false; + + TEST_CHECK_INT_EQUAL(data.called, test_cases[i].expected_count); + + sentry_close(); + } + + { + // *really* clear the logger instance + SENTRY_TEST_OPTIONS_NEW(options); + sentry_init(options); + sentry_close(); + } + } +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 8fba36755..90ee30e8c 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -60,6 +60,7 @@ XX(invalid_dsn) XX(invalid_proxy) XX(iso_time) XX(lazy_attachments) +XX(logger_level) XX(logs_disabled_by_default) XX(message_with_null_text_is_valid) XX(module_addr) From 8a1fc26362823e321cdc8443e79913cb0f3f8c75 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 27 Jun 2025 11:29:33 +0200 Subject: [PATCH 20/27] cleanup logs parameter extraction --- examples/example.c | 12 +-- src/sentry_logs.c | 230 ++++++++++++++++++++++++++------------------- 2 files changed, 137 insertions(+), 105 deletions(-) diff --git a/examples/example.c b/examples/example.c index 9eb235e91..9f6602822 100644 --- a/examples/example.c +++ b/examples/example.c @@ -361,12 +361,12 @@ main(int argc, char **argv) // TODO incorporate into test if (sentry_options_get_enable_logs(options)) { - sentry_log_trace("We log it up %i percent, %s style\n", 100, "trace"); - sentry_log_debug("We log it up %i percent, %s style\n", 100, "debug"); - sentry_log_info("We log it up %i percent, %s style\n", 100, "info"); - sentry_log_warn("We log it up %i percent, %s style\n", 100, "warn"); - sentry_log_error("We log it up %i percent, %s style\n", 100, "error"); - sentry_log_fatal("We log it up %i percent, %s style\n", 100, "fatal"); + sentry_log_trace("We log it up %i%%, %s style", 100, "trace"); + sentry_log_debug("We log it up %i%%, %s style", 100, "debug"); + sentry_log_info("We log it up %i%%, %s style", 100, "info"); + sentry_log_warn("We log it up %i%%, %s style", 100, "warn"); + sentry_log_error("We log it up %i%%, %s style", 100, "error"); + sentry_log_fatal("We log it up %i%%, %s style", 100, "fatal"); // Test the logger with various parameter types sentry_log_info( diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 18e0d154b..999e140ad 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -29,6 +29,127 @@ level_as_string(sentry_level_t level) } } +sentry_value_t +construct_param_from_conversion(const char conversion, va_list *args_copy) +{ + sentry_value_t param_obj = sentry_value_new_object(); + switch (conversion) { + case 'd': + case 'i': { + int val = va_arg(*args_copy, int); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_int32(val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("integer")); + break; + } + case 'u': + case 'x': + case 'X': + case 'o': { + unsigned int val = va_arg(*args_copy, unsigned int); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_int32((int32_t)val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("integer")); + break; + } + case 'f': + case 'F': + case 'e': + case 'E': + case 'g': + case 'G': { + double val = va_arg(*args_copy, double); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_double(val)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("double")); + break; + } + case 'c': { + int val = va_arg(*args_copy, int); + char str[2] = { (char)val, '\0' }; + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(str)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + case 's': { + const char *val = va_arg(*args_copy, const char *); + if (val) { + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(val)); + } else { + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string("(null)")); + } + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + case 'p': { + void *val = va_arg(*args_copy, void *); + char ptr_str[32]; + snprintf(ptr_str, sizeof(ptr_str), "%p", val); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string(ptr_str)); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + default: + // Unknown format specifier, skip the argument + (void)va_arg(*args_copy, void *); + sentry_value_set_by_key( + param_obj, "value", sentry_value_new_string("(unknown)")); + sentry_value_set_by_key( + param_obj, "type", sentry_value_new_string("string")); + break; + } + + return param_obj; +} + +static char * +skip_flags(char *fmt_ptr) +{ + while (*fmt_ptr + && (*fmt_ptr == '-' || *fmt_ptr == '+' || *fmt_ptr == ' ' + || *fmt_ptr == '#' || *fmt_ptr == '0')) { + fmt_ptr++; + } + return fmt_ptr; +} + +static char * +skip_width(char *fmt_ptr) +{ + while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { + fmt_ptr++; + } + return fmt_ptr; +} + +static char * +skip_precision(char *fmt_ptr) +{ + + if (*fmt_ptr == '.') { + fmt_ptr++; + while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { + fmt_ptr++; + } + } + return fmt_ptr; +} + +static char * +extract_length() +{ +} + static void populate_message_parameters( sentry_value_t attributes, const char *message, va_list args) @@ -37,7 +158,7 @@ populate_message_parameters( return; } - const char *fmt_ptr = message; + char *fmt_ptr = message; int param_index = 0; va_list args_copy; va_copy(args_copy, args); @@ -47,22 +168,16 @@ populate_message_parameters( if (*fmt_ptr == '%') { fmt_ptr++; // Skip the '%' - // Skip flags, width, and precision - while (*fmt_ptr - && (*fmt_ptr == '-' || *fmt_ptr == '+' || *fmt_ptr == ' ' - || *fmt_ptr == '#' || *fmt_ptr == '0')) { - fmt_ptr++; - } - while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { - fmt_ptr++; - } - if (*fmt_ptr == '.') { + if (*fmt_ptr == '%') { + // Escaped '%', not a format specifier fmt_ptr++; - while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { - fmt_ptr++; - } + continue; } + fmt_ptr = skip_flags(fmt_ptr); + fmt_ptr = skip_width(fmt_ptr); + fmt_ptr = skip_precision(fmt_ptr); + // Skip length modifiers while (*fmt_ptr && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L' @@ -70,97 +185,14 @@ populate_message_parameters( fmt_ptr++; } - if (*fmt_ptr == '%') { - // Escaped '%', not a format specifier - fmt_ptr++; - continue; - } - // Get the conversion specifier char conversion = *fmt_ptr; if (conversion) { char key[64]; snprintf(key, sizeof(key), "sentry.message.parameter.%d", param_index); - - sentry_value_t param_obj = sentry_value_new_object(); - - switch (conversion) { - case 'd': - case 'i': { - int val = va_arg(args_copy, int); - sentry_value_set_by_key( - param_obj, "value", sentry_value_new_int32(val)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("integer")); - break; - } - case 'u': - case 'x': - case 'X': - case 'o': { - unsigned int val = va_arg(args_copy, unsigned int); - sentry_value_set_by_key(param_obj, "value", - sentry_value_new_int32((int32_t)val)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("integer")); - break; - } - case 'f': - case 'F': - case 'e': - case 'E': - case 'g': - case 'G': { - double val = va_arg(args_copy, double); - sentry_value_set_by_key( - param_obj, "value", sentry_value_new_double(val)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("double")); - break; - } - case 'c': { - int val = va_arg(args_copy, int); - char str[2] = { (char)val, '\0' }; - sentry_value_set_by_key( - param_obj, "value", sentry_value_new_string(str)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("string")); - break; - } - case 's': { - const char *val = va_arg(args_copy, const char *); - if (val) { - sentry_value_set_by_key( - param_obj, "value", sentry_value_new_string(val)); - } else { - sentry_value_set_by_key(param_obj, "value", - sentry_value_new_string("(null)")); - } - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("string")); - break; - } - case 'p': { - void *val = va_arg(args_copy, void *); - char ptr_str[32]; - snprintf(ptr_str, sizeof(ptr_str), "%p", val); - sentry_value_set_by_key( - param_obj, "value", sentry_value_new_string(ptr_str)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("string")); - break; - } - default: - // Unknown format specifier, skip the argument - (void)va_arg(args_copy, void *); - sentry_value_set_by_key(param_obj, "value", - sentry_value_new_string("(unknown)")); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("string")); - break; - } - + sentry_value_t param_obj + = construct_param_from_conversion(conversion, &args_copy); sentry_value_set_by_key(attributes, key, param_obj); param_index++; fmt_ptr++; From 17c3a96fb7d1fc85f2b95f896bfeea9c5c4e5991 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:23:44 +0200 Subject: [PATCH 21/27] warn-as-error fix --- src/sentry_logs.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 999e140ad..9074648f9 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -145,11 +145,6 @@ skip_precision(char *fmt_ptr) return fmt_ptr; } -static char * -extract_length() -{ -} - static void populate_message_parameters( sentry_value_t attributes, const char *message, va_list args) @@ -158,7 +153,7 @@ populate_message_parameters( return; } - char *fmt_ptr = message; + const char *fmt_ptr = message; int param_index = 0; va_list args_copy; va_copy(args_copy, args); From 044e0bf68e17626f948546f6799c288ca8bf0ed0 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:33:12 +0200 Subject: [PATCH 22/27] const char* fix --- src/sentry_logs.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 9074648f9..efc6d5a83 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -112,8 +112,8 @@ construct_param_from_conversion(const char conversion, va_list *args_copy) return param_obj; } -static char * -skip_flags(char *fmt_ptr) +static const char * +skip_flags(const char *fmt_ptr) { while (*fmt_ptr && (*fmt_ptr == '-' || *fmt_ptr == '+' || *fmt_ptr == ' ' @@ -123,8 +123,8 @@ skip_flags(char *fmt_ptr) return fmt_ptr; } -static char * -skip_width(char *fmt_ptr) +static const char * +skip_width(const char *fmt_ptr) { while (*fmt_ptr && (*fmt_ptr >= '0' && *fmt_ptr <= '9')) { fmt_ptr++; @@ -132,8 +132,8 @@ skip_width(char *fmt_ptr) return fmt_ptr; } -static char * -skip_precision(char *fmt_ptr) +static const char * +skip_precision(const char *fmt_ptr) { if (*fmt_ptr == '.') { From 0dcc172509a362349296d905a5fe96baf7b8169a Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:59:08 +0200 Subject: [PATCH 23/27] static function --- src/sentry_logs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index efc6d5a83..7fe078053 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -29,7 +29,7 @@ level_as_string(sentry_level_t level) } } -sentry_value_t +static sentry_value_t construct_param_from_conversion(const char conversion, va_list *args_copy) { sentry_value_t param_obj = sentry_value_new_object(); From 940bd6a4af2788c2ff97c50dacdf83e816bcacf1 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 14 Jul 2025 14:49:16 +0200 Subject: [PATCH 24/27] feat(logs): add (u)int64 sentry_value_t type (#1301) * add (u)int64 sentry_value_t type * add value_to_msgpack missing switch cases * remove undefined behavior test (C99 6.3.1.4) * avoid Windows sized integer name collision * cleanup & apply code review feedback * more cleanup & remove type coercion --- include/sentry.h | 22 ++++++ src/sentry_json.c | 20 ++++++ src/sentry_json.h | 10 +++ src/sentry_value.c | 155 +++++++++++++++++++++++++++++++++++----- tests/unit/test_value.c | 110 +++++++++++++++++++++++++++- tests/unit/tests.inc | 2 + 6 files changed, 301 insertions(+), 18 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 482e14751..0a9ccb62d 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -137,6 +137,8 @@ typedef enum { SENTRY_VALUE_TYPE_NULL, SENTRY_VALUE_TYPE_BOOL, SENTRY_VALUE_TYPE_INT32, + SENTRY_VALUE_TYPE_INT64, + SENTRY_VALUE_TYPE_UINT64, SENTRY_VALUE_TYPE_DOUBLE, SENTRY_VALUE_TYPE_STRING, SENTRY_VALUE_TYPE_LIST, @@ -200,6 +202,16 @@ SENTRY_API sentry_value_t sentry_value_new_null(void); */ SENTRY_API sentry_value_t sentry_value_new_int32(int32_t value); +/** + * Creates a new 64-bit signed integer value. + */ +SENTRY_API sentry_value_t sentry_value_new_int64(int64_t value); + +/** + * Creates a new 64-bit unsigned integer value. + */ +SENTRY_API sentry_value_t sentry_value_new_uint64(uint64_t value); + /** * Creates a new double value. */ @@ -338,6 +350,16 @@ SENTRY_API size_t sentry_value_get_length(sentry_value_t value); */ SENTRY_API int32_t sentry_value_as_int32(sentry_value_t value); +/** + * Converts a value into a 64 bit signed integer. + */ +SENTRY_API int64_t sentry_value_as_int64(sentry_value_t value); + +/** + * Converts a value into a 64 bit unsigned integer. + */ +SENTRY_API uint64_t sentry_value_as_uint64(sentry_value_t value); + /** * Converts a value into a double value. */ diff --git a/src/sentry_json.c b/src/sentry_json.c index 8902406f7..ea7454f81 100644 --- a/src/sentry_json.c +++ b/src/sentry_json.c @@ -364,6 +364,26 @@ sentry__jsonwriter_write_int32(sentry_jsonwriter_t *jw, int32_t val) } } +void +sentry__jsonwriter_write_int64(sentry_jsonwriter_t *jw, int64_t val) +{ + if (can_write_item(jw)) { + char buf[24]; + snprintf(buf, sizeof(buf), "%" PRId64, val); + write_str(jw, buf); + } +} + +void +sentry__jsonwriter_write_uint64(sentry_jsonwriter_t *jw, uint64_t val) +{ + if (can_write_item(jw)) { + char buf[24]; + snprintf(buf, sizeof(buf), "%" PRIu64, val); + write_str(jw, buf); + } +} + void sentry__jsonwriter_write_double(sentry_jsonwriter_t *jw, double val) { diff --git a/src/sentry_json.h b/src/sentry_json.h index 1467547c0..20f302518 100644 --- a/src/sentry_json.h +++ b/src/sentry_json.h @@ -53,6 +53,16 @@ void sentry__jsonwriter_write_bool(sentry_jsonwriter_t *jw, bool val); */ void sentry__jsonwriter_write_int32(sentry_jsonwriter_t *jw, int32_t val); +/** + * Write a 64-bit signed integer, encoded as JSON number. + */ +void sentry__jsonwriter_write_int64(sentry_jsonwriter_t *jw, int64_t val); + +/** + * Write a 64-bit unsigned integer, encoded as JSON number. + */ +void sentry__jsonwriter_write_uint64(sentry_jsonwriter_t *jw, uint64_t val); + /** * Write a 64-bit float, encoded as JSON number. */ diff --git a/src/sentry_value.c b/src/sentry_value.c index f47b5249b..dcec55f3a 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -73,6 +73,8 @@ #define THING_TYPE_OBJECT 1 #define THING_TYPE_STRING 2 #define THING_TYPE_DOUBLE 3 +#define THING_TYPE_INT64 4 +#define THING_TYPE_UINT64 5 /* internal value helpers */ @@ -80,6 +82,8 @@ typedef struct { union { void *_ptr; double _double; + int64_t _i64; + uint64_t _u64; } payload; long refcount; uint8_t type; @@ -326,6 +330,38 @@ sentry_value_new_double(double value) return rv; } +sentry_value_t +sentry_value_new_int64(int64_t value) +{ + thing_t *thing = sentry_malloc(sizeof(thing_t)); + if (!thing) { + return sentry_value_new_null(); + } + thing->payload._i64 = value; + thing->refcount = 1; + thing->type = (uint8_t)(THING_TYPE_INT64 | THING_TYPE_FROZEN); + + sentry_value_t rv; + rv._bits = (uint64_t)(size_t)thing; + return rv; +} + +sentry_value_t +sentry_value_new_uint64(uint64_t value) +{ + thing_t *thing = sentry_malloc(sizeof(thing_t)); + if (!thing) { + return sentry_value_new_null(); + } + thing->payload._u64 = value; + thing->refcount = 1; + thing->type = (uint8_t)(THING_TYPE_UINT64 | THING_TYPE_FROZEN); + + sentry_value_t rv; + rv._bits = (uint64_t)(size_t)thing; + return rv; +} + sentry_value_t sentry_value_new_bool(int value) { @@ -490,6 +526,10 @@ sentry_value_get_type(sentry_value_t value) return SENTRY_VALUE_TYPE_OBJECT; case THING_TYPE_DOUBLE: return SENTRY_VALUE_TYPE_DOUBLE; + case THING_TYPE_INT64: + return SENTRY_VALUE_TYPE_INT64; + case THING_TYPE_UINT64: + return SENTRY_VALUE_TYPE_UINT64; } UNREACHABLE("invalid thing type"); } else if ((value._bits & TAG_MASK) == TAG_CONST) { @@ -626,6 +666,18 @@ sentry__value_as_uuid(sentry_value_t value) char * sentry__value_stringify(sentry_value_t value) { +#define STRINGIFY_NUMERIC(fmt, value_fn) \ + do { \ + char buf[24]; \ + size_t written \ + = (size_t)sentry__snprintf_c(buf, sizeof(buf), fmt, value_fn); \ + if (written >= sizeof(buf)) { \ + return sentry__string_clone(""); \ + } \ + buf[written] = '\0'; \ + return sentry__string_clone(buf); \ + } while (0) + switch (sentry_value_get_type(value)) { case SENTRY_VALUE_TYPE_LIST: case SENTRY_VALUE_TYPE_OBJECT: @@ -636,19 +688,17 @@ sentry__value_stringify(sentry_value_t value) sentry_value_is_true(value) ? "true" : "false"); case SENTRY_VALUE_TYPE_STRING: return sentry__string_clone(sentry_value_as_string(value)); + case SENTRY_VALUE_TYPE_INT64: + STRINGIFY_NUMERIC("%" PRIi64, sentry_value_as_int64(value)); + case SENTRY_VALUE_TYPE_UINT64: + STRINGIFY_NUMERIC("%" PRIi64, sentry_value_as_uint64(value)); case SENTRY_VALUE_TYPE_INT32: case SENTRY_VALUE_TYPE_DOUBLE: - default: { - char buf[24]; - size_t written = (size_t)sentry__snprintf_c( - buf, sizeof(buf), "%g", sentry_value_as_double(value)); - if (written >= sizeof(buf)) { - return sentry__string_clone(""); - } - buf[written] = '\0'; - return sentry__string_clone(buf); - } + default: + STRINGIFY_NUMERIC("%g", sentry_value_as_double(value)); } + +#undef STRINGIFY_NUMERIC } sentry_value_t @@ -679,6 +729,8 @@ sentry__value_clone(sentry_value_t value) } case THING_TYPE_STRING: case THING_TYPE_DOUBLE: + case THING_TYPE_INT64: + case THING_TYPE_UINT64: sentry_value_incref(value); return value; default: @@ -873,9 +925,18 @@ sentry_value_as_int32(sentry_value_t value) { if ((value._bits & TAG_MASK) == TAG_INT32) { return (int32_t)((int64_t)value._bits >> 32); - } else { - return 0; } + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + SENTRY_WARN("Cannot convert int64 into int32, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + SENTRY_WARN("Cannot convert uint64 into int32, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + SENTRY_WARN("Cannot convert double into int32, returning 0"); + } + return 0; } double @@ -888,9 +949,56 @@ sentry_value_as_double(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { return thing->payload._double; - } else { - return (double)NAN; } + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + SENTRY_WARN("Cannot convert int64 into double, returning NAN"); + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + SENTRY_WARN("Cannot convert uint64 into double, returning NAN"); + } + + return (double)NAN; +} + +int64_t +sentry_value_as_int64(sentry_value_t value) +{ + if ((value._bits & TAG_MASK) == TAG_INT32) { + return (int64_t)sentry_value_as_int32(value); + } + + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + return thing->payload._i64; + } + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + SENTRY_WARN("Cannot convert uint64 into int64, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + SENTRY_WARN("Cannot convert double into int64, returning 0"); + } + return 0; +} + +uint64_t +sentry_value_as_uint64(sentry_value_t value) +{ + if ((value._bits & TAG_MASK) == TAG_INT32) { + SENTRY_WARN("Cannot convert int32 into uint64, returning 0"); + return 0; + } + + const thing_t *thing = value_as_thing(value); + if (thing && thing_get_type(thing) == THING_TYPE_UINT64) { + return thing->payload._u64; + } + if (thing && thing_get_type(thing) == THING_TYPE_INT64) { + SENTRY_WARN("Cannot convert int64 into uint64, returning 0"); + } + if (thing && thing_get_type(thing) == THING_TYPE_DOUBLE) { + SENTRY_WARN("Cannot convert int64 into uint64, returning 0"); + } + return 0; } const char * @@ -899,9 +1007,8 @@ sentry_value_as_string(sentry_value_t value) const thing_t *thing = value_as_thing(value); if (thing && thing_get_type(thing) == THING_TYPE_STRING) { return (const char *)thing->payload._ptr; - } else { - return ""; } + return ""; } sentry_value_t @@ -938,6 +1045,10 @@ sentry_value_is_true(sentry_value_t value) return 0; case SENTRY_VALUE_TYPE_INT32: return sentry_value_as_int32(value) != 0; + case SENTRY_VALUE_TYPE_INT64: + return sentry_value_as_int64(value) != 0; + case SENTRY_VALUE_TYPE_UINT64: + return sentry_value_as_uint64(value) != 0; case SENTRY_VALUE_TYPE_DOUBLE: return sentry_value_as_double(value) != 0.0; case SENTRY_VALUE_TYPE_STRING: @@ -1002,6 +1113,12 @@ sentry__jsonwriter_write_value(sentry_jsonwriter_t *jw, sentry_value_t value) case SENTRY_VALUE_TYPE_INT32: sentry__jsonwriter_write_int32(jw, sentry_value_as_int32(value)); break; + case SENTRY_VALUE_TYPE_INT64: + sentry__jsonwriter_write_int64(jw, sentry_value_as_int64(value)); + break; + case SENTRY_VALUE_TYPE_UINT64: + sentry__jsonwriter_write_uint64(jw, sentry_value_as_uint64(value)); + break; case SENTRY_VALUE_TYPE_DOUBLE: sentry__jsonwriter_write_double(jw, sentry_value_as_double(value)); break; @@ -1054,6 +1171,12 @@ value_to_msgpack(mpack_writer_t *writer, sentry_value_t value) case SENTRY_VALUE_TYPE_INT32: mpack_write_i32(writer, sentry_value_as_int32(value)); break; + case SENTRY_VALUE_TYPE_INT64: + mpack_write_i64(writer, sentry_value_as_int64(value)); + break; + case SENTRY_VALUE_TYPE_UINT64: + mpack_write_u64(writer, sentry_value_as_uint64(value)); + break; case SENTRY_VALUE_TYPE_DOUBLE: mpack_write_double(writer, sentry_value_as_double(value)); break; diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 40d0a2ae5..18457c93b 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -71,6 +71,112 @@ SENTRY_TEST(value_int32) TEST_CHECK(sentry_value_refcount(val) == 1); } +SENTRY_TEST(value_int64) +{ + sentry_value_t val = sentry_value_new_int64(42LL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == 42LL); + // We don't convert int64 to double + TEST_CHECK(isnan(sentry_value_as_double(val))); + // We don't convert int64 to int32 + TEST_CHECK(sentry_value_as_int32(val) == 0); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "42"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test large positive value + val = sentry_value_new_int64(INT64_MAX); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == INT64_MAX); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "9223372036854775807"); + sentry_value_decref(val); + + // Test large negative value + val = sentry_value_new_int64(INT64_MIN); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == INT64_MIN); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "-9223372036854775808"); + sentry_value_decref(val); + + // Test zero + val = sentry_value_new_int64(0LL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_INT64); + TEST_CHECK(sentry_value_as_int64(val) == 0LL); + TEST_CHECK(!sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "0"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // We do convert int32 to int64 + val = sentry_value_new_int32(42); + TEST_CHECK(sentry_value_as_int64(val) == 42LL); + sentry_value_decref(val); + + // We don't convert uint64 to int64 + val = sentry_value_new_uint64(-42LL); + TEST_CHECK(sentry_value_as_int64(val) == 0); + sentry_value_decref(val); + + // We don't convert double to int64 + val = sentry_value_new_double(42.99); + TEST_CHECK(sentry_value_as_int64(val) == 0); + sentry_value_decref(val); +} + +SENTRY_TEST(value_uint64) +{ + sentry_value_t val = sentry_value_new_uint64(42ULL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == 42ULL); + // We don't convert uint64 to double + TEST_CHECK(isnan(sentry_value_as_double(val))); + // We don't convert uint64 to int32 + TEST_CHECK(sentry_value_as_int32(val) == 0); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "42"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // Test large positive value + val = sentry_value_new_uint64(UINT64_MAX); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == UINT64_MAX); + TEST_CHECK(sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "18446744073709551615"); + sentry_value_decref(val); + + // Test zero + val = sentry_value_new_uint64(0ULL); + TEST_CHECK(sentry_value_get_type(val) == SENTRY_VALUE_TYPE_UINT64); + TEST_CHECK(sentry_value_as_uint64(val) == 0ULL); + TEST_CHECK(!sentry_value_is_true(val)); + TEST_CHECK_JSON_VALUE(val, "0"); + TEST_CHECK(sentry_value_refcount(val) == 1); + TEST_CHECK(sentry_value_is_frozen(val)); + sentry_value_decref(val); + + // We don't convert int32 to uint64 + val = sentry_value_new_int32(42); + TEST_CHECK(sentry_value_as_uint64(val) == 0); + sentry_value_decref(val); + + // We don't convert double to uint64 + val = sentry_value_new_double(123.456); + TEST_CHECK(sentry_value_as_uint64(val) == 0); + sentry_value_decref(val); + + // We don't convert int64 to uint64 + val = sentry_value_new_int64(42LL); + TEST_CHECK(sentry_value_as_uint64(val) == 0); + sentry_value_decref(val); +} + SENTRY_TEST(value_double) { sentry_value_t val = sentry_value_new_double(42.05); @@ -452,7 +558,7 @@ SENTRY_TEST(value_json_parsing) "\"surrogates\":\"𐐷\"}"); sentry_value_decref(rv); - // unmatched surrogates don’t parse + // unmatched surrogates don't parse rv = sentry__value_from_json(STRING("\"\\uD801\"")); TEST_CHECK(sentry_value_is_null(rv)); rv = sentry__value_from_json( @@ -521,7 +627,7 @@ SENTRY_TEST(value_json_surrogates) TEST_CHECK_JSON_VALUE(rv, "{\"surrogates\":\"oh 𐐷 hi\"}"); sentry_value_decref(rv); - // unmatched surrogates don’t parse + // unmatched surrogates don't parse rv = sentry__value_from_json(STRING("\"\\uD801\"")); TEST_CHECK(sentry_value_is_null(rv)); rv = sentry__value_from_json( diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 90ee30e8c..c836832b3 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -149,6 +149,7 @@ XX(value_double) XX(value_freezing) XX(value_get_by_null_key) XX(value_int32) +XX(value_int64) XX(value_json_deeply_nested) XX(value_json_escaping) XX(value_json_invalid_doubles) @@ -166,6 +167,7 @@ XX(value_set_by_null_key) XX(value_set_stacktrace) XX(value_string) XX(value_string_n) +XX(value_uint64) XX(value_unicode) XX(value_user) XX(value_wrong_type) From 4e7d7949d1dd9f34be671c3c0814053b6fb641f1 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Mon, 14 Jul 2025 18:08:19 +0200 Subject: [PATCH 25/27] update logs param conversion --- src/sentry_logs.c | 8 ++++---- src/sentry_string.h | 11 +++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 7fe078053..786f67e2d 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -38,7 +38,7 @@ construct_param_from_conversion(const char conversion, va_list *args_copy) case 'i': { int val = va_arg(*args_copy, int); sentry_value_set_by_key( - param_obj, "value", sentry_value_new_int32(val)); + param_obj, "value", sentry_value_new_int64(val)); sentry_value_set_by_key( param_obj, "type", sentry_value_new_string("integer")); break; @@ -48,10 +48,10 @@ construct_param_from_conversion(const char conversion, va_list *args_copy) case 'X': case 'o': { unsigned int val = va_arg(*args_copy, unsigned int); + sentry_value_set_by_key(param_obj, "value", + sentry_value_new_string(sentry__uint64_to_string(val))); sentry_value_set_by_key( - param_obj, "value", sentry_value_new_int32((int32_t)val)); - sentry_value_set_by_key( - param_obj, "type", sentry_value_new_string("integer")); + param_obj, "type", sentry_value_new_string("string")); break; } case 'f': diff --git a/src/sentry_string.h b/src/sentry_string.h index 6b98c5766..48fa86f97 100644 --- a/src/sentry_string.h +++ b/src/sentry_string.h @@ -193,6 +193,17 @@ sentry__int64_to_string(int64_t val) return sentry__string_clone(buf); } +/** + * Converts an uint64_t into a string. + */ +static inline char * +sentry__uint64_to_string(uint64_t val) +{ + char buf[24]; + snprintf(buf, sizeof(buf), "%" PRIu64, val); + return sentry__string_clone(buf); +} + #ifdef SENTRY_PLATFORM_WINDOWS /** * Create a utf-8 string from a Wide String. From 6da518415de34af60cf9aa8ccb8c6a1873074c64 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 15 Jul 2025 11:21:16 +0200 Subject: [PATCH 26/27] own uint64 string --- src/sentry_logs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 786f67e2d..19589a486 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -49,7 +49,7 @@ construct_param_from_conversion(const char conversion, va_list *args_copy) case 'o': { unsigned int val = va_arg(*args_copy, unsigned int); sentry_value_set_by_key(param_obj, "value", - sentry_value_new_string(sentry__uint64_to_string(val))); + sentry__value_new_string_owned(sentry__uint64_to_string(val))); sentry_value_set_by_key( param_obj, "type", sentry_value_new_string("string")); break; From e6ea228bfe86a9f1beedd67787d460f2c5f659a7 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Tue, 15 Jul 2025 15:14:30 +0200 Subject: [PATCH 27/27] apply suggestions from code review --- src/CMakeLists.txt | 4 ++-- src/sentry_logs.c | 19 ++++++++++++------- src/sentry_value.c | 9 ++++++--- tests/unit/CMakeLists.txt | 2 +- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1eddcf4b3..171da4ab1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -15,6 +15,8 @@ sentry_target_sources_cwd(sentry sentry_json.h sentry_logger.c sentry_logger.h + sentry_logs.c + sentry_logs.h sentry_options.c sentry_options.h sentry_os.c @@ -51,8 +53,6 @@ sentry_target_sources_cwd(sentry transports/sentry_disk_transport.h transports/sentry_function_transport.c unwinder/sentry_unwinder.c - sentry_logs.c - sentry_logs.h ) # generic platform / path / symbolizer diff --git a/src/sentry_logs.c b/src/sentry_logs.c index 19589a486..9f87e3bd9 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -145,6 +145,17 @@ skip_precision(const char *fmt_ptr) return fmt_ptr; } +static const char * +skip_length(const char *fmt_ptr) +{ + while (*fmt_ptr + && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L' + || *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) { + fmt_ptr++; + } + return fmt_ptr; +} + static void populate_message_parameters( sentry_value_t attributes, const char *message, va_list args) @@ -172,13 +183,7 @@ populate_message_parameters( fmt_ptr = skip_flags(fmt_ptr); fmt_ptr = skip_width(fmt_ptr); fmt_ptr = skip_precision(fmt_ptr); - - // Skip length modifiers - while (*fmt_ptr - && (*fmt_ptr == 'h' || *fmt_ptr == 'l' || *fmt_ptr == 'L' - || *fmt_ptr == 'z' || *fmt_ptr == 'j' || *fmt_ptr == 't')) { - fmt_ptr++; - } + fmt_ptr = skip_length(fmt_ptr); // Get the conversion specifier char conversion = *fmt_ptr; diff --git a/src/sentry_value.c b/src/sentry_value.c index dcec55f3a..d353573d0 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -317,7 +317,7 @@ sentry_value_new_int32(int32_t value) sentry_value_t sentry_value_new_double(double value) { - thing_t *thing = sentry_malloc(sizeof(thing_t)); + thing_t *thing = SENTRY_MAKE(thing_t); if (!thing) { return sentry_value_new_null(); } @@ -333,7 +333,7 @@ sentry_value_new_double(double value) sentry_value_t sentry_value_new_int64(int64_t value) { - thing_t *thing = sentry_malloc(sizeof(thing_t)); + thing_t *thing = SENTRY_MAKE(thing_t); if (!thing) { return sentry_value_new_null(); } @@ -349,7 +349,7 @@ sentry_value_new_int64(int64_t value) sentry_value_t sentry_value_new_uint64(uint64_t value) { - thing_t *thing = sentry_malloc(sizeof(thing_t)); + thing_t *thing = SENTRY_MAKE(thing_t); if (!thing) { return sentry_value_new_null(); } @@ -666,6 +666,9 @@ sentry__value_as_uuid(sentry_value_t value) char * sentry__value_stringify(sentry_value_t value) { + // returns empty string if snprintf fails + // (returning -1, so casting this to size_t it becomes > the buffer size) + // or if the value is too large for the buffer #define STRINGIFY_NUMERIC(fmt, value_fn) \ do { \ char buf[24]; \ diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 877d6b7a3..1c17257b5 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -28,7 +28,7 @@ add_executable(sentry_test_unit test_fuzzfailures.c test_info.c test_logger.c - test_logs.c + test_logs.c test_modulefinder.c test_mpack.c test_options.c