[rfc][icedtea-web] added support for linux system logging
Jiri Vanek
jvanek at redhat.com
Mon Jan 6 01:16:34 PST 2014
On 01/03/2014 09:49 PM, Andrew Azores wrote:
> In IcedTeaPluginUtils.h, you have a string constant in a syslog call that references "itweb-config". Is this meant to be itweb-settings?
Tahnx for check. This was included also in UnixSystemLog.java - fixed.
>
> IcedTeaPluginUtilsTest.cc - your new lines seem to be using 4x spaces as indentation whereas the existing lines are using tabs.
I would not call it so strict, but yes. You are starting to have sharp eye;) - fixed
>
> 'man logger' also says that the 'error' level is a deprecated synonym for 'err', so perhaps the UnixSystemLog should be passing 'user.err' to the ProcessBuilder instead.
hmhmh, ok - fixed
>
> Is the Process#waitFor() call here also needed? It's a rudimentary way to ensure that if UnixSystemLog#log() is called several times in succession that the logs always are written to disk in the order the calls occur, but do we really need this logging to be synchronous like this? I'm not opposed to it, just wondering if it's considered necessary.
I see it differently. The waitFor is waiting for process logger to finish. When logger process
finish, then it mean that the data were just send to logger daemon. Not written to disk or somehow
other way processed by daemon. So my "synchronization" is just ensuring sync on itw side, not in
global logging itself. (by otherwords, lines are mixed only with messages from different processes,
not from itw itself. Without waitFor individual lines of stack trace may land in wrong order.
If the logger process is waiting with successful exit untill dameon not only recieve message, but
also untill it is proccessed, then this approach must be revisited.
>
> Finally, is there no library at all we can use for this rather than using ProcessBuilder? I mean, this /works/, but it feels kind of hacky to have to do it this way.
Nope. It took me a long time to say final word here. And I could not believe it myself too. Few
logging libraries have the support for linux logging. But always those are BIG dependencies much
bigger then ITW itself. And the reputation of theirs linux logging backend were everything but not
good. "this is not working, this is not implemented" ....
If you have any possible replacement, feel free to scratch my approach!!!
Although I consider it on one one hand hackish too, on second hand I consider it as cleanest
approach at all :)
>
> Thanks,
> Andrew A
I have also changed PluginDebug to MESSADE_DEBUG it have much more sense. It is no error, just info
(/me jsut hope some tests do not depend on it)
Thanx for double eye!
>
> ----- Original Message -----
> From: "Jiri Vanek" <jvanek at redhat.com>
> To: "IcedTea Distro List" <distro-pkg-dev at openjdk.java.net>
> Sent: Friday, January 3, 2014 7:25:31 AM
> Subject: [rfc][icedtea-web] added support for linux system logging
>
> I was long time thinking how to integrate this feature.
> - it is platform specific
> - java on its own have poor support for linux logging
> - stdout/err is logged into /var/log/messages via /etc/gdm/Xsession on SOME machines
>
> At the end I bet to new java console as main source of debug information for developers, and let the
> syslog sleep for a while.
> Finally I decided to add it - to syslog only most important error messages are incoming - mostly
> fatal one, and information how to find more information is included. Also they are going here in
> both terminal and gdm/Xsession run. It means that in gdm/Xsession the critical error is duplicated
> but the manual one is better formatted,and contains the url to fill the bug. And terminal or systems
> without gdm (or similar) are not lacking this output.
>
> C part was pretty straightforward via syslog(3), but java part ? At the end i picked up most simple
> solution forking new *logger* process on each line of multi-line message. As the system logger is
> from java side called only once per run, then the performance impact should be none.
>
>
> http://icedtea.classpath.org/wiki/IcedTea-Web#Filing_bugs will need to be updated accordingly
> - mention itw*settings role
> - mention individual settings ^
> - mention usefulness of logs from console/stouts ....
> - mention linux system log
> - mention irc/mailing list/ bugzilla
>
> Looking forward to have this in!
> J.
>
-------------- next part --------------
diff -r 45d7ddf403eb netx/net/sourceforge/jnlp/util/logging/OutputController.java
--- a/netx/net/sourceforge/jnlp/util/logging/OutputController.java Fri Jan 03 10:49:36 2014 +0100
+++ b/netx/net/sourceforge/jnlp/util/logging/OutputController.java Mon Jan 06 10:15:39 2014 +0100
@@ -174,8 +174,13 @@
if (LogConfig.getLogConfig().isLogToFile()) {
getFileLog().log(message);
}
- if (LogConfig.getLogConfig().isLogToSysLog()) {
- getSystemLog().log(message);
+ //only crucial stuff is going to system log
+ //only java messages handled here, plugin is onhis own
+ if (LogConfig.getLogConfig().isLogToSysLog() &&
+ (s.getHeader().level.equals(Level.ERROR_ALL) || s.getHeader().level.equals(Level.WARNING_ALL)) &&
+ s.getHeader().isC == false) {
+ //no headers here
+ getSystemLog().log(s.getMessage());
}
}
diff -r 45d7ddf403eb netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java
--- a/netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java Fri Jan 03 10:49:36 2014 +0100
+++ b/netx/net/sourceforge/jnlp/util/logging/UnixSystemLog.java Mon Jan 06 10:15:39 2014 +0100
@@ -44,14 +44,23 @@
}
-
+
@Override
- public void log(String s) {
-
+ public void log(String message) {
+ final String s = "IcedTea-Web java error - for more info see itweb-settings debug options or console. See http://icedtea.classpath.org/wiki/IcedTea-Web#Filing_bugs for help.\nIcedTea-Web java error manual log: \n" + message;
+ try {
+ String[] ss = s.split("\\n"); //exceptions have many lines
+ for (String m : ss) {
+ m = m.replaceAll("\t", " ");
+ ProcessBuilder pb = new ProcessBuilder("logger", "-p","user.err", "--", m);
+ Process p = pb.start();
+ p.waitFor();
+ OutputController.getLogger().log("System logger called with result of " + p.exitValue());
+ }
+ } catch (Exception ex) {
+ OutputController.getLogger().log(ex);
+ }
}
-
-
-
}
diff -r 45d7ddf403eb plugin/icedteanp/IcedTeaPluginUtils.h
--- a/plugin/icedteanp/IcedTeaPluginUtils.h Fri Jan 03 10:49:36 2014 +0100
+++ b/plugin/icedteanp/IcedTeaPluginUtils.h Mon Jan 06 10:15:39 2014 +0100
@@ -47,6 +47,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
+#include <syslog.h>
#include <sys/time.h>
#include <fcntl.h>
@@ -153,6 +154,9 @@
push_pre_init_messages(ldebug_channel_message); \
} \
} \
+ if (plugin_debug_to_system){ \
+ /*no debug messages to systemlog*/\
+ } \
} \
} while (0)
@@ -196,6 +200,15 @@
push_pre_init_messages(ldebug_channel_message); \
} \
} \
+ if (plugin_debug_to_system){ \
+ /*java can not have prefix*/ \
+ openlog("", LOG_NDELAY, LOG_USER);\
+ syslog(LOG_ERR, "%s", "IcedTea-Web c-plugin - for more info see itweb-settings debug options or console. See http://icedtea.classpath.org/wiki/IcedTea-Web#Filing_bugs for help.");\
+ syslog(LOG_ERR, "%s", "IcedTea-Web c-plugin error manual log:");\
+ /*no headers to syslog*/ \
+ syslog(LOG_ERR, "%s", ldebug_body); \
+ closelog(); \
+ } \
} while (0)
diff -r 45d7ddf403eb plugin/icedteanp/java/sun/applet/PluginDebug.java
--- a/plugin/icedteanp/java/sun/applet/PluginDebug.java Fri Jan 03 10:49:36 2014 +0100
+++ b/plugin/icedteanp/java/sun/applet/PluginDebug.java Mon Jan 06 10:15:39 2014 +0100
@@ -53,7 +53,7 @@
for (Object chunk : messageChunks) {
b.append(chunk);
}
- OutputController.getLogger().log(OutputController.Level.ERROR_ALL, b.toString());
+ OutputController.getLogger().log(OutputController.Level.MESSAGE_DEBUG, b.toString());
}
}
}
diff -r 45d7ddf403eb tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc
--- a/tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc Fri Jan 03 10:49:36 2014 +0100
+++ b/tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc Mon Jan 06 10:15:39 2014 +0100
@@ -138,7 +138,7 @@
}
-void doDebugErrorRun() {
+void doDebugErrorRun(int max) {
FILE* old1 = stdout;
FILE* old2 = stderr;
char* buf1 = " ";
@@ -149,7 +149,6 @@
clock_t begin1, end1;
clock_t begin2, end2;
int i;
- int max = 1000000;
std::string hello = std::string("hello");
std::string eello = std::string("eello");
@@ -172,7 +171,7 @@
}
end2 = clock();
fclose(stdout);
- fclose(stderr);
+ fclose(stderr);
stdout = old1;
stderr = old2;
long time_spent1 = ((end1 - begin1));
@@ -181,27 +180,42 @@
fprintf (stdout, "PLUGIN_ERROR %d\n", time_spent2);
}
+void doDebugErrorRun() {
+ doDebugErrorRun(1000000);
+}
+
+/*
+ *The family of PLUGIN_DEBUG_ERROR_PROFILING tests actually do not test.
+ *It is just messure that the mechanisms around do not break soething fataly.
+ */
+
TEST(PLUGIN_DEBUG_ERROR_PROFILING_debug_on_headers_off) {
bool plugin_debug_backup = plugin_debug;
bool plugin_debug_headers_backup = plugin_debug_headers;
bool plugin_debug_console_backup = plugin_debug_to_console;
+ bool plugin_debug_system_backup = plugin_debug_to_system;
plugin_debug_to_console = false;
plugin_debug = true;
+ plugin_debug_to_system = false; //no need to torture system log in testing
doDebugErrorRun();
plugin_debug = plugin_debug_backup;
plugin_debug_headers = plugin_debug_headers_backup;
plugin_debug_to_console = plugin_debug_console_backup;
+ plugin_debug_to_system = plugin_debug_system_backup;
}
TEST(PLUGIN_DEBUG_ERROR_PROFILING_debug_off_headers_off) {
bool plugin_debug_backup = plugin_debug;
bool plugin_debug_headers_backup = plugin_debug_headers;
bool plugin_debug_console_backup = plugin_debug_to_console;
+ bool plugin_debug_system_backup = plugin_debug_to_system;
plugin_debug_to_console = false;
plugin_debug = false;
+ plugin_debug_to_system = false; //no need to torture system log in testing
doDebugErrorRun();
plugin_debug = plugin_debug_backup;
plugin_debug_headers = plugin_debug_headers_backup;
plugin_debug_to_console = plugin_debug_console_backup;
+ plugin_debug_to_system = plugin_debug_system_backup;
}
@@ -209,25 +223,47 @@
bool plugin_debug_backup = plugin_debug;
bool plugin_debug_headers_backup = plugin_debug_headers;
bool plugin_debug_console_backup = plugin_debug_to_console;
+ bool plugin_debug_system_backup = plugin_debug_to_system;
plugin_debug_to_console = false;
plugin_debug = true;
plugin_debug_headers = true;
+ plugin_debug_to_system = false; //no need to torture system log in testing
doDebugErrorRun();
plugin_debug = plugin_debug_backup;
plugin_debug_headers = plugin_debug_headers_backup;
plugin_debug_to_console = plugin_debug_console_backup;
+ plugin_debug_to_system = plugin_debug_system_backup;
}
TEST(PLUGIN_DEBUG_ERROR_PROFILING_debug_off_headers_on) {
bool plugin_debug_backup = plugin_debug;
bool plugin_debug_headers_backup = plugin_debug_headers;
bool plugin_debug_console_backup = plugin_debug_to_console;
+ bool plugin_debug_system_backup = plugin_debug_to_system;
plugin_debug_to_console = false;
plugin_debug = false;
plugin_debug_headers = true;
+ plugin_debug_to_system = false; //no need to torture system log in testing
doDebugErrorRun();
plugin_debug = plugin_debug_backup;
plugin_debug_headers = plugin_debug_headers_backup;
plugin_debug_to_console = plugin_debug_console_backup;
+ plugin_debug_to_system = plugin_debug_system_backup;
}
+TEST(PLUGIN_DEBUG_ERROR_PROFILING_debug_on_headers_on_syslog_on) {
+ bool plugin_debug_backup = plugin_debug;
+ bool plugin_debug_headers_backup = plugin_debug_headers;
+ bool plugin_debug_console_backup = plugin_debug_to_console;
+ bool plugin_debug_system_backup = plugin_debug_to_system;
+ plugin_debug_to_console = false;
+ plugin_debug = true;
+ plugin_debug_headers = true;
+ plugin_debug_to_system = true;
+ doDebugErrorRun(50);
+ plugin_debug = plugin_debug_backup;
+ plugin_debug_headers = plugin_debug_headers_backup;
+ plugin_debug_to_console = plugin_debug_console_backup;
+ plugin_debug_to_system = plugin_debug_system_backup;
+}
+
More information about the distro-pkg-dev
mailing list