vircadia-web-sdk icon indicating copy to clipboard operation
vircadia-web-sdk copied to clipboard

Centralized logger implementation

Open namark opened this issue 4 years ago • 7 comments

used minimally just to solve https://github.com/vircadia/vircadia-web-sdk/issues/45

namark avatar Oct 30 '21 19:10 namark

Summary of recent updates.

SDK user facing changes: New LoggerConfiguration interface contains all the logger configuration for the user - output context and filters. SDK wide configuration is exposed in DefaultLoggerConfiguration object. New LoggerContext implementations allow to easily combine other context to log to multiple destinations, apply a filter per destination and buffer messages until an error is logged.
Main SDK logger is now prefixed with "vircadia-sdk" tag, and by default filtered to only log warning and error messages (will only have an effect once existing console logs are all replaced)

SDK developer facing changes: Message type parameter has been replaced with tag state changing function, which also supports multiple tags. Various logging functions now accept multiple objects to log constrained to specific list of types. The one time logging function is replaced with a state changing function and a getter for ease of use.

Regarding string and regexp filtering, I realized that the behavior would be context dependent. For example currently the console context just forwards parameters to the dev console, without ever combining them to a single string to apply a string filter to. Also even just conceptually for example the log level is not represented as text there, while it would be if logging to a file, which would change the behavior of such filters between these two contexts. So if we ever want to use this code for a user facing GUI log with string/regexp filter, we'll need to implement an appropriate context, which will itself provide the filtering functionality in whichever way it would make the most sense there.

namark avatar Nov 22 '21 15:11 namark

  • Please rename \tests\shared\Log.unit.test.ts to Log.unit.test.js (i.e., JavaScript - it doesn't appear to use any TypeScript features).

  • Please move \domain\shared\Log.ts into \shared\Log.ts. (The \domain folder is primarily for C++-equivalent domain server code.) Similarly move the corresponding Jest test file.

  • Unit tests are missing for the global Log object.

  • The JSDoc for types and interfaces is not coming through in the dev/sdk docs. To fix this, the JSDoc for types and interfaces need to be included inside a class (e.g., the main class where they're used). For example, the documentation for the following types doesn't appear:

    • LoggerConfiguration
    • LoggableObjectRecord

    For an example of including a type's JSDoc inside a class, see: \domain\networking\packets\AvatarData.ts.

  • It would be helpful to include in the PR description a list of the ways it is envisaged that the global Log object be used in practice.

ctrlaltdavid avatar Nov 24 '21 23:11 ctrlaltdavid

The Log object is meant to be used for logging purposed in the SDK, to replace all the current and future console logs. Similar to console it provides a logging function per level (debug, message, info, warning, error), and additionally a function that accepts a LogLevel enum which defines the logging levels and their numeric priority (can be expanded in future). The type of objects that can be logged is constrained to simplify the implementation of various backends (through LoggerContext interface), while minimizing any loss of information. Basic use example:

Log.warning("something's not right about these values", {value1: 13, value2: "friday"});

For structured filtering purposes in addition to log level the logger also implements tagging.

Log.tag("my-stuff", "important").message("my message");

The tag function accepts a list of strings returns a new logger instance that will include those tags with every message logged. If the given set of tags is used often, the new logger instance can be reused to avoiding repeating the tags.

const networkLog = Log.tag("networking");
...
networkingLog.error("Connection failed", ...);

Specifically to resolve https://github.com/vircadia/vircadia-web-sdk/issues/45 the logger implements prevention of duplicate messages, also using method chaining:

Log.one.warning("not implemented");

For completeness there is a once function that accepts a boolean enabled flag, but no specific use case for that.

The Log object is instantiated with DefaultLoggerConfiguration that specifies the output context and filters, and is exposed to SDK user for customization. Currently the defaults are, ConsoleLoggerContext which logs to dev console, and a simple filter to allow only warnings and errors, but in future this might be different between production and development or other build settings. The user can specify filters based on log levels, tags, or the full message, and specify the output by either using the LoggerContext implementations provided by the SDK, or implementing their own, which should be relatively straight forward, it's a single function with a well defined parameters to handle.

namark avatar Nov 26 '21 21:11 namark

I have tested with this PR and have had no problems. I approve this PR

Misterblue avatar Nov 28 '21 22:11 Misterblue

Ping! This does not merge into the master branch any more. Changes are needed.

Misterblue avatar Dec 03 '21 02:12 Misterblue

I believe David is adjusting it.

digisomni avatar Dec 03 '21 02:12 digisomni

Yes, I'm looking at this. It will need some further work (to be determined) before being merged.

ctrlaltdavid avatar Dec 03 '21 04:12 ctrlaltdavid