fix: Expose an ObjC setter for consolePrint - #3120
Conversation
## Background ### InternalLogger produces private messages to OSLog The SDK logs diagnostic output to the local device console via `InternalLogger`. By default, in iOS 14+, this implementation uses `OSLog`, and it flags all messages as `.private`. For a developer testing an iOS app, reading OSLog messages requires `Console.app` or `idevicesyslog`, and messages are redacted due to their privacy setting: ``` MyApp(DatadogInternal)[123] <Fault>: <private> ``` ### consolePrint can be overridden, but only in Swift The SDK allows an application to override this default behavior, redirecting SDK output as desired. This is achieved by overwriting the global `consolePrint` variable with a different function, which `InternalLogger` will then use. However, this override only works in Swift: global function-typed vars are not trivially exposed to Objective-C, and there's no existing setter function exposed in the Objective-C bindings. ## Changes in this PR This PR adds a single function to the ObjC bindings for `InternalLogger`: ``` public static func setConsolePrint(_ block: @escaping (String, objc_CoreLoggerLevel) -> Void) ``` ...where the provided `block` is wrapped and assigned to `DatadogInternal.consolePrint`. This allows Objective-C client code to override `consolePrint` in the same way that Swift client code can. ## Testing ### Unit test coverage The SDK's test suite has coverage for individual ObjC bindings, so I've added a simple test that verifies the expected behavior of `setConsolePrint`. ### Empirical test In my test application, I provided a `consolePrint` callback that routes output to `NSLog`: ```objc [DDDatadog setVerbosityLevel:DDCoreLoggerLevelDebug]; [DDInternalLogger setConsolePrint:^(NSString* message, DDCoreLoggerLevel level) { NSLog(@"[Datadog] %@", message); }]; ``` ...and as a result, when I ran my app with `devicectl device process launch --console`, I got unredacted output from the SDK: ``` MyApp[123:54321] [Datadog] 🔥 Datadog SDK usage error: `clientToken` cannot be empty. ```
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 582d1c2a83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Replaces the global `consolePrint` function used by the SDK to emit internal log messages. | ||
| @objc | ||
| public static func setConsolePrint(_ block: @escaping (String, objc_CoreLoggerLevel) -> Void) { | ||
| DatadogInternal.consolePrint = { message, level in |
There was a problem hiding this comment.
When Objective-C callers invoke this after Datadog.initialize, this only reassigns the global DatadogInternal.consolePrint; the active DD.logger remains an InternalLogger whose printFunction was captured from the old global in DD.swift/Datadog.initialize, and most SDK diagnostics go through DD.logger. As a result the newly exposed setter silently fails to redirect initialized SDK logs except for direct consolePrint call sites; update the logger/wrap the print function dynamically or document/enforce pre-initialization use.
AGENTS.md reference: AGENTS.md:L68-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is consistent with the behavior of consolePrint in the Swift API: you're expected to provide a custom print function before initializing the SDK. If you change consolePrint in Swift (or, as of this PR, call setConsolePrint() in ObjC) after SDK initialization, there's no guarantee that it will take effect.
I think this is a reasonable caveat to accept given that a.) it's the established behavior in the Swift path, and b.) this code is in DatadogInternal and is primarily intended for use by other Datadog SDKs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a0eaeccee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case .error: .error | ||
| case .critical: .critical | ||
| } | ||
| block(message, objcLevel) |
There was a problem hiding this comment.
In Swift 6 builds of DatadogInternal, assigning this wrapper to DatadogInternal.consolePrint type-checks the wrapper as @Sendable because consolePrint is declared @Sendable; capturing the non-Sendable block here produces capture of 'block' with non-Sendable type ... in a '@Sendable' closure, so the module fails to build before ObjC callers can use the new API. Store the callback through a Sendable wrapper (or otherwise satisfy the @Sendable requirement) before assigning it.
AGENTS.md reference: AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
| - [FIX] onSessionStart is now called only after sampling information used by WebView Tracking is in place, avoiding missing traces in early requests. See [#3104][] | ||
| - [FIX] Fix crash when defining `onSessionStart` in RUM configuration in Swift 6 projects. See [#3106][] | ||
| - [IMPROVEMENT] Forward `local_cache_hit` signal on RUM resources [#3074][] | ||
| - [IMPROVEMENT] Expose `setConsolePrint` to Objective-C, allowing ObjC client code to redirect SDK console output. See [#3120][] |
There was a problem hiding this comment.
It is internal. No need to be advertised.
2 participants
Background
InternalLogger produces private messages to OSLog
The SDK logs diagnostic output to the local device console via
InternalLogger. By default, in iOS 14+, this implementation usesOSLog, and it flags all messages as.private.For a developer testing an iOS app, reading OSLog messages requires
Console.apporidevicesyslog, and messages are redacted due to their privacy setting:consolePrint can be overridden, but only in Swift
The SDK allows an application to override this default behavior, redirecting SDK output as desired. This is achieved by overwriting the global
consolePrintvariable with a different function, whichInternalLoggerwill then use.However, this override only works in Swift: global function-typed vars are not trivially exposed to Objective-C, and there's no existing setter function exposed in the Objective-C bindings.
Changes in this PR
This PR adds a single function to the ObjC bindings for
InternalLogger:...where the provided
blockis wrapped and assigned toDatadogInternal.consolePrint.This allows Objective-C client code to override
consolePrintin the same way that Swift client code can.Testing
Unit test coverage
The SDK's test suite has coverage for individual ObjC bindings, so I've added a simple test that verifies the expected behavior of
setConsolePrint.Empirical test
In my test application, I can now provide a
consolePrintcallback that routes output toNSLog:...and as a result, when I run my app with
devicectl device process launch --console, I get unredacted output from the SDK:Review checklist
make api-surfacewhen adding new APIs