Logs review #5422

alexey-tikhonov · 2020-11-30T23:29:12Z

This is a set of patches intended to enable higher log level by default.

Currently I target SSSDBG_OP_FAILURE (level 2) as a default (It seems SSSDBG_MINOR_FAILURE level contains legitimate messages that admin can deliberately ignore, and those messages can get quite spammy)

Perhaps several patches are yet missing (I'm awaiting @sumit-bose response) but those already published are ready for review.

alexey-tikhonov · 2020-12-01T21:46:45Z

@sgoveas , could you please take a look at the test TestSanityKCM.test_kcm_debug_level_set?
This test is broken by this PR.
Do I understand correctly it relies on the default debug_level=0? This PR changes this...

elkoniu

I know that main idea behind this PR is to change default log level and not mess with the log message. Still, maybe this is good moment to unify some common values representation like strings and numbers. Also we could change logs printing function names to say something more meaningful for the user. At the end advanced user will end up reading the code and common user will not care if talloc_*_() function failed etc.

src/db/sysdb_sudo.c

src/providers/ad/ad_dyndns.c

src/providers/ad/ad_gpo.c

src/providers/data_provider_opts.c

sgoveas · 2020-12-02T11:54:47Z

@sgoveas , could you please take a look at the test TestSanityKCM.test_kcm_debug_level_set?
This test is broken by this PR.
Do I understand correctly it relies on the default debug_level=0? This PR changes this...

yes it checks there are no logs generated in sssd_kcm.log with debug_level=0

alexey-tikhonov · 2020-12-02T13:35:17Z

I know that main idea behind this PR is to change default log level and not mess with the log message. Still, maybe this is good moment to unify some common values representation like strings and numbers. Also we could change logs printing function names to say something more meaningful for the user. At the end advanced user will end up reading the code and common user will not care if talloc_*_() function failed etc.

Yes, this makes sense. But not within this PR. It is huge already and topics are different indeed.

alexey-tikhonov · 2020-12-02T19:52:36Z

@sgoveas , could you please take a look at the test TestSanityKCM.test_kcm_debug_level_set?
This test is broken by this PR.
Do I understand correctly it relies on the default debug_level=0? This PR changes this...

yes it checks there are no logs generated in sssd_kcm.log with debug_level=0

@sgoveas , with debug patch the following is seen in the output:

/var/log/sssd/sssd_kcm.log
(2020-12-02 18:17:50): [kcm] [server_setup] (0x0040): Starting with debug level = 0x0070
 -----x----x----x----x-----x----x----

So seems test uses default log level, not debug_level=0?

alexey-tikhonov · 2020-12-03T21:05:40Z

There was an old bug that prevented setting debug_level = 0 in sssd.conf explicitly.
Pushed additional patch to fix it.

src/providers/ldap/sdap.c

src/providers/krb5/krb5_child.c

src/providers/ipa/ipa_sudo.c

alexey-tikhonov · 2020-12-07T17:22:13Z

It's preferably to include fix for #5425 in this PR.

alexey-tikhonov · 2020-12-13T22:37:55Z

Again, "those already published are ready for review"

alexey-tikhonov · 2020-12-15T17:51:12Z

Hi @sumit-bose ,

thanks for your responses. I think I addressed most of them.

Could you please take a look at a result?

src/providers/ipa/ipa_subdomains_id.c

src/man/include/debug_levels.xml

src/db/sysdb_ops.c

sumit-bose · 2020-12-21T12:09:41Z

Hi,

thanks for the thorough examination. I added a few minor comments in-line.

bye,
Sumit

sumit-bose · 2020-12-22T18:35:02Z

Pushed PR: #5422

master
- bd2f38a - UTIL: find_domain_by_object_name_ex() changed log level
- 0db68a1 - LDAP: sdap_save_grpmem(): log level changed
- 00e3ac4 - LDAP: reduce log level in case of fail to store members of missing group (it might be built-in skipped intentionally)
- dba7de0 - SYSDB: changed logging in sysdb_get_real_name()
- e86599b - IPA: changed logging in ipa_get_subdom_acct_send()
- bf87359 - IPA: ignore failed group search in certain cases
- 60b17be - SYSDB: changed log level in sysdb_update_members_ex()
- 9390af3 - IPA: reduce log level in apply_subdomain_homedir()
- 9215cf4 - CERTMAP: removed stray debug message
- 0986cf6 - UTIL: fixed bug in server_setup() that prevented setting debug level to 0 explicitly
- 644453f - LOGS: default log level changed to <= SSSDBG_OP_FAILURE
- 4fe060a - FILES: reduced debug level in refresh_override_attrs() if case "No overrides, nothing to do"
- 29f243f - AD: reduced log level in case check_if_pac_is_available() can't find user entry. This is typical situation when, for example, INITGROUPS lookup is executed for uncached user.
- ed6ec56 - SDAP: reduced log level in case group without members
- 26fdc3c - CACHE_REQ: reduced log level in cache_req_object_by_name_well_known() Non fqdn input isn't necessarily an error here.
- a7b145b - LDAP: reduced log level in hosts_get_done()
- 6e3b4d7 - SBUS: reduced log level in case of unexpected signal
- 90dae38 - RESPONDER: reduce log level in sss_parse_inp_done() in case of "Unknown domain" since this might be search by UPN
- 69aa3e8 - DP: do not log failure in case provider doesn't support check_online method
- 1af8992 - IPA: corrected confusing message
- a419b7e - SSS_IFACE: corrected misleading return code
- 99e44d9 - LDAP: added missed \n in log message
- 52dc855 - SYSDB: reduce log level in sysdb_update_members_ex() in case failed attempt to DEL unexisting attribute
- a7b6413 - UTIL: sss_ldb_error_to_errno() improved
- ac22859 - PAM: reduce log level in may_do_cert_auth()
- 5068655 - UTIL: few debug message corrections
- 3cbd046 - PAM: few debug message corrections
- f028253 - NSS: few debug message corrections
- f457a1a - IFP: few debug message corrections
- 058644f - RESPONDER: few debug message corrections
- 01ba32f - CACHE_REQ: debug message correction
- 018c08a - AUTOFS: few debug message corrections
- fb052a4 - RESOLV: debug message correction
- d91409d - PROXY: few debug message corrections
- ff8f44c - LDAP: few debug message corrections
- 9244820 - KRB5: few debug message corrections
- 667b983 - IPA: few debug message corrections
- 2f70695 - DP: few debug message corrections
- d6f6f05 - AD: few debug message corrections
- 85d8adc - P11_CHILD: severity level of few debug messages adjusted
- fe0530e - MONITOR: severity level of few debug messages adjusted
- daa5454 - SYSDB:views: few debug message corrections
- 82dc14b - SYSDB:upgrade: debug message corrected
- e731368 - SYSDB:service: severity level of few debug messages adjusted
- f55c959 - SYSDB:selinux: debug message severity level was adjusted
- 7445824 - SYSDB:search: few debug messages were corrected
- 033c31a - SYSDB:ops: few debug messages were corrected
- a73df70 - SYSDB:ipnetworks: severity level of few debug messages adjusted
- b4acf71 - SYSDB:iphosts: severity level of few debug messages adjusted
- d8af1db - SYSDB:sudo: changed debug message to be consistent
- df723cb - SYSDB: wrong debug message corrected
- e350d91 - SYSDB:autofs: cosmetic updates

It consists of four test cases: 1. Check the default debug level by authenticating the user 2. Check default level as 0 and 1 3. Check default level as 2 4. Check SBUS code triggers failure message during modules startup Verifies: Issues: SSSD#5422 SSSD#5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319 Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>

It consists of four test cases: 1. Check default debug level when sssd start successfully 2. Check default debug level by successful authentication of the user 3. Check default level as 0 and 1 4. Check default level as 2 5. Check SBUS code should not trigger failure message during modules startup Verifies: Issues: SSSD#5422 SSSD#5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319 Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>

It consists of five test cases: 1. Check default debug level when sssd start successfully 2. Check default debug level by successful authentication of the user 3. Check default level as 0 and 1 4. Check default level as 2 5. Check SBUS code should not trigger failure message during modules startup Verifies: Issues: SSSD#5422 SSSD#5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319 Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>

It consists of five test cases: 1. Check default debug level when sssd start successfully 2. Check default debug level by successful authentication of the user 3. Check default level as 0 and 1 4. Check default level as 2 5. Check SBUS code should not trigger failure message during modules startup Verifies: Issues: SSSD#5422 SSSD#5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319

It consists of five test cases: 1. Check default debug level when sssd start successfully 2. Check default debug level by successful authentication of the user 3. Check default level as 0 and 1 4. Check default level as 2 5. Check SBUS code should not trigger failure message during modules startup Verifies: Issues: #5422 #5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319 Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Anuj Borah <aborah@redhat.com>

It consists of five test cases: 1. Check default debug level when sssd start successfully 2. Check default debug level by successful authentication of the user 3. Check default level as 0 and 1 4. Check default level as 2 5. Check SBUS code should not trigger failure message during modules startup Verifies: Issues: SSSD#5422 SSSD#5425 Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1893159 https://bugzilla.redhat.com/show_bug.cgi?id=1915319 Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Anuj Borah <aborah@redhat.com>

elkoniu reviewed Dec 2, 2020

View reviewed changes

src/db/sysdb_sudo.c Show resolved Hide resolved

src/providers/ad/ad_dyndns.c Outdated Show resolved Hide resolved

src/providers/ad/ad_gpo.c Show resolved Hide resolved

src/providers/data_provider_opts.c Show resolved Hide resolved

alexey-tikhonov force-pushed the logs-review branch from 4974363 to 9e8c135 Compare December 3, 2020 21:03

elkoniu reviewed Dec 3, 2020

View reviewed changes

alexey-tikhonov force-pushed the logs-review branch from 9e8c135 to 5a5d2b6 Compare December 4, 2020 13:45

alexey-tikhonov force-pushed the logs-review branch from 5a5d2b6 to d254fff Compare December 12, 2020 20:37

alexey-tikhonov added the Waiting for review label Dec 13, 2020

alexey-tikhonov force-pushed the logs-review branch from 42f2468 to 8e67f1d Compare December 13, 2020 22:34

alexey-tikhonov force-pushed the logs-review branch from 8e67f1d to afdbb29 Compare December 15, 2020 16:27

sumit-bose reviewed Dec 21, 2020

View reviewed changes

src/providers/ipa/ipa_subdomains_id.c Outdated Show resolved Hide resolved

sumit-bose reviewed Dec 21, 2020

View reviewed changes

src/man/include/debug_levels.xml Outdated Show resolved Hide resolved

sumit-bose reviewed Dec 21, 2020

View reviewed changes

src/db/sysdb_ops.c Show resolved Hide resolved

alexey-tikhonov added Changes requested and removed Waiting for review labels Dec 21, 2020

alexey-tikhonov added 8 commits December 22, 2020 15:29

SYSDB:autofs: cosmetic updates

1e8a606

SYSDB: wrong debug message corrected

d2f3ca1

SYSDB:sudo: changed debug message to be consistent

e1e697c

SYSDB:iphosts: severity level of few debug messages adjusted

507ad8b

SYSDB:ipnetworks: severity level of few debug messages adjusted

5bdb335

SYSDB:ops: few debug messages were corrected

f92fc84

SYSDB:search: few debug messages were corrected

f9416f3

SYSDB:selinux: debug message severity level was adjusted

decf20f

sumit-bose self-assigned this Dec 22, 2020

sumit-bose added Pushed and removed Accepted Ready to push Ready to push labels Dec 22, 2020

sumit-bose closed this Dec 22, 2020

madhuriupadhye mentioned this pull request Jan 22, 2021

Tests: alltests: Check default debug level of sssd and corresponding logs #5473

Closed

sgoveas mentioned this pull request Dec 6, 2021

Check default debug level of sssd and corresponding logs #5908

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logs review #5422

Logs review #5422

alexey-tikhonov commented Nov 30, 2020

alexey-tikhonov commented Dec 1, 2020

elkoniu left a comment

sgoveas commented Dec 2, 2020 •

edited

Loading

alexey-tikhonov commented Dec 2, 2020

alexey-tikhonov commented Dec 2, 2020

alexey-tikhonov commented Dec 3, 2020

alexey-tikhonov commented Dec 7, 2020

alexey-tikhonov commented Dec 13, 2020

alexey-tikhonov commented Dec 15, 2020

sumit-bose commented Dec 21, 2020

sumit-bose commented Dec 22, 2020

Logs review #5422

Logs review #5422

Conversation

alexey-tikhonov commented Nov 30, 2020

alexey-tikhonov commented Dec 1, 2020

elkoniu left a comment

Choose a reason for hiding this comment

sgoveas commented Dec 2, 2020 • edited Loading

alexey-tikhonov commented Dec 2, 2020

alexey-tikhonov commented Dec 2, 2020

alexey-tikhonov commented Dec 3, 2020

alexey-tikhonov commented Dec 7, 2020

alexey-tikhonov commented Dec 13, 2020

alexey-tikhonov commented Dec 15, 2020

sumit-bose commented Dec 21, 2020

sumit-bose commented Dec 22, 2020

sgoveas commented Dec 2, 2020 •

edited

Loading