Opened 2 years ago

Last modified 2 years ago

#5612 new change

Replace instances of NSLog with ABPLogger.debugLog()

Reported by: dzhang Assignee: dzhang
Priority: Unknown Milestone: Adblock-Plus-for-iOS-next
Module: Adblock-Plus-for-iOS/macOS Keywords:
Cc: mario, tiago, ashephard, CraftyDeano Blocked By:
Blocking: Platform: iOS
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by dzhang)

Background

There's a couple things motivating this issue.

  1. NSLog is bad because it can expose unintended information and is unprofessional to leave in release builds.
  2. Having an 'ABPLogger' will reduce the need to refactor every single file that uses logging when it is needed to refactor logging.

Transitioning to os_log may be logical as it is the next step in logging by Apple. If we use something like Crashlytics, that has its own logging calls.

Making our own logging API will insulate the code from these changing needs. For now, having

class ABPLogger
{
    func debugLog(message: String) 
    {
        ...
    }
}

will be a good replacement.

This is one of the same reasons for using RxSwift as it helps keep the code stable across changing iOS SDK changes.

What to change

Replace instances of NSLog with ABPLogger.debugLog() or equivalent function.

Change History (7)

comment:1 Changed 2 years ago by dzhang

  • Description modified (diff)

comment:2 Changed 2 years ago by dzhang

  • Milestone changed from Adblock-Browser-for-iOS-next to Adblock-Plus-for-iOS-next

comment:3 follow-up: Changed 2 years ago by ashephard

Interesting topic. How do you envision the os_log system working? Would we have a class for logging (i.e. LoggingManager.swift) that would define the subsystem/category, or would each class have their own instance of the log?

comment:4 Changed 2 years ago by dzhang

  • Description modified (diff)
  • Summary changed from Replace instances of NSLog with os_log to Replace instances of NSLog with ABPLogger.debugLog()

comment:5 Changed 2 years ago by dzhang

  • Description modified (diff)

comment:6 in reply to: ↑ 3 ; follow-up: Changed 2 years ago by dzhang

Replying to ashephard:

Interesting topic. How do you envision the os_log system working? Would we have a class for logging (i.e. LoggingManager.swift) that would define the subsystem/category, or would each class have their own instance of the log?

I've updated the issue to better explain the background behind the change. A single class should be enough.

comment:7 in reply to: ↑ 6 Changed 2 years ago by ashephard

Replying to dzhang:

Replying to ashephard:

Interesting topic. How do you envision the os_log system working? Would we have a class for logging (i.e. LoggingManager.swift) that would define the subsystem/category, or would each class have their own instance of the log?

I've updated the issue to better explain the background behind the change. A single class should be enough.

You could also add types of log message so that if we needed to, we could limit logs to Errors only, Warnings only, Verbose etc.
I believe you could do that by extending os_log with your ABPLogger class. Also, with os_log you could also specify the category. For a personal project, I split networking, internal functions, UI etc.

Note: See TracTickets for help on using tickets.