Skip to content

Avoid conditional compilation on whether tracing is enabled #914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/libmongoc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,7 @@ set (MONGOC_CC ${CMAKE_C_COMPILER})
set (MONGOC_USER_SET_CFLAGS ${CMAKE_C_FLAGS})
set (MONGOC_USER_SET_LDFLAGS ${CMAKE_EXE_LINKER_FLAGS})

set (MONGOC_TRACE 0)

if (ENABLE_TRACING)
set (MONGOC_TRACE 1)
endif ()
set (MONGOC_TRACE "${ENABLE_TRACING}")

# Sets SNAPPY_LIBRARIES and SNAPPY_INCLUDE_DIRS.
include (FindSnappy)
Expand Down
37 changes: 23 additions & 14 deletions src/libmongoc/src/mongoc/mongoc-config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@
#ifndef MONGOC_CONFIG_H
#define MONGOC_CONFIG_H

/* clang-format off */

/*
* NOTICE:
* If you're about to update this file and add a config flag, make sure to
* update:
* o The bitfield in mongoc-handshake-private.h
* o _mongoc_handshake_get_config_hex_string() in mongoc-handshake.c
* o examples/parse_handshake_cfg.py
* o test_handshake_config_string in test-mongoc-handshake.c
*/

/* MONGOC_USER_SET_CFLAGS is set from config based on what compiler flags were
* used to compile mongoc */
#define MONGOC_USER_SET_CFLAGS "@MONGOC_USER_SET_CFLAGS@"
Expand Down Expand Up @@ -353,13 +365,18 @@
/*
* Set if tracing is enabled. Logs things like network communication and
* entry/exit of certain functions.
*
*/
#define MONGOC_TRACE @MONGOC_TRACE@
#cmakedefine01 MONGOC_TRACE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unexpectedly broke PHPC's build system (autotools). I've reported CDRIVER-4247 to request a more portable approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in #917.


#if MONGOC_TRACE != 1
# undef MONGOC_TRACE
#endif
enum {
/**
* @brief Compile-time constant determining whether the mongoc library was
* compiled with tracing enabled.
*
* Can be controlled with the 'ENABLE_TRACING" configure-time boolean option
*/
MONGOC_TRACE_ENABLED = MONGOC_TRACE
};

/*
* Set if we have ICU support.
Expand Down Expand Up @@ -401,14 +418,6 @@
# undef MONGOC_ENABLE_MONGODB_AWS_AUTH
#endif

/*
* NOTICE:
* If you're about to update this file and add a config flag, make sure to
* update:
* o The bitfield in mongoc-handshake-private.h
* o _mongoc_handshake_get_config_hex_string() in mongoc-handshake.c
* o examples/parse_handshake_cfg.py
* o test_handshake_config_string in test-mongoc-handshake.c
*/
/* clang-format on */

#endif /* MONGOC_CONFIG_H */
6 changes: 3 additions & 3 deletions src/libmongoc/src/mongoc/mongoc-handshake.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,9 @@ _mongoc_handshake_get_config_hex_string (void)
_set_bit (bf, byte_count, MONGOC_MD_FLAG_ENABLE_SHM_COUNTERS);
#endif

#ifdef MONGOC_TRACE
_set_bit (bf, byte_count, MONGOC_MD_FLAG_TRACE);
#endif
if (MONGOC_TRACE_ENABLED) {
_set_bit (bf, byte_count, MONGOC_MD_FLAG_TRACE);
}

#ifdef MONGOC_ENABLE_ICU
_set_bit (bf, byte_count, MONGOC_MD_FLAG_ENABLE_ICU);
Expand Down
64 changes: 25 additions & 39 deletions src/libmongoc/src/mongoc/mongoc-log.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@
static bson_once_t once = BSON_ONCE_INIT;
static bson_mutex_t gLogMutex;
static mongoc_log_func_t gLogFunc = mongoc_log_default_handler;
#ifdef MONGOC_TRACE
static bool gLogTrace = true;
#endif
static bool gLogTrace = MONGOC_TRACE_ENABLED;
static void *gLogData;

static BSON_ONCE_FUN (_mongoc_ensure_mutex_once)
Expand All @@ -62,6 +60,26 @@ mongoc_log_set_handler (mongoc_log_func_t log_func, void *user_data)
bson_mutex_unlock (&gLogMutex);
}

bool
_mongoc_log_trace_is_enabled (void)
{
return gLogTrace && MONGOC_TRACE_ENABLED;
}

void
mongoc_log_trace_enable (void)
{
/* Enable trace logging if-and-only-if tracing is enabled at configure-time,
* otherwise tracing remains disabled.
*/
gLogTrace = MONGOC_TRACE_ENABLED;
}

void
mongoc_log_trace_disable (void)
{
gLogTrace = false;
}

/* just for testing */
void
Expand All @@ -85,10 +103,8 @@ mongoc_log (mongoc_log_level_t log_level,
bson_once (&once, &_mongoc_ensure_mutex_once);

stop_logging = !gLogFunc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check be combined into _mongoc_log_trace_is_enabled()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure. I left the logic as close to the original as possible.

#ifdef MONGOC_TRACE
stop_logging =
stop_logging || (log_level == MONGOC_LOG_LEVEL_TRACE && !gLogTrace);
#endif
stop_logging = stop_logging || (log_level == MONGOC_LOG_LEVEL_TRACE &&
!_mongoc_log_trace_is_enabled ());
if (stop_logging) {
return;
}
Expand Down Expand Up @@ -203,44 +219,16 @@ mongoc_log_default_handler (mongoc_log_level_t log_level,
message);
}

bool
_mongoc_log_trace_is_enabled (void)
{
#ifdef MONGOC_TRACE
return gLogTrace;
#else
return false;
#endif
}

void
mongoc_log_trace_enable (void)
{
#ifdef MONGOC_TRACE
gLogTrace = true;
#endif
}

void
mongoc_log_trace_disable (void)
{
#ifdef MONGOC_TRACE
gLogTrace = false;
#endif
}

void
mongoc_log_trace_bytes (const char *domain, const uint8_t *_b, size_t _l)
{
bson_string_t *str, *astr;
int32_t _i;
uint8_t _v;

#ifdef MONGOC_TRACE
if (!gLogTrace) {
if (!_mongoc_log_trace_is_enabled ()) {
return;
}
#endif

str = bson_string_new (NULL);
astr = bson_string_new (NULL);
Expand Down Expand Up @@ -291,11 +279,9 @@ mongoc_log_trace_iovec (const char *domain,
size_t _l = 0;
uint8_t _v;

#ifdef MONGOC_TRACE
if (!gLogTrace) {
if (!_mongoc_log_trace_is_enabled ()) {
return;
}
#endif

for (_i = 0; _i < _iovcnt; _i++) {
_l += _iov[_i].iov_len;
Expand Down
12 changes: 6 additions & 6 deletions src/libmongoc/src/mongoc/mongoc-server-monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ static BSON_GNUC_PRINTF (3, 4) void _server_monitor_log (
bson_free (msg);
}

#ifdef MONGOC_TRACE
#define MONITOR_LOG(sm, ...) \
_server_monitor_log (sm, MONGOC_LOG_LEVEL_TRACE, __VA_ARGS__)
#else
#define MONITOR_LOG(sm, ...) (void) 0
#endif
#define MONITOR_LOG(sm, ...) \
do { \
if (MONGOC_TRACE_ENABLED) { \
_server_monitor_log (sm, MONGOC_LOG_LEVEL_TRACE, __VA_ARGS__); \
} \
} while (0)

/* TODO CDRIVER-3710 use MONGOC_LOG_LEVEL_ERROR */
#define MONITOR_LOG_ERROR(sm, ...) \
Expand Down
4 changes: 2 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ _mongoc_socket_setkeepalive_windows (SOCKET sd)
}
}
#else
#ifdef MONGOC_TRACE

static const char *
_mongoc_socket_sockopt_value_to_name (int value)
{
Expand All @@ -463,7 +463,7 @@ _mongoc_socket_sockopt_value_to_name (int value)
return "Unknown option name";
}
}
#endif

static void
_mongoc_socket_set_sockopt_if_less (int sd, int name, int value)
{
Expand Down
2 changes: 0 additions & 2 deletions src/libmongoc/src/mongoc/mongoc-topology-description.c
Original file line number Diff line number Diff line change
Expand Up @@ -1857,7 +1857,6 @@ transition_t gSDAMTransitionTable
NULL,
_mongoc_topology_description_check_if_has_primary}};

#ifdef MONGOC_TRACE
/*
*--------------------------------------------------------------------------
*
Expand Down Expand Up @@ -1896,7 +1895,6 @@ _mongoc_topology_description_type (mongoc_topology_description_t *topology)
return "Invalid";
}
}
#endif


/*
Expand Down
Loading