Skip to content
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

Ability to trace application flow and customize logging. #663

Closed
wants to merge 3 commits into from

Conversation

mrublev
Copy link

@mrublev mrublev commented Oct 22, 2015

Improves logging by adding constants for .plist file, for:

  • SULogTraceLoggingKey - bool - enable tracing (default no)
  • SULogPersonalLogFileKey - bool - when enabled, application outputs
    to its own log file (default no = use common log file)
  • SULogClearAtLaunchKey - bool - indicates whether log file should
    be cleaned at app launch (default yes)
  • SULogMaxFileSizeKey - int - defines maximum file size for the log file,
    on each start updater check if this value is exceeded and trims it (1MB default)
  • SULogTrimMaxFileSizeCoefficientKey - float - the log file will be trimmed
    on this value (0.75 default)

SULog:

  • Add SULogTrace() function which is used to trace some important steps
    in the user flow.
  • SUMaybeTrimLogFile() checks file size and trims to desired size
  • SULoadLogSettingsFromBundle() loads settings from the bundle or user defaults

Michael Rublev added 2 commits October 22, 2015 14:36
Improves logging by adding constants for .plist file, for:
 - SULogTraceLoggingKey - bool - enable tracing (default no)
 - SULogPersonalLogFileKey - bool - when enabled, application outputs
 to its own log file (default no = use common log file)
 - SULogClearAtLaunchKey - bool - indicates whether log file should
 be cleaned at app launch (default yes)
 - SULogMaxFileSizeKey - int - defines maximum file size for the log file,
 on each start updater check if this value is exceeded and trims it (1MB default)
 - SULogTrimMaxFileSizeCoefficientKey - float - the log file will be trimmed
 on this value (0.75 default)

SULog:
* Add SULogTrace() function which is used to trace some important steps
 in the user flow.
* SUMaybeTrimLogFile() checks file size and trims to desired size
* SULoadLogSettingsFromBundle() loads settings from the bundle or user defaults
@kornelski
Copy link
Member

Thank you for the pull request.

It's useful to be able to trace what Sparkle is doing, but I'm not sure if has to be a runtime functionality enabled via info.plist. Would you use it for development, or instruct users to enable it?

Perhaps a macro that uses NSLog would suffice for trace messages? It wouldn't require log trimming functionality.

@mrublev
Copy link
Author

mrublev commented Oct 24, 2015

Thank you for your response.

Let me clarify the reasons of changes:

  • For a better support for an application, which uses Sparkle for updates, we would like to know what may cause a problem on a client side. For example, was it wrong appcast parameter or bad response, or may be user has old version because he skipped update before?... So it would be useful to have an event log, or a journal we could request from user.
  • It doesn't make sense to have records from other applications which uses Sparkle, that's why we need unique log file.
  • Truncation on start isn't good as well in my case.
  • Without truncation we need another mechanism for keeping file size in bounds, so trimming size to some sanity value on each start sounds good.
  • The reasons to have all those parameters in .plist file are: avoid rebuilding framework with some flags enabled or disabled, ability to configure official framework. Is the another or better way to configure Sparkle?

Also, default values for flags doesn't affect current approach Sparkle uses, so it must be compatible with previous versions.

@fzwo
Copy link

fzwo commented Dec 4, 2015

I don't have time to review the code, so I don't know whether the logs this produces would help avoid issues like #441 , but I welcome the changes outlined by @mrublev in principle. Separate, not auto-resetting log files are good.

@zorgiepoo
Copy link
Member

Not sure what my opinion on having this implemented, but I added a quick comment.

Altering the info.plist requires re-signing the app. IMO, as an app developer, we may as well re-compile the app. So one possibility is to expose an API to the app and have a different scheme or what not for tracing. Or another is having compile time flags but that'd require re-compiling Sparkle.

@kornelski
Copy link
Member

We've integrated deeper with macOS logging APIs instead. Now we fully use the Console.app, and don't rely on the logfile any more.

@kornelski kornelski closed this Mar 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants