New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup the Logging System #327

Closed
ilexp opened this Issue May 16, 2016 · 6 comments

Comments

1 participant
@ilexp
Member

ilexp commented May 16, 2016

Summary

The logging system is mostly fine right now, but its API is also the result of organic growth. Use the v3.0 major version step as an opportunity to cleanup and refactor the public API, as well as make some overdue changes to its internals.

Analysis

  • Log formatting methods could move into a distinct static LogFormat class.
  • Default logs could be moved to a static Logs class, making Log a completely non-static class.
  • Provide an easy way to introduce custom global logs, maybe using a generic Logs.Get<T> interface using static generic class storage internally.
  • Make indentation a property of log outputs, rather than logs or log entries, get rid of SharedState.
    • Could be expressed as ILogOutput Push / Pop indentation API.
  • LogEntry should not contain source log or context references. Consider making them part of the ILogOutput API instead.
    • For editor support, remove the core InMemoryLogOutput and replace it with an editor-specific log output that also stores source log and context information in addition to the LogEntry.
    • That way, core logs will be strictly serializable and more lightweight, while the editor can add as much diagnostic info as it wants.
  • Unify visual and text logging APIs, or at least make sure they're similar enough so they don't require their own "learning curve".
  • Use UTC Timestamps, update LogView to convert them back to local time.
  • Make Log and ILogOutput implementations thread-safe, if they aren't yet.
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 3, 2016

Progress

  • Moved formatting methods into new LogFormat static class.
  • Added human-friendly ToString of numeric IDs via LogFormat.HumanFriendlyId(...).

Immediate ToDo

  • Default logs could be moved to a static Logs class, making Log a completely non-static class.
  • LogEntry should not contain source log or context references. Consider making them part of the ILogOutput API instead.
    • For editor support, remove the core InMemoryLogOutput and replace it with an editor-specific log output that also stores source log and context information in addition to the LogEntry.
    • That way, core logs will be strictly serializable and more lightweight, while the editor can add as much diagnostic info as it wants.
  • Make indentation a property of log outputs, rather than logs or log entries, get rid of SharedState.
    • Could be expressed as ILogOutput Push / Pop indentation API.
  • Provide an easy way to introduce custom global logs, maybe using a generic Logs.Get<T> interface using static generic class storage internally.
  • Unify visual and text logging APIs, or at least make sure they're similar enough so they don't require their own "learning curve".
  • Use UTC Timestamps, update LogView to convert them back to local time.
  • Make Log and ILogOutput implementations thread-safe, if they aren't yet.
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 3, 2016

Progress

  • Default logs moved to a static Logs class, making Log a completely non-static class.
  • Did some early implementation sketches on possible log refactorings. Nothing pushed yet, mostly local experiments.

Notes on Local Draft

  • Take a close look at LogViewList and ViewEntry. The current API isn't really streamlined, there's room for improvement. Clean up rough edges!
  • See what can be done to improve on EditorLogOutput, with regard to both thread-safety and data access. It needs to become a thread-safe and performant aggregator.
    • Schedule received logs internally and only apply them on demand? (Is there a way to prevent others from keeping around the list reference?)
  • Take a close look at overall architecture and the big picture of the logging classes, as well as their integration with each other.
  • See if something can or should be done about the double-access to get any useful information out of EditorLogEntry.
  • See if it would make sense to make both LogEntry and EditorLogEntry a POD struct with public, rather than property access.
  • Modify LogViewList so the new entry event is fired only once for the complete list of new entries.
  • Modify EditorLogOutput so there is an event that (synchronously) notifies subscribers that there are new entries available. Use that event in LogViewList to enable the update timer on demand, rather than updating all the time.

Immediate ToDo

  • Conclude local experiments by either discarding or improving and pushing them.
  • LogEntry should not contain source log or context references. Consider making them part of the ILogOutput API instead.
    • For editor support, remove the core InMemoryLogOutput and replace it with an editor-specific log output that also stores source log and context information in addition to the LogEntry.
    • That way, core logs will be strictly serializable and more lightweight, while the editor can add as much diagnostic info as it wants.
  • Make indentation a property of log outputs, rather than logs or log entries, get rid of SharedState.
    • Could be expressed as ILogOutput Push / Pop indentation API.
  • Provide an easy way to introduce custom global logs, maybe using a generic Logs.Get<T> interface using static generic class storage internally.
  • Unify visual and text logging APIs, or at least make sure they're similar enough so they don't require their own "learning curve".
  • Use UTC Timestamps, update LogView to convert them back to local time.
  • Make Log and ILogOutput implementations thread-safe, if they aren't yet.
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 3, 2016

Progress

  • Finished the first refactoring step and pushed back all local changes.
  • LogEntry no longer contains source or context values and is now plain-old-data.
  • Replaced the previous generic InMemoryLogOutput with a more specific EditorLogOutput in the editor assembly. It's also secure for being used with multi-threaded log writing, as long as access is still single-threaded.
  • LogViewList now fires only a single event for all added log entries, rather than one for each.
  • Adjusted various logview-related APIs, but decided to keep LogEntry and EditorLogEntry as-is (non-public) for now.

Immediate ToDo

  • Make indentation a property of log outputs, rather than logs or log entries, get rid of SharedState.
    • Could be expressed as ILogOutput Push / Pop indentation API.
  • Provide an easy way to introduce custom global logs, maybe using a generic Logs.Get<T> interface using static generic class storage internally.
  • Unify visual and text logging APIs, or at least make sure they're similar enough so they don't require their own "learning curve".
  • Use UTC Timestamps, update LogView to convert them back to local time.
  • Make Log and ILogOutput implementations thread-safe, if they aren't yet.
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 3, 2016

Progress

  • UTC Timestamps
  • Indentation as output state, rather than input state, implemented via ILogOutput push / pop API.
  • Thread-safe Log implementation.

Immediate ToDo

  • Make all ILogOutput implementations thread-safe, if they aren't yet.
  • Provide an easy way to introduce custom global logs, maybe using a generic Logs.Get<T> interface using static generic class storage internally.
  • Unify visual and text logging APIs, or at least make sure they're similar enough so they don't require their own "learning curve".
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 4, 2016

Progress

  • Thread-safe ILogOutput implementation.
  • Lock-less thread safety in Log implementation
  • Custom global logs via Logs.Get method.
  • Applied all the recent updates to Log to VisualLog as well to sync their APIs.

Immediate ToDo

  • Extend the editor LogView to work with all available global logs, not just the predefined ones.
  • Allow attaching an EditorHintIcon to CustomLogInfo, which will be used by the LogView to display the channel.
    • This means every log needs to remember which CustomLogInfo it was created from.
@ilexp

This comment has been minimized.

Member

ilexp commented Dec 4, 2016

Done

@ilexp ilexp closed this Dec 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment