[PATCH] Make subdomain discovery less noisy

[Sumit Bose]

Hi,

this patch fixes https://fedorahosted.org/sssd/ticket/1517 by increasing
some of the debug levels and print a log messages about the actual
request only if the subdomain provider is configured.

bye,
Sumit

[Pavel Březina]

On 09/20/2012 09:16 PM, Sumit Bose wrote:
> Hi,
>
> this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> some of the debug levels and print a log messages about the actual
> request only if the subdomain provider is configured.
>
> bye,
> Sumit
>    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
>    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
>           err_maj, err_min, err_msg));
>
>     /* send reply back */
>
> -- 1.7.7.6

Hi,
in general the patch looks good to me. But I don't think that TRACE_ALL 
is a good debug level for every possible error (most configurations are 
not set with this level and for example DP_ERR_OFFLINE should be visible 
in normal configurations).

Anyway, why don't we skip subdomains request entirely for domains that 
doesn't have provider configured? This can be done for instance in 
get_domains_next().

Off topic:
We should unify debug levels of backend results. We are using multiple 
levels (from CONF_SETTINGS to TRACE_INTERNAL) for single message 
("Request processed. Returned %d,%d,%s").

[Sumit Bose]

On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > Hi,
> >
> > this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > some of the debug levels and print a log messages about the actual
> > request only if the subdomain provider is configured.
> >
> > bye,
> > Sumit
> >    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> >    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> >          err_maj, err_min, err_msg));
> >
> >    /* send reply back */
> >
> > -- 1.7.7.6
>
> Hi,
> in general the patch looks good to me. But I don't think that
> TRACE_ALL is a good debug level for every possible error (most
> configurations are not set with this level and for example
> DP_ERR_OFFLINE should be visible in normal configurations).

What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?
> Anyway, why don't we skip subdomains request entirely for domains
> that doesn't have provider configured? This can be done for instance
> in get_domains_next().

But then the responders must read and interpret the config of the
providers. It would be nice if the sssd_be of a domain can send at
startup a list of capabilities and other information, like e.g. the
preferred default regular expression to split user and domain name. But
this sounds more like a 1.10 feature.

bye,
Sumit
> Off topic:
> We should unify debug levels of backend results. We are using
> multiple levels (from CONF_SETTINGS to TRACE_INTERNAL) for single
> message ("Request processed. Returned %d,%d,%s").
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Pavel Březina]

On 09/21/2012 06:42 PM, Sumit Bose wrote:
> On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> > On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > > Hi,
> > >
> > > this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > > some of the debug levels and print a log messages about the actual
> > > request only if the subdomain provider is configured.
> > >
> > > bye,
> > > Sumit
> > >    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> > >    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> > >           err_maj, err_min, err_msg));
> > >
> > >     /* send reply back */
> > >
> > > -- 1.7.7.6
> >
> > Hi,
> > in general the patch looks good to me. But I don't think that
> > TRACE_ALL is a good debug level for every possible error (most
> > configurations are not set with this level and for example
> > DP_ERR_OFFLINE should be visible in normal configurations).
>
> What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?

Sorry, I should have been more clear. Can you make the debug level 
conditional please? Use the higher level if the subdomains target is not 
configured and the lower level otherwise.
> > Anyway, why don't we skip subdomains request entirely for domains
> > that doesn't have provider configured? This can be done for instance
> > in get_domains_next().
>
> But then the responders must read and interpret the config of the
> providers. It would be nice if the sssd_be of a domain can send at
> startup a list of capabilities and other information, like e.g. the
> preferred default regular expression to split user and domain name. But
> this sounds more like a 1.10 feature.

Ok.

Thanks,
Pavel.

[Jakub Hrozek]

On Mon, Sep 24, 2012 at 12:41:07PM +0200, Pavel Březina wrote:
> On 09/21/2012 06:42 PM, Sumit Bose wrote:
> > On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> > > On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > > > Hi,
> > > >
> > > > this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > > > some of the debug levels and print a log messages about the actual
> > > > request only if the subdomain provider is configured.
> > > >
> > > > bye,
> > > > Sumit
> > > >    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> > > >    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> > > >          err_maj, err_min, err_msg));
> > > >
> > > >    /* send reply back */
> > > >
> > > > -- 1.7.7.6
> > >
> > > Hi,
> > > in general the patch looks good to me. But I don't think that
> > > TRACE_ALL is a good debug level for every possible error (most
> > > configurations are not set with this level and for example
> > > DP_ERR_OFFLINE should be visible in normal configurations).
> >
> > What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?
>
> Sorry, I should have been more clear. Can you make the debug level
> conditional please? Use the higher level if the subdomains target is
> not configured and the lower level otherwise.

If the conditional would be too ugly then I would just stick with
TRACE_ALL. The reason for failure such as going offline would be
visible in the logs earlier anyway.

[Sumit Bose]

On Mon, Sep 24, 2012 at 01:03:44PM +0200, Jakub Hrozek wrote:
> On Mon, Sep 24, 2012 at 12:41:07PM +0200, Pavel Březina wrote:
> > On 09/21/2012 06:42 PM, Sumit Bose wrote:
> > > On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> > > > On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > > > > Hi,
> > > > >
> > > > > this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > > > > some of the debug levels and print a log messages about the actual
> > > > > request only if the subdomain provider is configured.
> > > > >
> > > > > bye,
> > > > > Sumit
> > > > >    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> > > > >    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> > > > >          err_maj, err_min, err_msg));
> > > > >
> > > > >    /* send reply back */
> > > > >
> > > > > -- 1.7.7.6
> > > >
> > > > Hi,
> > > > in general the patch looks good to me. But I don't think that
> > > > TRACE_ALL is a good debug level for every possible error (most
> > > > configurations are not set with this level and for example
> > > > DP_ERR_OFFLINE should be visible in normal configurations).
> > >
> > > What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?
> >
> > Sorry, I should have been more clear. Can you make the debug level
> > conditional please? Use the higher level if the subdomains target is
> > not configured and the lower level otherwise.
>
> If the conditional would be too ugly then I would just stick with
> TRACE_ALL. The reason for failure such as going offline would be
> visible in the logs earlier anyway.

New patch with condition added.

bye,
Sumit

[Pavel Březina]

On 09/24/2012 04:25 PM, Sumit Bose wrote:
> On Mon, Sep 24, 2012 at 01:03:44PM +0200, Jakub Hrozek wrote:
> > On Mon, Sep 24, 2012 at 12:41:07PM +0200, Pavel Březina wrote:
> > > On 09/21/2012 06:42 PM, Sumit Bose wrote:
> > > > On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> > > > > On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > > > > > Hi,
> > > > > >
> > > > > > this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > > > > > some of the debug levels and print a log messages about the actual
> > > > > > request only if the subdomain provider is configured.
> > > > > >
> > > > > > bye,
> > > > > > Sumit
> > > > > >    DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> > > > > >    DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> > > > > >           err_maj, err_min, err_msg));
> > > > > >
> > > > > >     /* send reply back */
> > > > > >
> > > > > > -- 1.7.7.6
> > > > >
> > > > > Hi,
> > > > > in general the patch looks good to me. But I don't think that
> > > > > TRACE_ALL is a good debug level for every possible error (most
> > > > > configurations are not set with this level and for example
> > > > > DP_ERR_OFFLINE should be visible in normal configurations).
> > > >
> > > > What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?
> > >
> > > Sorry, I should have been more clear. Can you make the debug level
> > > conditional please? Use the higher level if the subdomains target is
> > > not configured and the lower level otherwise.
> >
> > If the conditional would be too ugly then I would just stick with
> > TRACE_ALL. The reason for failure such as going offline would be
> > visible in the logs earlier anyway.
>
> New patch with condition added.
>
> bye,
> Sumit

Good enough. Ack.

Thanks,
Pavel.

[Jakub Hrozek]

On Mon, Sep 24, 2012 at 05:54:28PM +0200, Pavel Březina wrote:
> On 09/24/2012 04:25 PM, Sumit Bose wrote:
> > On Mon, Sep 24, 2012 at 01:03:44PM +0200, Jakub Hrozek wrote:
> > > On Mon, Sep 24, 2012 at 12:41:07PM +0200, Pavel Březina wrote:
> > > > On 09/21/2012 06:42 PM, Sumit Bose wrote:
> > > > > On Fri, Sep 21, 2012 at 11:06:28AM +0200, Pavel Březina wrote:
> > > > > > On 09/20/2012 09:16 PM, Sumit Bose wrote:
> > > > > > >Hi,
> > > > > > >
> > > > > > >this patch fixeshttps://fedorahosted.org/sssd/ticket/1517  by increasing
> > > > > > >some of the debug levels and print a log messages about the actual
> > > > > > >request only if the subdomain provider is configured.
> > > > > > >
> > > > > > >bye,
> > > > > > >Sumit
> > > > > > >-        DEBUG(SSSDBG_TRACE_LIBS, ("Request processed. Returned %d,%d,%s\n",
> > > > > > >+        DEBUG(SSSDBG_TRACE_ALL, ("Request processed. Returned %d,%d,%s\n",
> > > > > > >                err_maj, err_min, err_msg));
> > > > > > >
> > > > > > >          /* send reply back */
> > > > > > >-- 1.7.7.6
> > > > > >
> > > > > > Hi,
> > > > > > in general the patch looks good to me. But I don't think that
> > > > > > TRACE_ALL is a good debug level for every possible error (most
> > > > > > configurations are not set with this level and for example
> > > > > > DP_ERR_OFFLINE should be visible in normal configurations).
> > > > >
> > > > > What would you suggest, shall I switch it back to SSSDBG_TRACE_LIBS?
> > > >
> > > > Sorry, I should have been more clear. Can you make the debug level
> > > > conditional please? Use the higher level if the subdomains target is
> > > > not configured and the lower level otherwise.
> > >
> > > If the conditional would be too ugly then I would just stick with
> > > TRACE_ALL. The reason for failure such as going offline would be
> > > visible in the logs earlier anyway.
> >
> > New patch with condition added.
> >
> > bye,
> > Sumit
>
> Good enough. Ack.
>
> Thanks,
> Pavel.

Pushed to master.