[SSSD] [sssd PR#5570][opened] LDAP: make connection log levels consistent

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Author: alexey-tikhonov
Title: #5570: LDAP: make connection log levels consistent
Action: opened

PR body:
"""
Connection related events (established, expired, released) now use same debug level.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1945888 (I don't think it's worth cloning upstream)
"""

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

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Waiting for review

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Waiting for review

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Changes requested

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Author: alexey-tikhonov
Title: #5570: LDAP: make connection log levels consistent
Action: synchronized

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

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Accepted

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Changes requested

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

elkoniu commented:
"""
LGTM. It is up to you if you want to introduce some common keyword in those 3 log lines for easy grep.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-824811056

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

alexey-tikhonov commented:
"""
I was thinking about printing `fd`/`uri`.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-824819451

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

elkoniu commented:
"""
I am not sure if `fs` / `uri` is easy accessible in case of those two log lines.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-824825199

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

alexey-tikhonov commented:
"""

I am not sure if `fs` / `uri` is easy accessible in case of those two log lines.

Well, I was thinking about getting those via `LDAP_OPT_SOCKBUF` and `LDAP_OPT_URI` options from ldap handle.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-824851248

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Author: alexey-tikhonov
Title: #5570: LDAP: make connection log levels consistent
Action: synchronized

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

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Accepted

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Author: alexey-tikhonov
Title: #5570: LDAP: make connection log levels consistent
Action: synchronized

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

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

alexey-tikhonov commented:
"""
Added logging of `fd` to `sdap_id_release_conn_data()` (`sdap_id_conn_data_expire_handler()` just calls the former).
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-825923189

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Bugzilla

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Waiting for review

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

elkoniu commented:
"""
In the `sdap_id_conn_data_expire_handler` there is also an access to `struct sdap_id_conn_data` which you used in `sdap_id_release_conn_data` to extract and print file descriptor.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-827201785

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

alexey-tikhonov commented:
"""
That's correct, but as I pointed out: "sdap_id_conn_data_expire_handler() just calls the former", i.e. those two lines are always near in the log. It doesn't make sense to "pollute" the code with fd extraction, IMO.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-827441832

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Waiting for review

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Accepted

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Accepted

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

elkoniu commented:
"""
I have last question - in old log for `sdap_ldap_connect_callback_add` there was an `LDAP` keyword. In new log it has been removed. If it is not something which QA / client may be looking for in logs? Maybe instead we should have `LDAP` keyword in all those log lines? Other than that LGTM.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-827479734

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

alexey-tikhonov commented:
"""

Post by elkoniu
I have last question - in old log for `sdap_ldap_connect_callback_add` there was an `LDAP` keyword. In new log it has been removed.

I removed "LDAP" because it doesn't add any information and to make it consistent with other messages.

Post by elkoniu
If it is not something which QA / client may be looking for in logs?

Tests that run in upstream PR CI didn't fail.

Post by elkoniu
Maybe instead we should have `LDAP` keyword in all those log lines?

This is symmetric: if any test greps for a particular line addition of "LDAP" equally likely can cause issues.
To remove this word in a single place is easier (lesser patch) than to add in several places.

"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-827504462

[elkoniu]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Accepted

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Ready to push

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

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

* `master`
* 99beee3c320f2d81ffad6290c58ef303623b2f89 - LDAP: make connection log levels consistent

"""

See the full comment at https://github.com/SSSD/sssd/pull/5570#issuecomment-829105529

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: +Pushed

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Accepted

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Title: #5570: LDAP: make connection log levels consistent

Label: -Ready to push

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5570
Author: alexey-tikhonov
Title: #5570: LDAP: make connection log levels consistent
Action: closed

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