From 432eb434acd9faf512082a233c26ad4a166d8c62 Mon Sep 17 00:00:00 2001 From: Jamie Quadri Date: Tue, 18 Jun 2024 14:52:21 -0500 Subject: [PATCH] [fix]: short circuit worker logging & require explicit opt-in --- Workflow/Sources/WorkflowLogger.swift | 40 +++++++++++++++++++--- WorkflowCombine/Sources/Logger.swift | 7 ++++ WorkflowConcurrency/Sources/Logger.swift | 7 ++++ WorkflowReactiveSwift/Sources/Logger.swift | 16 +++++++-- WorkflowRxSwift/Sources/Logger.swift | 16 +++++++-- 5 files changed, 77 insertions(+), 9 deletions(-) diff --git a/Workflow/Sources/WorkflowLogger.swift b/Workflow/Sources/WorkflowLogger.swift index d68a5eb17..44f211381 100644 --- a/Workflow/Sources/WorkflowLogger.swift +++ b/Workflow/Sources/WorkflowLogger.swift @@ -14,14 +14,19 @@ * limitations under the License. */ +import Foundation import os.signpost private extension OSLog { /// Logging will use this log handle when enabled static let workflow = OSLog(subsystem: "com.squareup.Workflow", category: "Workflow") - /// The active log handle to use when logging. Defaults to the shared `.disabled` handle. - static var active: OSLog = .disabled + /// The active log handle to use when logging. If `WorkflowLogging.osLoggingSupported` is + /// `true`, defaults to the `workflow` handle, otherwise defaults to the shared `.disabled` + /// handle. + static var active: OSLog = { + WorkflowLogging.isOSLoggingAllowed ? .workflow : .disabled + }() } // MARK: - @@ -29,6 +34,19 @@ private extension OSLog { /// Namespace for specifying logging configuration data. public enum WorkflowLogging {} +extension WorkflowLogging { + /// Flag indicating whether `OSLog` logs may be recorded. Note, actual emission of + /// log statements in specific cases may depend on additional configuration options, so + /// this being `true` does not necessarily imply logging will occur. + public static let isOSLoggingAllowed: Bool = { + let env = ProcessInfo.processInfo.environment + guard let value = env["com.squareup.workflow.allowOSLogging"] else { + return false + } + return (value as NSString).boolValue + }() +} + extension WorkflowLogging { public struct Config { /// Configuration options to control logging during a render pass. @@ -93,7 +111,10 @@ final class WorkflowLogger { // MARK: Workflows static func logWorkflowStarted(ref: WorkflowNode) { - guard WorkflowLogging.config.logLifetimes else { return } + guard + WorkflowLogging.isOSLoggingAllowed, + WorkflowLogging.config.logLifetimes + else { return } let signpostID = OSSignpostID(log: .active, object: ref) os_signpost( @@ -107,14 +128,20 @@ final class WorkflowLogger { } static func logWorkflowFinished(ref: WorkflowNode) { - guard WorkflowLogging.config.logLifetimes else { return } + guard + WorkflowLogging.isOSLoggingAllowed, + WorkflowLogging.config.logLifetimes + else { return } let signpostID = OSSignpostID(log: .active, object: ref) os_signpost(.end, log: .active, name: "Alive", signpostID: signpostID) } static func logSinkEvent(ref: AnyObject, action: Action) { - guard WorkflowLogging.config.logActions else { return } + guard + WorkflowLogging.isOSLoggingAllowed, + WorkflowLogging.config.logActions + else { return } let signpostID = OSSignpostID(log: .active, object: ref) os_signpost( @@ -165,6 +192,9 @@ final class WorkflowLogger { private static func shouldLogRenderTimings( isRootNode: Bool ) -> Bool { + guard WorkflowLogging.isOSLoggingAllowed else { + return false + } switch WorkflowLogging.config.renderLoggingMode { case .none: return false diff --git a/WorkflowCombine/Sources/Logger.swift b/WorkflowCombine/Sources/Logger.swift index 6fa3279c1..2c7f3f68d 100644 --- a/WorkflowCombine/Sources/Logger.swift +++ b/WorkflowCombine/Sources/Logger.swift @@ -15,6 +15,7 @@ */ import os.signpost +import Workflow private extension OSLog { static let worker = OSLog(subsystem: "com.squareup.WorkflowCombine", category: "Worker") @@ -29,6 +30,8 @@ final class WorkerLogger { // MARK: - Workers func logStarted() { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .begin, log: .worker, @@ -40,6 +43,8 @@ final class WorkerLogger { } func logFinished(status: StaticString) { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .end, log: .worker, @@ -50,6 +55,8 @@ final class WorkerLogger { } func logOutput() { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .event, log: .worker, diff --git a/WorkflowConcurrency/Sources/Logger.swift b/WorkflowConcurrency/Sources/Logger.swift index 5a9fa4e21..55d1b7335 100644 --- a/WorkflowConcurrency/Sources/Logger.swift +++ b/WorkflowConcurrency/Sources/Logger.swift @@ -15,6 +15,7 @@ */ import os.signpost +import Workflow private extension OSLog { static let worker = OSLog(subsystem: "com.squareup.WorkflowConcurrency", category: "Worker") @@ -29,6 +30,8 @@ final class WorkerLogger { // MARK: - Workers func logStarted() { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .begin, log: .worker, @@ -40,6 +43,8 @@ final class WorkerLogger { } func logFinished(status: StaticString) { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .end, log: .worker, @@ -50,6 +55,8 @@ final class WorkerLogger { } func logOutput() { + guard WorkflowLogging.isOSLoggingAllowed else { return } + os_signpost( .event, log: .worker, diff --git a/WorkflowReactiveSwift/Sources/Logger.swift b/WorkflowReactiveSwift/Sources/Logger.swift index 8921814a7..9b2a27684 100644 --- a/WorkflowReactiveSwift/Sources/Logger.swift +++ b/WorkflowReactiveSwift/Sources/Logger.swift @@ -15,6 +15,7 @@ */ import os.signpost +import Workflow // Namespace for Worker logging public enum WorkerLogging {} @@ -22,14 +23,19 @@ public enum WorkerLogging {} extension WorkerLogging { public static var enabled: Bool { get { OSLog.active === OSLog.worker } - set { OSLog.active = newValue ? .worker : .disabled } + set { + guard WorkflowLogging.isOSLoggingAllowed else { return } + OSLog.active = newValue ? .worker : .disabled + } } } private extension OSLog { static let worker = OSLog(subsystem: "com.squareup.WorkflowReactiveSwift", category: "Worker") - static var active: OSLog = .disabled + static var active: OSLog = { + WorkflowLogging.isOSLoggingAllowed ? .worker : .disabled + }() } // MARK: - @@ -43,6 +49,8 @@ final class WorkerLogger { // MARK: - Workers func logStarted() { + guard WorkerLogging.enabled else { return } + os_signpost( .begin, log: .active, @@ -54,10 +62,14 @@ final class WorkerLogger { } func logFinished(status: StaticString) { + guard WorkerLogging.enabled else { return } + os_signpost(.end, log: .active, name: "Running", signpostID: signpostID, status) } func logOutput() { + guard WorkerLogging.enabled else { return } + os_signpost( .event, log: .active, diff --git a/WorkflowRxSwift/Sources/Logger.swift b/WorkflowRxSwift/Sources/Logger.swift index a3d2807e2..0537df337 100644 --- a/WorkflowRxSwift/Sources/Logger.swift +++ b/WorkflowRxSwift/Sources/Logger.swift @@ -15,6 +15,7 @@ */ import os.signpost +import Workflow // Namespace for Worker logging public enum WorkerLogging {} @@ -22,14 +23,19 @@ public enum WorkerLogging {} extension WorkerLogging { public static var enabled: Bool { get { OSLog.active === OSLog.worker } - set { OSLog.active = newValue ? .worker : .disabled } + set { + guard WorkflowLogging.isOSLoggingAllowed else { return } + OSLog.active = newValue ? .worker : .disabled + } } } private extension OSLog { static let worker = OSLog(subsystem: "com.squareup.WorkflowRxSwift", category: "Worker") - static var active: OSLog = .disabled + static var active: OSLog = { + WorkflowLogging.isOSLoggingAllowed ? .worker : .disabled + }() } // MARK: - @@ -43,6 +49,8 @@ final class WorkerLogger { // MARK: - Workers func logStarted() { + guard WorkerLogging.enabled else { return } + os_signpost( .begin, log: .active, @@ -54,10 +62,14 @@ final class WorkerLogger { } func logFinished(status: StaticString) { + guard WorkerLogging.enabled else { return } + os_signpost(.end, log: .active, name: "Running", signpostID: signpostID, status) } func logOutput() { + guard WorkerLogging.enabled else { return } + os_signpost( .event, log: .active,