Discussion:
[SSSD] [sssd PR#5585][opened] Poor man's backtrace.
alexey-tikhonov
2021-04-14 13:22:21 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Author: alexey-tikhonov
Title: #5585: Poor man's backtrace.
Action: opened

PR body:
"""
In case SSSD is run with debug_level < 9, log everything
to a ring buffer in memory and flush the buffer to a log file on any
error (up to and including `min(0x0040, debug_level)`)
(i.e. if `debug_level` is explicitly set to 0 or 1 then only those
error levels will trigger backtrace, otherwise up to 2).

Feature is only supported for `logger == files`.

Feature is configurable via 'debug_backtrace_enabled' option.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5585/head:pr5585
git checkout pr5585
alexey-tikhonov
2021-04-14 13:53:27 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Author: alexey-tikhonov
Title: #5585: Poor man's backtrace.
Action: edited

Changed field: body
Original value:
"""
In case SSSD is run with debug_level < 9, log everything
to a ring buffer in memory and flush the buffer to a log file on any
error (up to and including `min(0x0040, debug_level)`)
(i.e. if `debug_level` is explicitly set to 0 or 1 then only those
error levels will trigger backtrace, otherwise up to 2).

Feature is only supported for `logger == files`.

Feature is configurable via 'debug_backtrace_enabled' option.
"""
alexey-tikhonov
2021-04-14 18:19:07 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: +Waiting for review
alexey-tikhonov
2021-04-28 12:57:02 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Author: alexey-tikhonov
Title: #5585: Poor man's backtrace.
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5585/head:pr5585
git checkout pr5585
alexey-tikhonov
2021-04-28 12:58:54 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Rebased and added one patch to avoid backtrace dump in case "DP target is not configured" (for example, in case provider == ldap it's expected that "Target [subdomains] is not initialized", etc)
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-828434325
elkoniu
2021-04-28 22:03:56 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

elkoniu commented:
"""
Small test I did by injecting some debug messages into LDAP `sbus_issue_request_done()`:
```
[sssd.conf]
debug_backtrace_enabled = true
debug_level = 0x3ff0
```
Extra log lines added to code:
```
[src/sbus/router/sbus_router_handler.c]
137 DEBUG(SSSDBG_TRACE_LDB, "11> SSSDBG_TRACE_LDB\n");
138 DEBUG(SSSDBG_BE_FO, "10>SSSDBG_BE_FO\n");
139 DEBUG(SSSDBG_TRACE_ALL, "9> SSSDBG_TRACE_ALL\n");
140 DEBUG(SSSDBG_TRACE_INTERNAL, "8> SSSDBG_TRACE_INTERNAL\n");
141 DEBUG(SSSDBG_TRACE_LIBS, "7> SSSDBG_TRACE_LIBS\n");
142 DEBUG(SSSDBG_TRACE_FUNC, "6> SSSDBG_TRACE_FUNC\n");
143 DEBUG(SSSDBG_FUNC_DATA, "5> SSSDBG_FUNC_DATA\n");
144 DEBUG(SSSDBG_CONF_SETTINGS, "4> SSSDBG_CONF_SETTINGS\n");
145 DEBUG(SSSDBG_MINOR_FAILURE, "3> SSSDBG_MINOR_FAILURE\n");
146 DEBUG(SSSDBG_OP_FAILURE, "2> SSSDBG_OP_FAILURE\n");
147 //DEBUG(SSSDBG_CRIT_FAILURE, "1> SSSDBG_CRIT_FAILURE\n");
148 //DEBUG(SSSDBG_FATAL_FAILURE, "0> SSSDBG_FATAL_FAILURE\n");
149
150 DEBUG(SSSDBG_TRACE_LDB, "11> SSSDBG_TRACE_LDB\n");
151 DEBUG(SSSDBG_BE_FO, "10>SSSDBG_BE_FO\n");
152 DEBUG(SSSDBG_TRACE_ALL, "9> SSSDBG_TRACE_ALL\n");
153 DEBUG(SSSDBG_TRACE_INTERNAL, "8> SSSDBG_TRACE_INTERNAL\n");
154 DEBUG(SSSDBG_TRACE_LIBS, "7> SSSDBG_TRACE_LIBS\n");
155 DEBUG(SSSDBG_TRACE_FUNC, "6> SSSDBG_TRACE_FUNC\n");
156 DEBUG(SSSDBG_FUNC_DATA, "5> SSSDBG_FUNC_DATA\n");
157 DEBUG(SSSDBG_CONF_SETTINGS, "4> SSSDBG_CONF_SETTINGS\n");
158 DEBUG(SSSDBG_MINOR_FAILURE, "3> SSSDBG_MINOR_FAILURE\n");
159 //DEBUG(SSSDBG_OP_FAILURE, "2> SSSDBG_OP_FAILURE\n");
160 DEBUG(SSSDBG_CRIT_FAILURE, "1> SSSDBG_CRIT_FAILURE\n");
161 //DEBUG(SSSDBG_FATAL_FAILURE, "0> SSSDBG_FATAL_FAILURE\n");
162
163 DEBUG(SSSDBG_TRACE_LDB, "11> SSSDBG_TRACE_LDB\n");
164 DEBUG(SSSDBG_BE_FO, "10>SSSDBG_BE_FO\n");
165 DEBUG(SSSDBG_TRACE_ALL, "9> SSSDBG_TRACE_ALL\n");
166 DEBUG(SSSDBG_TRACE_INTERNAL, "8> SSSDBG_TRACE_INTERNAL\n");
167 DEBUG(SSSDBG_TRACE_LIBS, "7> SSSDBG_TRACE_LIBS\n");
168 DEBUG(SSSDBG_TRACE_FUNC, "6> SSSDBG_TRACE_FUNC\n");
169 DEBUG(SSSDBG_FUNC_DATA, "5> SSSDBG_FUNC_DATA\n");
170 DEBUG(SSSDBG_CONF_SETTINGS, "4> SSSDBG_CONF_SETTINGS\n");
171 DEBUG(SSSDBG_MINOR_FAILURE, "3> SSSDBG_MINOR_FAILURE\n");
172 //DEBUG(SSSDBG_OP_FAILURE, "2> SSSDBG_OP_FAILURE\n");
173 //DEBUG(SSSDBG_CRIT_FAILURE, "1> SSSDBG_CRIT_FAILURE\n");
174 DEBUG(SSSDBG_FATAL_FAILURE, "0> SSSDBG_FATAL_FAILURE\n");
175
176 DEBUG(SSSDBG_TRACE_LDB, "11> SSSDBG_TRACE_LDB\n");
177 DEBUG(SSSDBG_BE_FO, "10>SSSDBG_BE_FO\n");
178 DEBUG(SSSDBG_TRACE_ALL, "9> SSSDBG_TRACE_ALL\n");
179 DEBUG(SSSDBG_TRACE_INTERNAL, "8> SSSDBG_TRACE_INTERNAL\n");
180 DEBUG(SSSDBG_TRACE_LIBS, "7> SSSDBG_TRACE_LIBS\n");
181 DEBUG(SSSDBG_TRACE_FUNC, "6> SSSDBG_TRACE_FUNC\n");
182 DEBUG(SSSDBG_FUNC_DATA, "5> SSSDBG_FUNC_DATA\n");
183 DEBUG(SSSDBG_CONF_SETTINGS, "4> SSSDBG_CONF_SETTINGS\n");
184 DEBUG(SSSDBG_MINOR_FAILURE, "3> SSSDBG_MINOR_FAILURE\n");
185 DEBUG(SSSDBG_OP_FAILURE, "2> SSSDBG_OP_FAILURE\n");
186 DEBUG(SSSDBG_CRIT_FAILURE, "1> SSSDBG_CRIT_FAILURE\n");
187 DEBUG(SSSDBG_FATAL_FAILURE, "0> SSSDBG_FATAL_FAILURE\n");
```
Result log with backtrace:
```
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0040): 2> SSSDBG_OP_FAILURE
********************** PREVIOUS MESSAGE WAS TRIGGERED BY THE FOLLOWING BACKTRACE:
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): org.freedesktop.DBus.NameOwnerChanged: Success
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_dispatch] (0x4000): Dispatching.
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_dispatch] (0x4000): Dispatching.
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_method_handler] (0x2000): Received D-Bus method sssd.dataprovider.getDomains on /sssd
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_senders_lookup] (0x2000): Looking for identity of sender [sssd.nss]
* (2021-04-28 21:42:47): [be[ldap.vm]] [dp_attach_req] (0x0400): DP Request [Subdomains #3]: New request. Flags [0000].
* (2021-04-28 21:42:47): [be[ldap.vm]] [dp_attach_req] (0x0400): Number of active DP request: 1
* (2021-04-28 21:42:47): [be[ldap.vm]] [dp_find_method] (0x0100): Target [subdomains] is not initialized
* (2021-04-28 21:42:47): [be[ldap.vm]] [_dp_req_recv] (0x0400): DP Request [Subdomains #3]: Receiving request data.
* (2021-04-28 21:42:47): [be[ldap.vm]] [dp_req_destructor] (0x0400): DP Request [Subdomains #3]: Request removed.
* (2021-04-28 21:42:47): [be[ldap.vm]] [dp_req_destructor] (0x0400): Number of active DP request: 0
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x8000): 10>SSSDBG_BE_FO
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x4000): 9> SSSDBG_TRACE_ALL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0040): 2> SSSDBG_OP_FAILURE
********************** BACKTRACE DUMP ENDS HERE *********************************

(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0020): 1> SSSDBG_CRIT_FAILURE
********************** PREVIOUS MESSAGE WAS TRIGGERED BY THE FOLLOWING BACKTRACE:
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x8000): 10>SSSDBG_BE_FO
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x4000): 9> SSSDBG_TRACE_ALL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0020): 1> SSSDBG_CRIT_FAILURE
********************** BACKTRACE DUMP ENDS HERE *********************************

(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0010): 0> SSSDBG_FATAL_FAILURE
********************** PREVIOUS MESSAGE WAS TRIGGERED BY THE FOLLOWING BACKTRACE:
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x8000): 10>SSSDBG_BE_FO
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x4000): 9> SSSDBG_TRACE_ALL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0010): 0> SSSDBG_FATAL_FAILURE
********************** BACKTRACE DUMP ENDS HERE *********************************

(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0040): 2> SSSDBG_OP_FAILURE
********************** PREVIOUS MESSAGE WAS TRIGGERED BY THE FOLLOWING BACKTRACE:
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x8000): 10>SSSDBG_BE_FO
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x4000): 9> SSSDBG_TRACE_ALL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x2000): 8> SSSDBG_TRACE_INTERNAL
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x1000): 7> SSSDBG_TRACE_LIBS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0400): 6> SSSDBG_TRACE_FUNC
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0200): 5> SSSDBG_FUNC_DATA
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0100): 4> SSSDBG_CONF_SETTINGS
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0080): 3> SSSDBG_MINOR_FAILURE
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0040): 2> SSSDBG_OP_FAILURE
********************** BACKTRACE DUMP ENDS HERE *********************************

(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0020): 1> SSSDBG_CRIT_FAILURE
(2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0010): 0> SSSDBG_FATAL_FAILURE
********************** PREVIOUS MESSAGE WAS TRIGGERED BY THE FOLLOWING BACKTRACE:
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0020): 1> SSSDBG_CRIT_FAILURE
* (2021-04-28 21:42:47): [be[ldap.vm]] [sbus_issue_request_done] (0x0010): 0> SSSDBG_FATAL_FAILURE
********************** BACKTRACE DUMP ENDS HERE *********************************
```
Result:
- `SSSDBG_TRACE_LDB` is not covered by `debug_level = 0x3ff0` and it is not included in backtrace `_all_levels_enabled()` so it is not presented in logs and backtrace dump
- `SSSDBG_TRACE_ALL` and `SSSDBG_BE_FO` are not covered by `debug_level = 0x3ff0` but are included in backtrace `_all_levels_enabled()` so they are not presented in logs but presented in backtrace dump
- `SSSDBG_FATAL_FAILURE`, `SSSDBG_CRIT_FAILURE` and `SSSDBG_OP_FAILURE` triggers backtrace dump
- with situation where two backtrace triggers are close to each other having treshold in `_bt_empty()` set to `2` is the right choice
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-828810740
elkoniu
2021-04-28 22:10:47 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

elkoniu commented:
"""
Post by alexey-tikhonov
Rebased and added one patch to avoid backtrace dump in case "DP target is not configured" (for example, in case provider == ldap it's expected that "Target [subdomains] is not initialized", etc)
I guess there will be more places like this - with wrong log level or false errors generating backtrace. Maybe we can ask QE to run some random tests on top of this PR and check if we can already spot another unwanted backtraces?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-828814191
elkoniu
2021-04-28 22:12:02 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

elkoniu commented:
"""
Apart of some small comments +1 from me.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-828814779
alexey-tikhonov
2021-04-29 08:20:18 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Inclusion of a specific message to a backtrace is controlled by `_backtrace_is_enabled()`, not `_all_levels_enabled()` (maybe names are not telling, but there are small comments provided).
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829036448
alexey-tikhonov
2021-04-29 08:22:44 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Post by elkoniu
Apart of some small comments +1 from me.
Thank you for the review.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829038049
alexey-tikhonov
2021-04-29 08:24:05 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Inclusion of a specific message to a backtrace is controlled by `_backtrace_is_enabled()`, not `_all_levels_enabled()` (maybe names are not telling, but there are small comments provided).

Exclusion of `SSSDBG_TRACE_LDB` from backtrace is intentional.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829036448
pbrezina
2021-04-29 10:22:03 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

pbrezina commented:
"""
Can we make it work also with other logger than files? If there is a reason why only files is supported, please include a comment in the commit message.

Why do you use underscore before function names? I'm not completely against it, but it is not something that is common for SSSD.

IIRC we also talked about making the backtrace size and trigger level configurable, did you omit it on purpose?

Please, write the release note as a release note and not as a commit description (don't dive into technical details and make it understandable by normal users), e.g. "If `debug_backtrace_enabled` is set to true, all debug messages are printed to the logs when an error is detected even if `debug_level` is set to low values." Or something like that.


"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829114268
pbrezina
2021-04-29 10:23:01 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

pbrezina commented:
"""
Code-wise LGTM.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829114806
alexey-tikhonov
2021-04-29 11:14:18 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Post by pbrezina
Can we make it work also with other logger than files?
- `stderr`: I think it's not required at all to support feature for `stderr` logger. As buffer is quite large, it would be very inconvenient to get it in console.
- `journal`: that's the question. I'm not sure that users who setup journal as a logger for sssd (probably with very low debug level) want to receive storm of messages (or huge packet) in case of error. Probably we will add this later (maybe feature will be disabled by default for journal). But I would really like to get some feedback first (for logger=files).
Post by pbrezina
If there is a reason why only files is supported, please include a comment in the commit message.
Ok.
Post by pbrezina
Why do you use underscore before function names? I'm not completely against it, but it is not something that is common for SSSD.
This is my way to indicate "private" (local/static to this module) functions (and `sss_debug_backtrace_` prefix for "public" functions)
I could also use `s_` prefix but I don't think you would like it more :)
FWIW, I don't recall anything relevant in "coding style"...
Post by pbrezina
IIRC we also talked about making the backtrace size and trigger level configurable, did you omit it on purpose?
Yes, I decided to keep it simple in a first version.
I have a feeling this will only be useful in a totally "default" configuration.
I guess users that consciously change debug related options in sssd.conf most probably want specific verbosity as a generic log. Maybe there are some use cases... But again, I'd like to have some feedback (or bug reports) first, before complicating feature.
Post by pbrezina
Please, write the release note as a release note and not as a commit description (don't dive into technical details and make it understandable by normal users), e.g. "If `debug_backtrace_enabled` is set to true, all debug messages are printed to the logs when an error is detected even if `debug_level` is set to low values." Or something like that.
Ok.

Thanks for review.


"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829143361
pbrezina
2021-04-29 12:39:47 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

pbrezina commented:
"""
Post by pbrezina
Can we make it work also with other logger than files?
* `stderr`: I think it's not required at all to support feature for `stderr` logger. As buffer is quite large, it would be very inconvenient to get it in console.
* `journal`: that's the question. I'm not sure that users who setup journal as a logger for sssd (probably with very low debug level) want to receive storm of messages (or huge packet) in case of error. Probably we will add this later (maybe feature will be disabled by default for journal). But I would really like to get some feedback first (for logger=files).
Post by pbrezina
If there is a reason why only files is supported, please include a comment in the commit message.
Ok.
Post by pbrezina
Why do you use underscore before function names? I'm not completely against it, but it is not something that is common for SSSD.
This is my way to indicate "private" (local/static to this module) functions (and `sss_debug_backtrace_` prefix for "public" functions)
I could also use `s_` prefix but I don't think you would like it more :)
FWIW, I don't recall anything relevant in "coding style"...
Ok.
Post by pbrezina
IIRC we also talked about making the backtrace size and trigger level configurable, did you omit it on purpose?
Yes, I decided to keep it simple in a first version.
Ok.
I have a feeling this will only be useful in a totally "default" configuration.
I guess users that consciously change debug related options in sssd.conf most probably want specific verbosity as a generic log. Maybe there are some use cases... But again, I'd like to have some feedback (or bug reports) first, before complicating feature.
The use case is to get more debug lines if needed for errors that happen only sporadicaly and customers needs to run sssd for a long time. Having a large debug level set is inconvenient for such situation. But we can add it later.
Post by pbrezina
Please, write the release note as a release note and not as a commit description (don't dive into technical details and make it understandable by normal users), e.g. "If `debug_backtrace_enabled` is set to true, all debug messages are printed to the logs when an error is detected even if `debug_level` is set to low values." Or something like that.
Ok.
Thanks for review.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-829204089
alexey-tikhonov
2021-04-30 17:19:05 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Author: alexey-tikhonov
Title: #5585: Poor man's backtrace.
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5585/head:pr5585
git checkout pr5585
alexey-tikhonov
2021-04-30 17:20:32 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

alexey-tikhonov commented:
"""
Rebased and updated commit message.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-830242339
thalman
2021-05-04 09:40:49 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: +Accepted
thalman
2021-05-04 09:40:53 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: -Waiting for review
thalman
2021-05-04 09:41:23 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

thalman commented:
"""
works for me. Thank you for the patch. ACK
"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-831813764
pbrezina
2021-05-04 10:00:51 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: +Ready to push
pbrezina
2021-05-05 15:12:57 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5585

* `master`
* 80963d683a0c7fd146fcfd21770990043b9d8449 - SBUS: changed debug level in sbus_issue_request_done() to avoid backtrace dump in case of 'ERR_MISSING_DP_TARGET'
* 6fb987b5cf64675a42dfaaccc877ed867867bd73 - SERVER: decrease log level in `orderly_shutdown()` to avoid backtrace in this case.
* f693078fe44ff0ae2f2c53180afaf532cfb92a1f - CERTMAP: removed "sss_certmap initialized" debug
* 97f046e72bdec06356a5ed5295f283d0529eb440 - DEBUG: log IMPORTANT_INFO if any bit >= OP_FAILURE is on
* 0aaf61c66b6f2758f89152f946360e00755a9846 - DEBUG: makes debug backtrace switchable
* 6b78b7aa802529fc885877f326650fb7e6527607 - CACHE_REQ: fixed REVERSE_INULL warning
* e3426ebeb52a821495ea5d34fdc408fe23df7416 - PAM: fixes a couple of covscan issues
* 59ba14e5a70ed0b9253c7a881d664fcd28c337e7 - DEBUG: poor man's backtrace
* f66b5aedab31f22642a50df8f7f458af8d3d7391 - DEBUG: got rid of most explicit DEBUG_IS_SET checks as a preliminary step for "logs backtrace" feature

"""

See the full comment at https://github.com/SSSD/sssd/pull/5585#issuecomment-832769687
pbrezina
2021-05-05 15:13:00 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: +Pushed
pbrezina
2021-05-05 15:13:01 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: -Accepted
pbrezina
2021-05-05 15:13:02 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Title: #5585: Poor man's backtrace.

Label: -Ready to push
pbrezina
2021-05-05 15:13:04 UTC
Permalink
URL: https://github.com/SSSD/sssd/pull/5585
Author: alexey-tikhonov
Title: #5585: Poor man's backtrace.
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5585/head:pr5585
git checkout pr5585

Loading...