Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions Workflow/Sources/WorkflowLogger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions WorkflowCombine/Sources/Logger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import os.signpost
@_spi(Logging) import Workflow

private extension OSLog {
static let worker = OSLog(subsystem: "com.squareup.WorkflowCombine", category: "Worker")
Expand All @@ -29,6 +30,8 @@ final class WorkerLogger<WorkerType: Worker> {
// MARK: - Workers

func logStarted() {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.begin,
log: .worker,
Expand All @@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
}

func logFinished(status: StaticString) {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.end,
log: .worker,
Expand All @@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
}

func logOutput() {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.event,
log: .worker,
Expand Down
7 changes: 7 additions & 0 deletions WorkflowConcurrency/Sources/Logger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import os.signpost
@_spi(Logging) import Workflow

private extension OSLog {
static let worker = OSLog(subsystem: "com.squareup.WorkflowConcurrency", category: "Worker")
Expand All @@ -29,6 +30,8 @@ final class WorkerLogger<WorkerType: Worker> {
// MARK: - Workers

func logStarted() {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.begin,
log: .worker,
Expand All @@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
}

func logFinished(status: StaticString) {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.end,
log: .worker,
Expand All @@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
}

func logOutput() {
guard WorkflowLogging.isOSLoggingAllowed else { return }

os_signpost(
.event,
log: .worker,
Expand Down
16 changes: 14 additions & 2 deletions WorkflowReactiveSwift/Sources/Logger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: -
Expand All @@ -43,6 +49,8 @@ final class WorkerLogger<WorkerType: Worker> {
// MARK: - Workers

func logStarted() {
guard WorkerLogging.enabled else { return }

os_signpost(
.begin,
log: .active,
Expand All @@ -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,
Expand Down
16 changes: 14 additions & 2 deletions WorkflowRxSwift/Sources/Logger.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: -
Expand All @@ -43,6 +49,8 @@ final class WorkerLogger<WorkerType: Worker> {
// MARK: - Workers

func logStarted() {
guard WorkerLogging.enabled else { return }

os_signpost(
.begin,
log: .active,
Expand All @@ -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,
Expand Down