Skip to content

Commit a8494ce

Browse files
jamieQjamieQ
authored
[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.
1 parent dcb58d7 commit a8494ce

File tree

5 files changed

+84
-10
lines changed

5 files changed

+84
-10
lines changed

‎Workflow/Sources/WorkflowLogger.swift

+42-6
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,40 @@
1414
* limitations under the License.
1515
*/
1616

17+
import Foundation
1718
import os.signpost
1819

1920
private extension OSLog {
2021
/// Logging will use this log handle when enabled
2122
static let workflow = OSLog(subsystem: "com.squareup.Workflow", category: "Workflow")
2223

23-
/// The active log handle to use when logging. Defaults to the shared `.disabled` handle.
24-
static var active: OSLog = .disabled
24+
/// The active log handle to use when logging. If `WorkflowLogging.osLoggingSupported` is
25+
/// `true`, defaults to the `workflow` handle, otherwise defaults to the shared `.disabled`
26+
/// handle.
27+
static var active: OSLog = {
28+
WorkflowLogging.isOSLoggingAllowed ? .workflow : .disabled
29+
}()
2530
}
2631

2732
// MARK: -
2833

2934
/// Namespace for specifying logging configuration data.
3035
public enum WorkflowLogging {}
3136

37+
extension WorkflowLogging {
38+
/// Flag indicating whether `OSLog` logs may be recorded. Note, actual emission of
39+
/// log statements in specific cases may depend on additional configuration options, so
40+
/// this being `true` does not necessarily imply logging will occur.
41+
@_spi(Logging)
42+
public static let isOSLoggingAllowed: Bool = {
43+
let env = ProcessInfo.processInfo.environment
44+
guard let value = env["com.squareup.workflow.allowOSLogging"] else {
45+
return false
46+
}
47+
return (value as NSString).boolValue
48+
}()
49+
}
50+
3251
extension WorkflowLogging {
3352
public struct Config {
3453
/// Configuration options to control logging during a render pass.
@@ -60,11 +79,16 @@ extension WorkflowLogging {
6079
/// To enable logging, at a minimum you must set:
6180
/// `WorkflowLogging.enabled = true`
6281
///
82+
/// Additionally, ``isOSLoggingAllowed`` must also be configured to be `true`.
83+
///
6384
/// If you wish for more control over what the runtime will log, you may additionally specify
6485
/// a custom value for `WorkflowLogging.config`.
6586
public static var enabled: Bool {
6687
get { OSLog.active === OSLog.workflow }
67-
set { OSLog.active = newValue ? .workflow : .disabled }
88+
set {
89+
guard isOSLoggingAllowed else { return }
90+
OSLog.active = newValue ? .workflow : .disabled
91+
}
6892
}
6993

7094
/// Configuration options used to determine which activities are logged.
@@ -93,7 +117,10 @@ final class WorkflowLogger {
93117
// MARK: Workflows
94118

95119
static func logWorkflowStarted<WorkflowType>(ref: WorkflowNode<WorkflowType>) {
96-
guard WorkflowLogging.config.logLifetimes else { return }
120+
guard
121+
WorkflowLogging.isOSLoggingAllowed,
122+
WorkflowLogging.config.logLifetimes
123+
else { return }
97124

98125
let signpostID = OSSignpostID(log: .active, object: ref)
99126
os_signpost(
@@ -107,14 +134,20 @@ final class WorkflowLogger {
107134
}
108135

109136
static func logWorkflowFinished<WorkflowType>(ref: WorkflowNode<WorkflowType>) {
110-
guard WorkflowLogging.config.logLifetimes else { return }
137+
guard
138+
WorkflowLogging.isOSLoggingAllowed,
139+
WorkflowLogging.config.logLifetimes
140+
else { return }
111141

112142
let signpostID = OSSignpostID(log: .active, object: ref)
113143
os_signpost(.end, log: .active, name: "Alive", signpostID: signpostID)
114144
}
115145

116146
static func logSinkEvent<Action: WorkflowAction>(ref: AnyObject, action: Action) {
117-
guard WorkflowLogging.config.logActions else { return }
147+
guard
148+
WorkflowLogging.isOSLoggingAllowed,
149+
WorkflowLogging.config.logActions
150+
else { return }
118151

119152
let signpostID = OSSignpostID(log: .active, object: ref)
120153
os_signpost(
@@ -165,6 +198,9 @@ final class WorkflowLogger {
165198
private static func shouldLogRenderTimings(
166199
isRootNode: Bool
167200
) -> Bool {
201+
guard WorkflowLogging.isOSLoggingAllowed else {
202+
return false
203+
}
168204
switch WorkflowLogging.config.renderLoggingMode {
169205
case .none:
170206
return false

‎WorkflowCombine/Sources/Logger.swift

+7
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
import os.signpost
18+
@_spi(Logging) import Workflow
1819

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

3132
func logStarted() {
33+
guard WorkflowLogging.isOSLoggingAllowed else { return }
34+
3235
os_signpost(
3336
.begin,
3437
log: .worker,
@@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
4043
}
4144

4245
func logFinished(status: StaticString) {
46+
guard WorkflowLogging.isOSLoggingAllowed else { return }
47+
4348
os_signpost(
4449
.end,
4550
log: .worker,
@@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
5055
}
5156

5257
func logOutput() {
58+
guard WorkflowLogging.isOSLoggingAllowed else { return }
59+
5360
os_signpost(
5461
.event,
5562
log: .worker,

‎WorkflowConcurrency/Sources/Logger.swift

+7
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
import os.signpost
18+
@_spi(Logging) import Workflow
1819

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

3132
func logStarted() {
33+
guard WorkflowLogging.isOSLoggingAllowed else { return }
34+
3235
os_signpost(
3336
.begin,
3437
log: .worker,
@@ -40,6 +43,8 @@ final class WorkerLogger<WorkerType: Worker> {
4043
}
4144

4245
func logFinished(status: StaticString) {
46+
guard WorkflowLogging.isOSLoggingAllowed else { return }
47+
4348
os_signpost(
4449
.end,
4550
log: .worker,
@@ -50,6 +55,8 @@ final class WorkerLogger<WorkerType: Worker> {
5055
}
5156

5257
func logOutput() {
58+
guard WorkflowLogging.isOSLoggingAllowed else { return }
59+
5360
os_signpost(
5461
.event,
5562
log: .worker,

‎WorkflowReactiveSwift/Sources/Logger.swift

+14-2
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,27 @@
1515
*/
1616

1717
import os.signpost
18+
@_spi(Logging) import Workflow
1819

1920
// Namespace for Worker logging
2021
public enum WorkerLogging {}
2122

2223
extension WorkerLogging {
2324
public static var enabled: Bool {
2425
get { OSLog.active === OSLog.worker }
25-
set { OSLog.active = newValue ? .worker : .disabled }
26+
set {
27+
guard WorkflowLogging.isOSLoggingAllowed else { return }
28+
OSLog.active = newValue ? .worker : .disabled
29+
}
2630
}
2731
}
2832

2933
private extension OSLog {
3034
static let worker = OSLog(subsystem: "com.squareup.WorkflowReactiveSwift", category: "Worker")
3135

32-
static var active: OSLog = .disabled
36+
static var active: OSLog = {
37+
WorkflowLogging.isOSLoggingAllowed ? .worker : .disabled
38+
}()
3339
}
3440

3541
// MARK: -
@@ -43,6 +49,8 @@ final class WorkerLogger<WorkerType: Worker> {
4349
// MARK: - Workers
4450

4551
func logStarted() {
52+
guard WorkerLogging.enabled else { return }
53+
4654
os_signpost(
4755
.begin,
4856
log: .active,
@@ -54,10 +62,14 @@ final class WorkerLogger<WorkerType: Worker> {
5462
}
5563

5664
func logFinished(status: StaticString) {
65+
guard WorkerLogging.enabled else { return }
66+
5767
os_signpost(.end, log: .active, name: "Running", signpostID: signpostID, status)
5868
}
5969

6070
func logOutput() {
71+
guard WorkerLogging.enabled else { return }
72+
6173
os_signpost(
6274
.event,
6375
log: .active,

‎WorkflowRxSwift/Sources/Logger.swift

+14-2
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,27 @@
1515
*/
1616

1717
import os.signpost
18+
@_spi(Logging) import Workflow
1819

1920
// Namespace for Worker logging
2021
public enum WorkerLogging {}
2122

2223
extension WorkerLogging {
2324
public static var enabled: Bool {
2425
get { OSLog.active === OSLog.worker }
25-
set { OSLog.active = newValue ? .worker : .disabled }
26+
set {
27+
guard WorkflowLogging.isOSLoggingAllowed else { return }
28+
OSLog.active = newValue ? .worker : .disabled
29+
}
2630
}
2731
}
2832

2933
private extension OSLog {
3034
static let worker = OSLog(subsystem: "com.squareup.WorkflowRxSwift", category: "Worker")
3135

32-
static var active: OSLog = .disabled
36+
static var active: OSLog = {
37+
WorkflowLogging.isOSLoggingAllowed ? .worker : .disabled
38+
}()
3339
}
3440

3541
// MARK: -
@@ -43,6 +49,8 @@ final class WorkerLogger<WorkerType: Worker> {
4349
// MARK: - Workers
4450

4551
func logStarted() {
52+
guard WorkerLogging.enabled else { return }
53+
4654
os_signpost(
4755
.begin,
4856
log: .active,
@@ -54,10 +62,14 @@ final class WorkerLogger<WorkerType: Worker> {
5462
}
5563

5664
func logFinished(status: StaticString) {
65+
guard WorkerLogging.enabled else { return }
66+
5767
os_signpost(.end, log: .active, name: "Running", signpostID: signpostID, status)
5868
}
5969

6070
func logOutput() {
71+
guard WorkerLogging.enabled else { return }
72+
6173
os_signpost(
6274
.event,
6375
log: .active,

0 commit comments

Comments
 (0)