Add debug system to Log class

[Pessimistress]

Follow up of the discussion on visgl/deck.gl#3957

[coveralls]

Coverage increased (+0.4%) to 41.431% when pulling 1dd3cfb on x/log-debug into 6fa9454 on master.

[tsherif]

I'm not sure what this is gaining us... The application can choose which events to emit, but it can also just choose when to log in the current system, no?

[tsherif]

Actually, thinking about this a bit further, I'd really rather not go in this direction. It's turning the logger into a generic pub/sub event system, which could lead to a maintenance nightmare, e.g. what's to stop people form just putting non-logging code in in the handler?

log.setEventHandlers({
  'model_event': (model) => model.setUniforms(...);
  'other_event': (resource) => resource.delete();
});

My feelings are that:

1. I don't think logging needs something that powerful.
2. I'd rather not have generic pub/sub events at all in our frameworks. My experience working on systems that use these kind of events internally is that they become very difficult to maintain, essentially because any part of the system can indirectly talk to any other part of the system.

[ibgreen]

I'm not sure what this is gaining us...

Yes good question... trying to articulate the potential pros of this system:

• small general gain: For most logging calls, this system can prevent the actual message string from being bundled, but that is partly offset by the need for an event identifier.
• small general gain: avoids cost of dynamically building a string or generating an inline function when debugging is not enabled (but I think that could be avoided without a new API by implementing Deprecate log message function syntax in favor of concatenation of params #108)
• big gain in few cases: We do have some very extensive logging in luma.gl (attributes tables etc) that would benefit from this.
• consistency: it is turned on/off using the existing probe.gl console system.

Cons:

• My initial reaction was that the log.event API looks disconnected from the rest of the Log API, it doesn't look like a log call. Perhaps if it took a priority parameter it would look more consistent (although there is some duplication as the handler of the event can apply priority too).

log.debugEvent(priority, ....);

[ibgreen]

I'd rather not have generic pub/sub events all in our frameworks.

That is a fair concern, though if we want debugging code to be dynamically loadable, aren't we are going to need something along these lines?

Perhaps we could redesign or add some safeties?

• prevent the user from overwriting the registered handlers
• do specific solutions in each module so it looks less like a generic pub-sub system.
• Agree that it would need to be applied with discipline for debugging purposes only.
• ..

[tsherif]

big gain in few cases: We do have some very extensive logging in luma.gl (attributes tables etc) that would benefit from this.

I don't see how an event system would help... what do we get from it that isn't accomplished by just making the logging conditional?

Perhaps we could redesign or add some safeties?

Maybe we should have a conversation about this? I'm starting the feel the problem definition isn't completely clear for this work...

[ibgreen]

I don't see how an event system would help... what do we get from it that isn't accomplished by just making the logging conditional?

Maybe we should have a conversation about this? I'm starting the feel the problem definition isn't completely clear for this work...

Yes sounds good. I didn't see you on the deck.gl slack channel, maybe start there?

[Pessimistress]

I get the concern about making this a generic system. After some cleaning up it doesn't look so bad living in the module itself: https://github.com/uber/deck.gl/pull/3957/files#diff-8f29c94ea19bb5f1fb8ced597e0b8305

So I'd rather not land this in probe for now until we have a better understanding of the goals.

[ibgreen]

So I'd rather not land this in probe for now until we have a better understanding of the goals.

Understood and this is fine with me.

[tsherif]

👍