feat: [SDK-4977] wire KMP remote logging lifecycle

[fadi-george]

Description

One Line Summary

Wire iOS SDK logs and lifecycle events into the shared KMP remote logger behind SDK_CUSTOM_LOGGING and remote log-level gates.

Details

Motivation

The SDK-4976 adapters do not emit telemetry until iOS composes LoggerFactory, routes OneSignalLog events, and flushes the shared logger during app lifecycle transitions.

Scope

• Creates and owns KMP remote telemetry after remote parameters load.
• Requires both sdk_custom_logging and a non-NONE logging_config.log_level; the flag-off path keeps existing local logging unchanged.
• Applies remote severity filtering, forwards SDK logs through LogLoggingHelper, flushes on app/scene backgrounding, and shuts down on termination/reset.
• Emits startup diagnostics with SDK/KMP versions, active path, feature flag, and crash directory.
• Includes KMP PR #15 as a stacked dependency.

Testing

Unit testing

• Focused OSLoggerAdaptersTests and OSRemoteLoggingControllerTests pass on the iOS simulator.
• Coverage verifies dual-gate behavior, severity filtering, routing, background flush, and shutdown.

Manual testing

• OneSignalFramework Release build succeeds for generic iOS.
• No physical-device /sdk/log smoke test was performed; rollout remains disabled unless remote config enables both gates.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

Made with Cursor

[cursor]

Multi-model adversarial review (interrogate)

Intent

Wire iOS SDK logs and lifecycle events into the shared KMP remote logger behind SDK_CUSTOM_LOGGING and remote log-level gates. After remote params load, create/own KMP remote telemetry only when both gates pass; otherwise keep local logging unchanged. Apply severity filtering, forward via LogLoggingHelper, flush on background, shut down on termination/reset, and emit startup diagnostics.

Reviewers

• Reviewer A: claude-fable-5-thinking-xhigh, 11 findings
• Reviewer B: gpt-5.6-sol-xhigh, 5 findings
• Reviewer C: cursor-grok-4.5-high-fast, 8 findings
• Reviewer D: claude-opus-5-thinking-xhigh, 11 findings

Verdict: request changes. Highest-confidence blockers are the stale/conflicting base and the WARN body-prefix bug (4/4 models).

Act On

1. PR is merge-conflicting with current fadi/sdk-4976 (C) — Base tip already moved adapters into OneSignalOSCore/Source/Logging/ with injected providers; this branch still edits the deleted monolithic Source/OSLoggerAdapters.swift. Rebase/rewrite onto the composition model before any further polish.
2. WARN remote bodies keep WARNING: prefix (A/B/C/D) — message(from:) strips "WARN: " but OneSignalLog.m formats warn as "WARNING: ". ERROR-only tests miss it. Prefer raw message on OneSignalLogEvent, or match the real prefix table.
3. Unparseable log_level enables zombie telemetry (A/B/C) — isRemoteLoggingEnabled accepts any non-NONE string; allows() then rejects all events. Parse once into a typed level and derive both enablement and filtering from that.
4. Background flush is async with no background-task assertion (B/C/D) — Observer returns before flush/network completes; iOS can suspend the process. Use OSBackgroundTaskManager and end the task from the flush completion. Prefer flush-then-shutdown on terminate.
5. Lifecycle observers register app + scene unconditionally (A/C/D) — Canonical OneSignalLifecycleObserver is exclusive via OSBundleUtils.isAppUsingUIScene. Dual registration double-flushes and can flush on single-scene backgrounding.

Consider

• Startup diagnostic at .LL_WARN on every params fetch, including path=local (A/C/D) — Default console level is WARN, so this changes local logging for everyone. Drop to DEBUG/VERBOSE when disabled, or emit only on the KMP path.
• Privacy-consent ordering (B) — configure runs before the remote privacy-consent key is applied; HTTP sender uses raw URLSession. Confirm whether remote log upload must be consent-gated like other mutating traffic.
• Speculative multi-shape feature-flag parsing (A/C/D) — Four payload shapes / three list key spellings with no pinned backend contract. Parse one documented shape and add constants like other remote params.
• App-ID change does not reset telemetry (B) — Stale flag/threshold can linger across setAppId until a successful params refresh.
• Controller snapshot vs provider live re-parse (D) — Provider calls OSRemoteLoggingConfiguration.current per access while the controller holds a snapshot; inject the snapshot instead.
• Test gaps (A/C/D) — Missing WARN body, invalid level, disable-after-enable, terminate flush, and teardown that always removes the global listener.

Noted

• Hand-written ObjC header shadowing Swift @objc class (drift risk).
• isListening redundant with telemetry != nil.
• Self-amplifying WARN loop if/when file-store failure logs are wired back through the listener (not fully live on this branch).

Dismissed

• Broad “delete unused file-store adapters” as a hard blocker on this PR alone — factory/KMP ownership may still consume them after rebase; revisit once composition is current.
• Nested stateQueue.sync deadlock via arbitrary public log listeners — real structure to keep in mind, but lower urgency than the verified correctness/lifecycle issues above.

Agreement Map

Strong consensus on WARN prefix stripping, invalid-level enablement, background-task gap, and dual lifecycle observers. Merge-conflict/stale-base was raised by one model and independently verified (mergeable: CONFLICTING). Privacy consent and app-ID retention were lone-model but concrete enough to keep under Consider.

  

_{Sent by Cursor Automation: Untitled}

[abdulraqeeb33]

Multimodal adversarial review (round 2)

Thanks for the harden pass — the prior Act-On blockers from the first review are fixed (raw OneSignalLogEvent.message, typed dual-gate, background-task flush, scene XOR app observers, consent/app-id reset, focused tests, KMP #15 bump). Merge base is clean.

Verdict: request changes for one remaining composition bug; everything else looks solid pending green CI.

Act on

1. Construct OSRemoteLogger / OSLoggerPlatformProvider on the main thread — configure() runs from the iOS params URLSession success callback (background), then creates the provider which reads UIDevice in init. The provider’s own docs require main-thread construction.

Consider

2. Move logStartupDiagnostic() off the stateQueue.sync section so a host OSLogListener that calls reset/configure cannot deadlock.
3. Prefer one ObjC surface for OSRemoteLoggingController (generated Swift header vs hand-written .h) to avoid signature drift.

Residual

• No physical-device /sdk/log smoke yet (ticket Done-when).
• CI was still pending at review time — wait for green after the main-thread fix.
• Keep exporterLoggingEnabled false until there is a non-reentrant diagnostic sink (IOSLogger → OneSignalLog → remote listener).

Full canvas write-up is in the Cursor session for this review.

[abdulraqeeb33]

Multimodal review (round 3)

Thanks for the harden passes — raw message, typed dual-gate, background-task flush, scene XOR app, and main-thread construction all look solid. OSRemoteLoggingControllerTests (8/8) are green; the red Build-and-Test job is failing in unrelated IAM/User/LiveActivities suites.

Blocker before merge: please answer the open consent thread (r3750752628) with an explicit product call (keep vs drop). That gate is review-invented, not in SDK-4977 Done-when, and Android does not mirror it today.

If keeping consent (recommended): inject a shared raw/binary POST transport into OneSignalLogHttpSender so /sdk/log reuses the SDK NSURLSession and consent runs at send-time. No KMP change needed (ILogHttpSender is already the injection port). Don’t force OTLP through today’s JSON-only executeRequest.

Also replied on the levelName → KMP question: leave native.

[abdulraqeeb33]

Looks build/test fails, but looks great otherwise. thank you for addressing all the items.

[nan-li]

reviewing

[fadi-george]

@nan-li addressed current set of comments

[nan-li]

Looks great!