-
Notifications
You must be signed in to change notification settings - Fork 98
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
Custom logger implementation on iOS #1026
Custom logger implementation on iOS #1026
Conversation
I will be making a docs PR after this is merged |
PrebidMobile/Logging/Log.swift
Outdated
@@ -33,38 +33,70 @@ public class Log: NSObject { | |||
public static var logToFile = false | |||
|
|||
public static func error(_ object: Any, filename: String = #file, line: Int = #line, function: String = #function) { | |||
log(object, logLevel: .error, filename: filename, line: line, function: function) | |||
if (customLogger == nil) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, this kind of functionality should be implemented in the following way:
- Introduce the class
SDKConsoleLogger
which implementsPrebidLogger
protocol - Introduce internal property
var logger: PrebidLogger? = SDKConsoleLogger()
- Introduce a custom setter for the
logger
so pubs can set up own one. - in all functions of the
Log
class just uselogger?.dosomething( .... )
It will alow to avoid if ... else
and seamlessly add another SDK loggers like SDKFileLogger
if needed.
PrebidMobile/Logging/Log.swift
Outdated
public static var logLevel: LogLevel = .debug | ||
public static var logToFile = false | ||
|
||
public static func error(_ object: Any, filename: String = #file, line: Int = #line, function: String = #function) { | ||
log(object, logLevel: .error, filename: filename, line: line, function: function) | ||
logger?.error(object, filename: filename, line: line, function: function) | ||
} | ||
|
||
public static func info(_ object: Any, filename: String = #file, line: Int = #line, function: String = #function) { | ||
log(object, logLevel: .info, filename: filename, line: line, function: function) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, you should remove system fogging from the Log class. Otherwise, the same messages will be logged twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They shouldn't because the SDKConsoleLogger's log
method doesn't post to the system log.
729d976
to
bb768f8
Compare
@ValentinPostindustria, could you please finalize this PR? |
f534dcf
to
0d602ae
Compare
bffef8e
to
b58f46c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #1023