Skip to content

Commit

Permalink
MINOR: trace: ensure -dt priority over traces config section
Browse files Browse the repository at this point in the history
Traces can be activated on startup either via -dt command line argument
or via the traces configuration section. This can caused confusion as it
may not be clear as trace source can be completed or overriden by one or
the other.

Fix the precedence to give the priority to the command line argument.
Now, each trace source configured via -dt is first resetted to a default
state before applying new settings. Then, it is impossible to change a
trace source via the configuration file if it was already targetted via
-dt argument.
  • Loading branch information
a-denoyelle committed Jan 10, 2025
1 parent da9a7e0 commit a50dd07
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 17 deletions.
1 change: 1 addition & 0 deletions include/haproxy/trace-t.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ struct trace_source {
enum trace_state state;
const void *lockon_ptr; // what to lockon when lockon is set
const struct trace_source *follow; // other trace source's tracker to follow
int cmdline; // true if source was activated via -dt command line args
};

#endif /* _HAPROXY_TRACE_T_H */
Expand Down
66 changes: 49 additions & 17 deletions src/trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <haproxy/istbuf.h>
#include <haproxy/list.h>
#include <haproxy/log.h>
#include <haproxy/global.h>
#include <haproxy/quic_conn-t.h>
#include <haproxy/sink.h>
#include <haproxy/trace.h>
Expand Down Expand Up @@ -348,18 +349,24 @@ void trace_no_cb(enum trace_level level, uint64_t mask, const struct trace_sourc
/* do nothing */
}

/* registers trace source <source>. Modifies the list element!
* The {start,pause,stop,report} events are not changed so the source may
* preset them.
*/
void trace_register_source(struct trace_source *source)
static void trace_source_reset(struct trace_source *source)
{
source->lockon = TRACE_LOCKON_NOTHING;
source->level = TRACE_LEVEL_USER;
source->verbosity = 1;
source->sink = NULL;
source->state = TRACE_STATE_STOPPED;
source->lockon_ptr = NULL;
source->cmdline = 0;
}

/* registers trace source <source>. Modifies the list element!
* The {start,pause,stop,report} events are not changed so the source may
* preset them.
*/
void trace_register_source(struct trace_source *source)
{
trace_source_reset(source);
LIST_APPEND(&trace_sources, &source->source_link);
}

Expand Down Expand Up @@ -473,6 +480,15 @@ static struct sink *_trace_get_sink(const char *name, char **msg,
return sink;
}

/* Returns true if <src> trace source configuration can be changed. */
static int trace_enforce_origin_priority(const struct trace_source *src)
{
/* Trace cannot be modified via configuration file (during startup) if
* already activated via -dt command line argument.
*/
return !src->cmdline || !(global.mode & MODE_STARTING);
}

/* Parse a "trace" statement. Returns a severity as a LOG_* level and a status
* message that may be delivered to the user, in <msg>. The message will be
* nulled first and msg must be an allocated pointer. A null status message output
Expand Down Expand Up @@ -555,6 +571,9 @@ static int _trace_parse_statement(char **args, char **msg, const char *file, int
return LOG_ERR;
}

if (src && !trace_enforce_origin_priority(src))
goto out;

if (strcmp(args[cur_arg], "follow") == 0) {
const struct trace_source *origin = src ? HA_ATOMIC_LOAD(&src->follow) : NULL;

Expand Down Expand Up @@ -589,12 +608,15 @@ static int _trace_parse_statement(char **args, char **msg, const char *file, int
}
}

if (src)
if (src) {
HA_ATOMIC_STORE(&src->follow, origin);
else
list_for_each_entry(src, &trace_sources, source_link)
if (src != origin)
}
else {
list_for_each_entry(src, &trace_sources, source_link) {
if (src != origin && trace_enforce_origin_priority(src))
HA_ATOMIC_STORE(&src->follow, origin);
}
}
cur_arg += 2;
goto next_stmt;
}
Expand Down Expand Up @@ -712,11 +734,15 @@ static int _trace_parse_statement(char **args, char **msg, const char *file, int
return LOG_ERR;
}

if (src)
if (src) {
HA_ATOMIC_STORE(&src->sink, sink);
else
list_for_each_entry(src, &trace_sources, source_link)
HA_ATOMIC_STORE(&src->sink, sink);
}
else {
list_for_each_entry(src, &trace_sources, source_link) {
if (trace_enforce_origin_priority(src))
HA_ATOMIC_STORE(&src->sink, sink);
}
}

cur_arg += 2;
goto next_stmt;
Expand Down Expand Up @@ -750,11 +776,15 @@ static int _trace_parse_statement(char **args, char **msg, const char *file, int
return *name ? LOG_ERR : LOG_WARNING;
}

if (src)
if (src) {
HA_ATOMIC_STORE(&src->level, level);
else
list_for_each_entry(src, &trace_sources, source_link)
HA_ATOMIC_STORE(&src->level, level);
}
else {
list_for_each_entry(src, &trace_sources, source_link) {
if (trace_enforce_origin_priority(src))
HA_ATOMIC_STORE(&src->level, level);
}
}

cur_arg += 2;
goto next_stmt;
Expand Down Expand Up @@ -960,10 +990,12 @@ static int trace_parse_statement(char **args, char **msg)

void _trace_parse_cmd(struct trace_source *src, int level, int verbosity)
{
trace_source_reset(src);
src->sink = sink_find("stderr");
src->level = level >= 0 ? level : TRACE_LEVEL_ERROR;
src->verbosity = verbosity >= 0 ? verbosity : 1;
src->state = TRACE_STATE_RUNNING;
src->cmdline = 1;
}

/* Parse a process argument specified via "-dt".
Expand Down

0 comments on commit a50dd07

Please sign in to comment.