[danrbailey]

This came out of a few conversations and raised issues. Our logging is complicated. We use an optional third-party library (https://github.com/log4cplus/log4cplus) which provides lots of functionality, 98% of which we don't use. Most studios do not switch this extra logging on (I have never personally used it) and it is not tested in our CI infrastructure at all. The CMake code in particular is clunky and complicated.

However... it still provides some value, particularly for Houdini. We have lots of macros such as OPENVDB_LOG_ERROR that are used throughout the codebase. When Log4cplus is not enabled, it simply routes all the errors, logs, warnings, etc to stderr.

In this change, I have removed support for Log4cplus entirely and replaced it with a simple alternative that does everything we currently use Log4cplus which is to collect log data and propagate it.

Here's a simple example, this is what happens if you connect up a VDB Project Non-Divergent SOP with an empty velocity field, you get this output in the shell in which you launched Houdini. It is buried and often not seen by the user:

WARNING: pcg::solve(): matrix has dimension zero

The node itself displays this:

With this alternative logging implementation hooked up, you now get this:

In summary, in addition to the warning being output to the shell, it is also propagated to Houdini's UT_ErrorManager so that it is output on the node as well.

Houdini is the main use case, but this also affects binaries. There is now a flag on each of the binaries to set the logging diagnostic level (vdb_print -error for example).

@jmlait @Idclip

[jmlait]

I think the default is currently not to compile log4, so this change will effectively engage the warning logs in Houdini.

Those use the HoudniSink, which will send them to the current error manager. This can be troublesome as that isn't always the proper target - historically we've got a lot of errors sent to "/" and other nodes. We also have an issue that it is taking ERROR/FATAL and adding them as ABORT/FATAL. This becomes a significant behaviour change when logging is enabled - things will go from a cerr output to a red-flag on a node and stop the processing. If the logger intended a true ERROR/FATAL they need to raise an exception or otherwise return, not expect the logging path itself to terminate execution...

So I think the HoudniSink should downgrade error/fatal to warnings, with the assumption there will be an exception actually red-flag the node. (It is also less harmful with sending warnings to the wrong node - sending errors to the wrong node can break cooking)

The right thing is probably to have the HoudiniSink tie to the https://www.sidefx.com/docs/hdk/class_u_t___universal_log_source.html but I also don't think the perfect has to be the enemy of the good, as we want to remove log4fcplus as well.

My suggestion is thus to just downgrade the errors (with an explanation of why they can't be past at the same error level) and disable the HoudiniSink by default, having people opt into hte HoudiniSink like they did before. That way we can get rid of log4cplus and figure out the proper HoudiniSink as a separate pass.

[danrbailey]

I think the default is currently not to compile log4, so this change will effectively engage the warning logs in Houdini.

Those use the HoudniSink, which will send them to the current error manager. This can be troublesome as that isn't always the proper target - historically we've got a lot of errors sent to "/" and other nodes. We also have an issue that it is taking ERROR/FATAL and adding them as ABORT/FATAL. This becomes a significant behaviour change when logging is enabled - things will go from a cerr output to a red-flag on a node and stop the processing. If the logger intended a true ERROR/FATAL they need to raise an exception or otherwise return, not expect the logging path itself to terminate execution...

So I think the HoudniSink should downgrade error/fatal to warnings, with the assumption there will be an exception actually red-flag the node. (It is also less harmful with sending warnings to the wrong node - sending errors to the wrong node can break cooking)

The right thing is probably to have the HoudiniSink tie to the https://www.sidefx.com/docs/hdk/class_u_t___universal_log_source.html but I also don't think the perfect has to be the enemy of the good, as we want to remove log4fcplus as well.

My suggestion is thus to just downgrade the errors (with an explanation of why they can't be past at the same error level) and disable the HoudiniSink by default, having people opt into hte HoudiniSink like they did before. That way we can get rid of log4cplus and figure out the proper HoudiniSink as a separate pass.

Thanks for the feedback. I've added OPENVDB_HOUDINI_LOG_FORWARDING in the CMake which defaults to OFF now. I made the changes you suggested to use the UniversalLogSource and it seems to work ok. I can see "OpenVDB" entries showing up in the Log Viewer as intended. I also downgraded ERROR/FATAL as you suggested.