[fix]: short circuit worker logging & require explicit opt-in (#285)

### Issue the existing os logging implementations log various type descriptions as part of the log message. these strings are apparently calculated even if the logging has been disabled, which introduces unneeded/undesirable work in release builds. these logs shouldn't be made in release builds, and should be opt-in in other contexts. ### Description this change adds short circuiting logic for all the os logging methods in addition to a new environment variable check that can be used to enable the logging when debugging.
square · Jun 21, 2024 · a8494ce · a8494ce
1 parent dcb58d7
commit a8494ce
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 10 deletions.
diff --git a/Workflow/Sources/WorkflowLogger.swift b/Workflow/Sources/WorkflowLogger.swift
@@ -14,21 +14,40 @@
  * 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: -
 
 /// 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.
+    @_spi(Logging)
+    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.
@@ -60,11 +79,16 @@ extension WorkflowLogging {
     /// To enable logging, at a minimum you must set:
     /// `WorkflowLogging.enabled = true`
     ///
+    /// Additionally, ``isOSLoggingAllowed`` must also be configured to be `true`.
+    ///
     /// If you wish for more control over what the runtime will log, you may additionally specify
     /// a custom value for `WorkflowLogging.config`.
     public static var enabled: Bool {
         get { OSLog.active === OSLog.workflow }
-        set { OSLog.active = newValue ? .workflow : .disabled }
+        set {
+            guard isOSLoggingAllowed else { return }
+            OSLog.active = newValue ? .workflow : .disabled
+        }
     }
 
     /// Configuration options used to determine which activities are logged.
@@ -93,7 +117,10 @@ final class WorkflowLogger {
     // MARK: Workflows
 
     static func logWorkflowStarted<WorkflowType>(ref: WorkflowNode<WorkflowType>) {
-        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 +134,20 @@ final class WorkflowLogger {
     }
 
     static func logWorkflowFinished<WorkflowType>(ref: WorkflowNode<WorkflowType>) {
-        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<Action: WorkflowAction>(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 +198,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
@@ -15,6 +15,7 @@
  */
 
 import os.signpost
+@_spi(Logging) import Workflow
 
 private extension OSLog {
     static let worker = OSLog(subsystem: "com.squareup.WorkflowCombine", category: "Worker")
@@ -29,6 +30,8 @@ final class WorkerLogger<WorkerType: Worker> {
     // MARK: - Workers
 
     func logStarted() {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .begin,
             log: .worker,
@@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     func logFinished(status: StaticString) {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .end,
             log: .worker,
@@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     func logOutput() {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .event,
             log: .worker,

diff --git a/WorkflowConcurrency/Sources/Logger.swift b/WorkflowConcurrency/Sources/Logger.swift
@@ -15,6 +15,7 @@
  */
 
 import os.signpost
+@_spi(Logging) import Workflow
 
 private extension OSLog {
     static let worker = OSLog(subsystem: "com.squareup.WorkflowConcurrency", category: "Worker")
@@ -29,6 +30,8 @@ final class WorkerLogger<WorkerType: Worker> {
     // MARK: - Workers
 
     func logStarted() {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .begin,
             log: .worker,
@@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     func logFinished(status: StaticString) {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .end,
             log: .worker,
@@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     func logOutput() {
+        guard WorkflowLogging.isOSLoggingAllowed else { return }
+
         os_signpost(
             .event,
             log: .worker,

diff --git a/WorkflowReactiveSwift/Sources/Logger.swift b/WorkflowReactiveSwift/Sources/Logger.swift
@@ -15,21 +15,27 @@
  */
 
 import os.signpost
+@_spi(Logging) import Workflow
 
 // Namespace for Worker logging
 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<WorkerType: Worker> {
     // MARK: - Workers
 
     func logStarted() {
+        guard WorkerLogging.enabled else { return }
+
         os_signpost(
             .begin,
             log: .active,
@@ -54,10 +62,14 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     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
@@ -15,21 +15,27 @@
  */
 
 import os.signpost
+@_spi(Logging) import Workflow
 
 // Namespace for Worker logging
 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<WorkerType: Worker> {
     // MARK: - Workers
 
     func logStarted() {
+        guard WorkerLogging.enabled else { return }
+
         os_signpost(
             .begin,
             log: .active,
@@ -54,10 +62,14 @@ final class WorkerLogger<WorkerType: Worker> {
     }
 
     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,