Re: Support a wildcard in backtrace_functions

On Fri, 8 Mar 2024 at 17:17, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
> Hm, we may not want backtrace_on_internal_error to be on by default.
> AFAICS, all ERRCODE_INTERNAL_ERROR are identifiable with the error
> message, so it's sort of easy for one to track down the cause of it
> without actually needing to log backtrace by default.

While maybe all messages uniquely identify the exact error, these
errors usually originate somewhere deep down the call stack in a
function that is called from many other places. Having the full stack
trace can thus greatly help us to find what caused this specific error
to occur. I think that would be quite useful to enable by default, if
only because it would make many bug reports much more actionable.

> 1. Rename the new GUC backtrace_functions_min_level to backtrace_min_level.
> 2. Add 'internal' to backtrace_min_level_options enum
> +static const struct config_enum_entry backtrace_functions_level_options[] = {
> +   {"internal", INTERNAL, false},
> +    {"debug5", DEBUG5, false},
> +    {"debug4", DEBUG4, false},
> 3. Remove backtrace_on_internal_error GUC which is now effectively
> covered by backtrace_min_level = 'internal';
>
> Does anyone see a problem with it?

Honestly, it seems a bit confusing to me to add INTERNAL as a level,
since it's an error code not log level. Also merging it in this way,
brings up certain questions:
1. How do you get the current backtrace_on_internal_error=true
behaviour? Would you need to set both backtrace_functions='*' and
backtrace_min_level=INTERNAL?
2. What is the default value for backtrace_min_level?
3. You still wouldn't be able to limit INTERNAL errors to FATAL level

I personally think having three GUCs in this patchset make sense,
especially since I think it would be good to turn
backtrace_on_internal_error on by default. The only additional change
that I think might be worth making is to make
backtrace_on_internal_error take a level argument, so that you could
configure postgres to only add stack traces to FATAL internal errors.

(attached is a patch that should fix the CI issue by adding
GUC_NOT_IN_SAMPLE backtrace_functions_min_level)