feat(logger): [SDK-4835] adopt shared KMP logger module via OneSignal-KMP-SDK submodule#2687
Conversation
…otel replacement Introduces a new Kotlin Multiplatform `logger` module that mirrors the decoupled architecture of the `otel` module but has zero OpenTelemetry dependency, so the same observability pipeline can be shared with iOS. - commonMain: LogRecord/LogSeverity model, platform/logger/telemetry/crash interfaces, hand-rolled OTLP/JSON encoder, coroutine batch processor, remote + crash-local telemetry, crash reporter/uploader, and LoggerFactory composition root. - HTTP, file IO, clock and uuid are injected (no Ktor/okio); iOS actuals stubbed. - commonTest: unit tests for encoder, fields, batching, telemetry, crash flows. - core: Android adapters (logger, platform provider, HTTP sender, file store, crash handler, ANR detector) + LoggerLifecycleManager, wired behind a single LoggerModuleSwitch.USE_LOGGER_MODULE toggle (default false keeps the otel path and all existing tests unchanged). Logging gains a parallel logger sink. otel module left intact; removal tracked as follow-up. Co-authored-by: Cursor <cursoragent@cursor.com>
…its, and background I/O Switch the KMP logger module's wire format from OTLP/JSON to OTLP/protobuf (application/x-protobuf) via a dependency-free ProtoWriter, so the ingestion endpoint receives byte-compatible payloads with the old otel exporter. - Encode via hand-rolled ProtoWriter; drop kotlinx.serialization + OtlpDtos. - Port otel LogLimits: cap log-record attributes at 128 and truncate values to 32,000 chars (bounds long exception.stacktrace). - Make ILogFileStore.listReadable/delete suspend; run file I/O off-thread. - Route OneSignalLogHttpSender diagnostics through ILogger, gated on the exporter-logging toggle instead of unconditional logcat. - Add independent protobuf decode tests (ProtoTestReader) and limit tests. - Fix stale OTLP/JSON doc comments. Co-authored-by: Cursor <cursoragent@cursor.com>
…KMP-SDK submodule Extract the multiplatform :logger module into the standalone OneSignal-KMP-SDK repo (single source of truth for Android + iOS parity) and consume it here as a git submodule pinned to a commit. Remap the :OneSignal:logger project directory to the submodule so the existing project path and dependency substitution are unchanged. Fetch submodules recursively in every CI checkout that compiles the SDK, and document the clone/init workflow in CONTRIBUTING and README. Co-authored-by: Cursor <cursoragent@cursor.com>
…main Re-point the logger submodule from the feature-branch commit to the squash-merged main commit (503d610, PR #1). Logger sources are identical to the previously verified commit; :OneSignal:logger:testDebugUnitTest stays green. Co-authored-by: Cursor <cursoragent@cursor.com>
Companion to OneSignal-KMP-SDK#3 — FileLogStore reports persistence success, and crash/ANR handlers call saveCrash synchronously. Co-authored-by: Cursor <cursoragent@cursor.com>
Reusable (workflow_call) job the OneSignal-KMP-SDK Release workflow invokes to re-point the submodule gitlink to a released tag and open a bump PR here. Mirrors the OneSignal/sdk-shared fan-out pattern and authenticates with the shared org GH_PUSH_TOKEN, so no dedicated GitHub App is needed. Co-authored-by: Cursor <cursoragent@cursor.com>
…logger path Brings the KMP logger path to parity with the otel path after merging main (SDK-4822): - AndroidLogAnrDetector is now app-state aware, reusing the shared AnrCheckEvaluator (same tested decision core as OtelAnrDetector): foreground blocks report a fatal ANR via saveCrash; backgrounded blocks report a non-fatal warning via saveNonFatal under a distinct exception type + stack fingerprint, so they stay out of the ANR metric. - Wire isAppInForeground from the platform provider's appState in LoggerLifecycleManager. - Crash/ANR handlers call the synchronous saveCrash/saveNonFatal (no runBlocking), matching the fatal-handler-safe crash API. - Bump OneSignal-KMP-SDK submodule to 9cf1a33 (adds saveNonFatal + ossdk.crash.fatal). Co-authored-by: Cursor <cursoragent@cursor.com>
Companion to KMP revert of the sync crash API — durability remains the blocking ILogFileStore.save; hosts block the fatal handler until complete. Co-authored-by: Cursor <cursoragent@cursor.com>
Companion to KMP restoring synchronous ILogCrashReporter entry points. Co-authored-by: Cursor <cursoragent@cursor.com>
The bump PR now embeds the published OneSignal-KMP-SDK release notes for the target tag (fetched best-effort via gh; falls back to linking the release page) so a reviewer can see what the new logger version contains before merging. Re-runs refresh the PR body via `gh pr edit`. Co-authored-by: Cursor <cursoragent@cursor.com>
Add a "Working on the shared :logger module" section explaining the nested two-git model and two Mermaid flowcharts: Flow A (local edit → commit → push → merge to KMP main, no tag) and Flow B (manual Release workflow → tag + Android submodule-bump PR). Co-authored-by: Cursor <cursoragent@cursor.com>
|
@claude review once |
fadi-george
left a comment
fadi-george
left a comment
There was a problem hiding this comment.
:core now publishes a runtime dependency on com.onesignal:logger, but the logger module has no Maven publishing tasks. This would make the next SDK release unresolvable for Maven consumers. Please configure/publish the logger artifact or embed it in core, then verify using a Maven-local consumer build.
Needed so the logger module can stamp and verify vX.Y.* provenance under actions/checkout's shallow submodule clone. Co-authored-by: Cursor <cursoragent@cursor.com>
|
yup, i was planning to do that much later, but here you go. there are related PRs that address the publishing. |
Align the wrapped exceptionMessage argument to satisfy spotlessCheck. Co-authored-by: Cursor <cursoragent@cursor.com>
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)3/432 touched executable lines covered (0.7% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 0.7% (minimum 80%). |
|
The KMP publishing fix exists at aae38acc, but this PR still pins 9cf1a33, whose logger module has no publishing configuration. The publication blocker remains until the submodule is bumped and Android adopts the renamed :kmp module. |
Bump the OneSignal-KMP-SDK submodule to aae38ac, which renames the shared
Gradle module :logger -> :kmp and adds opt-in Maven publishing (published as
com.onesignal:kmp). Previously :core's POM referenced com.onesignal:logger,
which had no publishing configuration, so an SDK release would be unresolvable
for Maven consumers.
Android-side adoption:
- settings.gradle: include :OneSignal:kmp at ../OneSignal-KMP-SDK/kmp and
substitute com.onesignal:kmp.
- core/build.gradle: depend on project(':OneSignal:kmp').
- gradle.properties: set onesignalPublishSharedKmp=true so the shared module
wires up Maven publishing during the Android SDK release.
- CONTRIBUTING.md: update module name/tasks to :kmp.
The Kotlin package stays com.onesignal.logger, so no source imports change.
Verified with a Maven-local consumer build: com.onesignal:kmp:5.9.7 publishes
and :core's POM now depends on it.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks @fadi-george — addressed. Pushed Android-side adoption:
Provenance / pin: tagged the KMP repo Verified with a Maven-local consumer build:
Note: I dispatched the KMP |
2 participants
Summary
Introduces the multiplatform
:loggermodule — the OpenTelemetry-free replacement for theotelmodule — and wires the Android SDK to consume it from the standalone OneSignal-KMP-SDK repo as a git submodule, so the exact same logging/telemetry pipeline can be shared with the iOS SDK and both release in parity.This branch is up to date with
main(latest merge brings in SDK-4822, the app-state-aware ANR watchdog + fatal/non-fatal crash indicator), and that work is now ported into the new logger path so it stays at parity withotel.What's included
OneSignal-KMP-SDKadded as a git submodule;settings.gradleremaps:OneSignal:loggerto the submodule path (project path + dependency substitution unchanged). In-repo logger sources removed.otelbatch sizes,LogLimits(128 attrs / 32000 chars),service.name=OneSignalDeviceSDK/ scopeloggerBuilder, and background-thread I/O.AndroidLogger,FileLogStore,OneSignalLogHttpSender,AndroidLogCrashHandler,AndroidLogAnrDetector,LoggerPlatformProviderAdapter, composed viaLoggerPlatformFactory/LoggerLifecycleManager.otel:log(level-driven, carriesexception.*),saveCrash(Severity.FATAL) for real crashes and foreground ANRs,saveNonFatal(Severity.WARN) for backgrounded main-thread blocks,ossdk.crash.fatalOTLP bool attribute so the backend segments crash/ANR metrics on a stable flag rather than inferring intent from severity/exception type.AndroidLogAnrDetectorreuses the shared, JVM-testedAnrCheckEvaluator— the same decision coreOtelAnrDetectoruses, so the two can't drift. Foreground block > threshold → fatal ANR viasaveCrash; backgrounded block > (higher) threshold → non-fatal warning viasaveNonFatalunder a distinctBackgroundMainThreadBlockException+ stack fingerprint; watchdog-oversleep (Doze/frozen process) is suppressed.isAppInForegroundis sourced from the platform provider'sappState.LoggerModuleSwitchgatesotelvs the newloggermodule.project-setup,e2e,codeql,publish-release).bump-kmp-submodule.yml(workflow_call) that the KMP repo's Release workflow calls to re-point the submodule to a released tag and open a bump PR here — authenticated with the shared orgGH_PUSH_TOKEN(mirrors the OneSignal/sdk-shared pattern; no GitHub App).CONTRIBUTING.md+README.mddocument cloning with--recurse-submodules.Related / dependencies
@Throws,ILogFileStore.save(): Boolean, and restoresservice.name/scope. The Android adapters here are written against that contract (crash saves are synchronous, norunBlocking).feat/anr-app-state-and-fatal-flag(stacked on onPause NPE #3) — addssaveNonFatal+ossdk.crash.fatal+ bool-attribute encoding. The submodule here pins that branch's commit9cf1a33.GH_PUSH_TOKENis already granted to the KMP repo.Notes for reviewers
git submodule update --init --recursive(CI updated; documented in CONTRIBUTING).9cf1a33, a commit on the unmerged KMP branchfeat/anr-app-state-and-fatal-flag(stacked on unmerged onPause NPE #3). Release ordering before this can ship:main,feat/anr-app-state-and-fatal-flag→main,FileLogStore,OneSignalLogHttpSender,AndroidLog*,LoggerLifecycleManager) are intentional: some replace functionality the OTel libraries provided (HTTP export, disk buffering), the rest are parallelILog*-based analogues of theIOtel*classes so both stacks can coexist behind the feature flag during migration. They collapse into a single stack onceotelis removed.Test plan
./gradlew :logger:testDebugUnitTest :logger:iosSimulatorArm64Test spotlessCheck(green)../gradlew :OneSignal:core:compileDebugKotlin+ crash-package unit tests (com.onesignal.debug.internal.crash.*) green against the submodule.LoggerModuleSwitchenabled; confirm logs + a forced crash upload reach the backend (verified previously on production).ossdk.crash.fatal=false.Made with Cursor