{{{
/ Directory Server log function /
static void (slapi_logger)(int, char , char *, ...) = NULL;
}}}
nunc-stans can have no references to projects that use nunc-stans - it should be an application neutral library and conform to standards wherever possible, such as syslog.

Usually the way syslog is used is that the app does the openlog() very early during startup, then calls syslog repeatedly, then calls closelog() at the very end of shutdown. We need to have something like an ns_logging_startup and ns_logging_shutdown functions. For example, if you just want to use fprintf, then ns_logging_startup would call static FILE *logfd = fopen("mylogfile", "w+") and ns_logging_shutdown would call fclose(logfd).

There shouldn't be separate functions for ns_log_error, ns_log_debug. There should just be one function ns_log(int priority, const char format, ...) or similar - we can probably use the same levels from syslog (e.g. LOG_EMERG, LOG_ALERT, etc.) - dirsrv will need to be able to figure out what the numbers mean in order to map them to the dirsrv log levels. For example, to use fprintf, we can define an ns_log_fprintf(int priority, const char format, ...) function that would just wrap vfprintf.

The config logging function should be have the same signature as vsyslog: void (log_fct)(int priority, const char format, va_list ap);

dirsrv should define a new function like this:

void nunc_stans_logging(int priority, const char *format, va_list ap);

This function should convert the arguments into the form used for slapi_log_error_ext() e.g. map LOG_DEBUG to SLAPI_LOG_NUNCSTANS, LOG_WARNING and above to SLAPI_LOG_FATAL or maybe do something for SLAPI_LOG_CONNS, and add "nunc-stans" as the subsystem.

Finally, the underlying event framework also logs. For example, libevent has event_set_log_callback and event_set_fatal_callback. We probably need to extend nunc-stans typedef struct ns_event_fw_t to interface to the logging functions provided by the event framework.

tevent uses tevent_set_debug to set the callback function to use for logging

libev also uses event_set_log_callback

We should also make sure whatever approach we use will work with the systemd journal for logging.

Replying to [comment:5 nkinder]:

We should also make sure whatever approach we use will work with the systemd journal for logging.

I'm pretty sure if you use syslog(3) it will go to the systemd journal.

Replying to [comment:6 rmeggins]:

Replying to [comment:5 nkinder]:

We should also make sure whatever approach we use will work with the systemd journal for logging.

I'm pretty sure if you use syslog(3) it will go to the systemd journal.

Yes it does, I confirmed this earlier.

Replying to [comment:7 mreynolds]:

Replying to [comment:6 rmeggins]:

Replying to [comment:5 nkinder]:

We should also make sure whatever approach we use will work with the systemd journal for logging.

I'm pretty sure if you use syslog(3) it will go to the systemd journal.

Yes it does, I confirmed this earlier.

Using syslog will go to the systemd journal, but there is also a native systemd journal API (install systemd-devel and run 'man sd-journal' for details). The native API has some benefits that can prove useful in debugging, as it will track things like the source file, line number, and function name where the log message was output in the journal metadata.

Lennart also has a nice blog post about the benefits of the native systemd journal API here:

http://0pointer.de/blog/projects/journal-submit.html

New patches attached...

For this:
{{{
vbuf = PR_vsmprintf(fmt, varg);
62 syslog(priority, "%s", vbuf);
63 PR_smprintf_free (vbuf);
}}}
can you just use
{{{
vsyslog(priority, fmt, varg);
}}}
instead?

in ns_log - why do you need two va_list variables?
in ns_log - you could have it just call ns_log_valist

for the ds patch - nunc_stans_logging shouldn't be in the public interface - it will only ever be called from nunc-stans via the log_fct pointer - it should either be a static in daemon.c, or a "proto-slap.h" "private" function in log.c

nunc_stans_logging - you can't just call slapi_log_error_ext with varg twice - you'll need to make a copy using va_copy, then clean up the copy with va_end.

DS patch
0001-Ticket-48038-logging-should-be-pluggable.patch

nunc-stans patch
0001-Ticket-48038-nunc-stans-logging-should-be-pluggable.patch

Replying to [comment:11 rmeggins]:

For this:
{{{
vbuf = PR_vsmprintf(fmt, varg);
62 syslog(priority, "%s", vbuf);
63 PR_smprintf_free (vbuf);
}}}
can you just use
{{{
vsyslog(priority, fmt, varg);
}}}
instead?

Nice.

in ns_log - why do you need two va_list variables?

I stole code from cleanruv_log() - so that function can probably be revised(via a different ticket).

in ns_log - you could have it just call ns_log_valist

for the ds patch - nunc_stans_logging shouldn't be in the public interface - it will only ever be called from nunc-stans via the log_fct pointer - it should either be a static in daemon.c, or a "proto-slap.h" "private" function in log.c

Done.

nunc_stans_logging - you can't just call slapi_log_error_ext with varg twice - you'll need to make a copy using va_copy, then clean up the copy with va_end.

Done.

New patches attached...

nunc stans:

commit 5499cd4df5932a11072737c5ce78d300cfe52669

DS:

commit 050299e802661d6183927eb40b0719cf80368b15

nunc-stans part is now https://fedorahosted.org/nunc-stans/ticket/4

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1369

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.