Allow customisable log line length #292
Conversation
lib/log_blackbox.c
Outdated
| chunk = msg_len_pt + sizeof(uint32_t); /* Reset */ | ||
|
|
||
| msg_len = qb_vsnprintf_serialize(chunk, QB_LOG_MAX_LEN, | ||
| msg_len = qb_vsnprintf_serialize(chunk, max_size, | ||
| "Log message too long to be stored in the blackbox. "\ | ||
| "Maximum is QB_LOG_MAX_LEN" , ap); |
There was a problem hiding this comment.
Not changing the message indicates me there are likely other issues to be hit,
at least in blackbox context.
lib/log.c
Outdated
| @@ -1196,6 +1221,9 @@ qb_log_ctl2(int32_t t, enum qb_log_conf c, qb_log_ctl2_arg_t arg_not4directuse) | |||
| case QB_LOG_CONF_EXTENDED: | |||
| conf[t].extended = arg_i32; | |||
| break; | |||
| case QB_LOG_CONF_MAX_LINE_LEN: | |||
| conf[t].max_line_length = arg_i32; | |||
| break; | |||
There was a problem hiding this comment.
No boundary requirement is no-go.
There was a problem hiding this comment.
You could, e.g., easily overflow the blackbox with a single message like that.
|
TBH I'm tempted to leave the blackbox line length restriction at QB_LOG_MAX_LEN as it's the one place where having log lines can really clog up the system. Any ideas for a limit for what would be the actual hard limit imposed in qb_log_ctl()? In principle I was hoping to avoid arbitrary limits in there. if you ask for more than is reasonable then you suffer the consequences at malloc() time - same as if you called malloc() yourself. I'm easy on this though, a 64K limit would not be unreasonable I think. |
No preference here. I think an upper limit anywhere from 2K to 16K would be reasonable. |
|
On 30/01/18 17:53 +0000, Ken Gaillot wrote:
> Any ideas for a limit for what would be the actual hard limit
> imposed in qb_log_ctl()? In principle I was hoping to avoid
> arbitrary limits in there. if you ask for more than is reasonable
> then you suffer the consequences at malloc() time - same as if you
> called malloc() yourself. I'm easy on this though, a 64K limit
> would not be unreasonable I think.
No preference here. I think an upper limit anywhere from 2K to 16K
would be reasonable.
I'd stay with 4K (page size) as the all-time maximum for _the whole,
combined, final message_ (incl. trailing '\0', giving recommendation
not to exceed 2K, and even better to fit into default 512B as it's
a pain to deal with longish lines -- and for machine handling, it's
becoming prevalent to use structured logging, see #286) limit + would
be noisy if 512B limit for a _format string_ for internally logged
messages is exceeded -- QB_LOG_FILTER_FORMAT_REGEX filter would
presumably suffer a lot performance-wise if we were liberal here as
well (we already are, but since the limited message length so far, it
would be pretty stupid idea ... until the point of relaxing the
maximum overall length), and frankly, if 512 ASCII characters are not
enough to explain some matter incl. references, something is utterly
wrong (plus emergency workaround is as simple as using qb_log("%s",
actual_string)). Since the format string is necessarily a
compile-time constant (you cannot build otherwise with callsite
section enabled), it should be feasible to arrange such a check as
assurably compile-time assertion.
And going deeper, proposed simple API is insufficient.
What one likely wants to know: "under current conditions, with the
current target->format (possibly modified qb_log_format_set()),
_user_tags_stringify_fn etc. how many bytes are there left for my
actual messages?".
Hence, qb_log_ctl should return this information upon setting
new value (0 ~ do not change anything, just return this
information). That's also rather convoluted, e.g. for %g extra
specifier without width constraint, "tags" for all 32+1 exclusive
bit combinations should be obtained as a best effort heuristics
which one yields longest string (and documention made explicit
it's desirable to have the specifier always width-constrained).
It'd be then easy to adapt new size accordingly if one is not
satisfied, by mere subtraction, and ideally it would happen
only in corner case scenarios (excessively long hostname?)
Also, figuring this limit internally, it'd be easier (worst-case
estimation and possibly slight correction) to implement strace-style
information on how many bytes have been omitted, which is definitely
more informational than mere three dots (which cannot really be
discerned from regular ellipsis in some conceivable cases). Related
extension would be to add ellipsis only for the actual inner message,
not at the end of the buffer, should the fields be ordered in fancy
ways.
…--
Jan (Poki)
|
include/qb/qblog.h
Outdated
| @@ -584,6 +584,7 @@ enum qb_log_conf { | |||
| QB_LOG_CONF_EXTENDED, | |||
| QB_LOG_CONF_IDENT, | |||
| QB_LOG_CONF_MAX_LINE_LEN, | |||
| QB_LOG_CONF_ELIPSIS, | |||
There was a problem hiding this comment.
fyi spelling is "ellipsis"
There was a problem hiding this comment.
Thanks, I knew it looked wrong, but couldn't quite figure out why!
Signed-off-by: Christine Caulfield <[email protected]>
ie after the format string is applied, not before! Signed-off-by: Christine Caulfield <[email protected]>
Keep the limit of 512 in the black box as it has limited capacity
|
In retrospect, thanks for sticking with my advice to keep the maximum (from the neighbour context |
|
Good spot! it just seemed a sensible suggestion at the time :) |
|
Oh, and this likely requires even more considerations before being https://lists.clusterlabs.org/pipermail/users/2019-April/025735.html |
Allow the logging line length to be changed. Any reasonable length is allowable, the default is 512 as before. Anything more than 512 incurs several mallocs.
Log lines that exceed the length will have ... as the last 3 characters.