Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEBUG: Add debug to display ldapsearch requests #841

Closed
wants to merge 6 commits into from

Conversation

@thalman
Copy link
Contributor

thalman commented Jun 28, 2019

The existing debug output from SSSD is quite helpful in this respect,
but it would be better to have systemtap probes that can monitor executed
LDAP search calls and display returned results.

Two new probes are created (SDAP_PARSE_ENTRY, SDAP_PARSE_ENTRY_DONE) in
this PR to expose received attributes. The probe SDAP_GET_GENERIC_EXT_SEND
is extended of requested attributes list.

The systemtap script contrib/systemtap/ldap_perf.stp can be used to monitor
LDAP queries.

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1542137

@thalman thalman changed the title DEBUG: Add debug to display ldapsearch requests WIP DEBUG: Add debug to display ldapsearch requests Jun 28, 2019
@thalman thalman changed the title WIP DEBUG: Add debug to display ldapsearch requests DEBUG: Add debug to display ldapsearch requests Jun 28, 2019
@jhrozek

This comment has been minimized.

Copy link
Contributor

jhrozek commented Jul 31, 2019

The code looks OK. I haven't tried it, but looks simple enough.

But most importantly, it is not enough to resolve the issue, I think. What the original reporter asked for was a way to filter only these messages. And we can either add a special debug level, but wouldn't it be even better to add systemtap messages? See commits like d46d59e 1182dd9 and f199c74 to see how we added some generic instrumentation to the DP..

@thalman thalman force-pushed the thalman:ldapdebug branch from eb51dc9 to 7ba5811 Oct 9, 2019
@mzidek-gh mzidek-gh self-assigned this Oct 17, 2019
@mzidek-gh

This comment has been minimized.

Copy link
Contributor

mzidek-gh commented Oct 17, 2019

Assigning to myself for now, but I will get to this probably on Monday. If someone wants to review it sooner feel free to re-assign to yourself.

@mzidek-gh

This comment has been minimized.

Copy link
Contributor

mzidek-gh commented Oct 22, 2019

As per offline IRC discussion Tomas will add one more commit for man page change. Adding changes requested for now.

thalman added 2 commits Oct 7, 2019
SSSD systemtap probes for LDAP queries doesn't check the ldap search
filter. It's value can be NULL (means "no filter") and stap fails with

    ERROR: user string copy fault -14 at ...

This patch tests filter value before it is converted to printable
string and the filter is set to "<no filter>" in case of NULL value.
We would like to see list of requested attributes from LDAP server.
This patch adds attribute list to probe sdap_search_send where it
is pretty formated and stored in attrs:string variable.

Content of attrs may for example look like

    ["objectClass", "uid", "userPassword", "uidNumber", ...]
@thalman thalman force-pushed the thalman:ldapdebug branch from e302f2e to f461639 Oct 22, 2019
@thalman

This comment has been minimized.

Copy link
Contributor Author

thalman commented Oct 22, 2019

As per offline IRC discussion Tomas will add one more commit for man page change. Adding changes requested for now.

Man page extended of list of sample scripts and their purpose.

@thalman thalman force-pushed the thalman:ldapdebug branch from f461639 to 83d165e Oct 23, 2019
@mzidek-gh

This comment has been minimized.

Copy link
Contributor

mzidek-gh commented Oct 23, 2019

Please add the new script to the Makefile.am here:

1519 dist_sssdtapscript_DATA = \
1520     contrib/systemtap/id_perf.stp \
1521     contrib/systemtap/nested_group_perf.stp \
1522     contrib/systemtap/dp_request.stp \
1523     $(NULL)

And to the contrib/sssd.spec.in file here:

1081 %dir %{_datadir}/sssd/systemtap
1082 %{_datadir}/sssd/systemtap/id_perf.stp
1083 %{_datadir}/sssd/systemtap/nested_group_perf.stp
1084 %{_datadir}/sssd/systemtap/dp_request.stp

Other than that it LGTM.

thalman added 4 commits Oct 9, 2019
This patch adds two new probes witch allows to print
received LDAP object and response. System tap script
ldap_perf.stp uses those probes and provides feedback
to admin about LDAP queries executed by backend.
Add new probes with their description to the man page.
Having probes in sdap code requires out tests to be
linked with generated probes interface.
Describe sample SystemTap scripts in more detail.
@thalman thalman force-pushed the thalman:ldapdebug branch from 83d165e to c355dea Oct 25, 2019
@thalman

This comment has been minimized.

Copy link
Contributor Author

thalman commented Oct 31, 2019

retest this, please

@thalman

This comment has been minimized.

Copy link
Contributor Author

thalman commented Oct 31, 2019

Well, I don't see any error in logs, looks like CI issue, running retest for CentOS.

@mzidek-gh

This comment has been minimized.

Copy link
Contributor

mzidek-gh commented Nov 4, 2019

ACK

@pbrezina

This comment has been minimized.

Copy link
Member

pbrezina commented Nov 5, 2019

  • master
    • c7c08e1 - MAN: Update SystemTap man page
    • c790970 - TESTS: tests have to be linked with systemtap
    • c4568a9 - MAN: update systemtap man page
    • 88b875f - LDAP: Add probes to be able print ldap attributes
    • 7fd907c - LDAP: extend LDAP systemtap probes of attr list
    • 44d46cf - LDAP: Systemtap ldap probes fail without filter
@pbrezina pbrezina added Pushed and removed Accepted Ready to push labels Nov 5, 2019
@pbrezina pbrezina closed this Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.