From 1c984d54f492c79035e8d675fbc4e1929fef61fa Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Sat, 2 May 2026 13:53:19 -0500 Subject: [PATCH] chore: Integrate MQTT logging with Kermit and enhance PII sanitization (#5338) --- .../app/analytics/FdroidPlatformAnalytics.kt | 17 +++--- .../app/analytics/GooglePlatformAnalytics.kt | 8 ++- .../network/repository/KermitMqttLogger.kt | 48 +++++++++++++++++ .../network/repository/MQTTRepositoryImpl.kt | 54 ++++++++++++++++--- 4 files changed, 108 insertions(+), 19 deletions(-) create mode 100644 core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/KermitMqttLogger.kt diff --git a/app/src/fdroid/kotlin/org/meshtastic/app/analytics/FdroidPlatformAnalytics.kt b/app/src/fdroid/kotlin/org/meshtastic/app/analytics/FdroidPlatformAnalytics.kt index 0432560e5..5faa8e0e8 100644 --- a/app/src/fdroid/kotlin/org/meshtastic/app/analytics/FdroidPlatformAnalytics.kt +++ b/app/src/fdroid/kotlin/org/meshtastic/app/analytics/FdroidPlatformAnalytics.kt @@ -31,26 +31,21 @@ import org.meshtastic.core.repository.PlatformAnalytics class FdroidPlatformAnalytics : PlatformAnalytics { init { // For F-Droid builds we don't initialize external analytics services. - // In debug builds we attach a DebugTree for convenient local logging, but - // release builds rely on system logging only. - if (BuildConfig.DEBUG) { - Logger.setMinSeverity(Severity.Debug) - Logger.i { "F-Droid platform no-op analytics initialized (Debug mode)." } - } else { - Logger.setMinSeverity(Severity.Info) - Logger.i { "F-Droid platform no-op analytics initialized." } + // Configure Kermit logging: Debug level for debug builds, Info level for release builds. + Logger.setMinSeverity(if (BuildConfig.DEBUG) Severity.Debug else Severity.Info) + Logger.i { + "F-Droid platform no-op analytics initialized (${if (BuildConfig.DEBUG) "Debug" else "Info"} logging)." } } override fun setDeviceAttributes(firmwareVersion: String, model: String) { - // No-op for F-Droid - Logger.d { "Set device attributes called: firmwareVersion=$firmwareVersion, deviceHardware=$model" } + if (BuildConfig.DEBUG) Logger.d { "setDeviceAttributes: firmwareVersion=$firmwareVersion, model=$model" } } override val isPlatformServicesAvailable: Boolean get() = false override fun track(event: String, vararg properties: DataPair) { - Logger.d { "Track called: event=$event, properties=${properties.toList()}" } + // No-op for F-Droid } } diff --git a/app/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt b/app/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt index 0deea7aeb..26107cb95 100644 --- a/app/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt +++ b/app/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt @@ -343,7 +343,13 @@ class GooglePlatformAnalytics(private val context: Context, private val analytic // Explicitly handle String else -> bundle.putString(it.name, value.toString()) // Fallback for other types } - KermitLogger.withTag(TAG).d { "Analytics: track $event (${it.name} : $value)" } + KermitLogger.withTag(TAG).d { + if (BuildConfig.DEBUG) { + "Analytics: track $event (${it.name} : $value)" + } else { + "Analytics: track $event (${it.name})" + } + } } Firebase.analytics.logEvent(event, bundle) } diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/KermitMqttLogger.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/KermitMqttLogger.kt new file mode 100644 index 000000000..8bd2cbda5 --- /dev/null +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/KermitMqttLogger.kt @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.network.repository + +import co.touchlab.kermit.Logger +import org.meshtastic.mqtt.MqttLogLevel +import org.meshtastic.mqtt.MqttLogger + +/** + * Adapter that implements [MqttLogger] to send MQTT client logs to Kermit's [Logger]. + * + * This allows the MQTTastic library logging to integrate with the app's logging infrastructure, including any + * configured log sinks (Crashlytics, Datadog, etc.). + * + * The library's [tag] (e.g. "MqttClient", "MqttConnection") is forwarded as a structured Kermit tag so that Datadog + * receives it as an indexed attribute rather than freetext in the message body, enabling per-component filtering in log + * queries. + * + * Note: The production log level should be set to [MqttLogLevel.WARN] (not INFO) to prevent the library's own + * INFO-level messages (which include endpoint addresses and topic strings) from reaching remote analytics sinks. + */ +class KermitMqttLogger : MqttLogger { + override fun log(level: MqttLogLevel, tag: String, message: String, throwable: Throwable?) { + val logger = Logger.withTag(tag) + when (level) { + MqttLogLevel.TRACE -> logger.v(throwable) { message } + MqttLogLevel.DEBUG -> logger.d(throwable) { message } + MqttLogLevel.INFO -> logger.i(throwable) { message } + MqttLogLevel.WARN -> logger.w(throwable) { message } + MqttLogLevel.ERROR -> logger.e(throwable) { message } + MqttLogLevel.NONE -> return + } + } +} diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt index 5ea482624..9838af9a7 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt @@ -47,6 +47,7 @@ import org.meshtastic.mqtt.ConnectionState import org.meshtastic.mqtt.MqttClient import org.meshtastic.mqtt.MqttEndpoint import org.meshtastic.mqtt.MqttException +import org.meshtastic.mqtt.MqttLogLevel import org.meshtastic.mqtt.MqttMessage import org.meshtastic.mqtt.QoS import org.meshtastic.mqtt.packet.Subscription @@ -57,6 +58,7 @@ import kotlin.concurrent.Volatile class MQTTRepositoryImpl( private val radioConfigRepository: RadioConfigRepository, private val nodeRepository: NodeRepository, + private val buildConfigProvider: org.meshtastic.core.common.BuildConfigProvider, dispatchers: CoroutineDispatchers, ) : MQTTRepository { @@ -109,6 +111,11 @@ class MQTTRepositoryImpl( autoReconnect = true username = mqttConfig?.username mqttConfig?.password?.let { password(it) } + logger = KermitMqttLogger() + // WARN for production: the library emits endpoint addresses and topic strings at + // INFO level. WARN messages (reconnect, timeout, retry) contain no PII and are + // exactly the signals needed for production diagnostics. + logLevel = if (buildConfigProvider.isDebug) MqttLogLevel.DEBUG else MqttLogLevel.WARN } client = newClient @@ -139,7 +146,27 @@ class MQTTRepositoryImpl( launch { newClient.messages.collect { msg -> processMessage(msg) } } // Forward the client's connection state to the repo-level StateFlow for UI observation. - launch { newClient.connectionState.collect { _connectionState.value = it } } + // Also emit structured log messages on transitions so reconnect attempt counts and + // disconnect reason codes are visible in Crashlytics/Datadog without any PII. + launch { + newClient.connectionState.collect { state -> + _connectionState.value = state + when (state) { + ConnectionState.Connecting -> Logger.i { "MQTT connecting" } + + ConnectionState.Connected -> Logger.i { "MQTT connected" } + + is ConnectionState.Reconnecting -> { + val errorDetail = state.lastError?.message?.let { ": $it" } ?: "" + Logger.w { "MQTT reconnecting (attempt ${state.attempt}$errorDetail)" } + } + + is ConnectionState.Disconnected -> { + state.reason?.let { Logger.w { "MQTT disconnected: ${it.message}" } } + } + } + } + } // Retry the initial connect with exponential backoff. Once established, // autoReconnect handles subsequent drops and re-subscribes internally. @@ -147,7 +174,9 @@ class MQTTRepositoryImpl( var reconnectDelay = INITIAL_RECONNECT_DELAY_MS while (true) { val result = safeCatching { - Logger.i { "MQTT Connecting to $endpoint" } + Logger.i { + if (buildConfigProvider.isDebug) "MQTT Connecting to $endpoint" else "MQTT Connecting..." + } newClient.connect(endpoint) if (subscriptions.isNotEmpty()) { Logger.d { "MQTT subscribing to ${subscriptions.size} topics" } @@ -181,13 +210,11 @@ class MQTTRepositoryImpl( private fun ProducerScope.processMessage(msg: MqttMessage) { val topic = msg.topic val payload = msg.payload.toByteArray() - Logger.d { "MQTT received message on topic $topic (size: ${payload.size} bytes)" } if (topic.contains("/json/")) { try { val jsonStr = payload.decodeToString() json.decodeFromString(jsonStr) - Logger.d { "MQTT parsed JSON payload successfully" } trySend(MqttClientProxyMessage(topic = topic, text = jsonStr, retained = msg.retain)) } catch (e: JsonDecodingException) { Logger.e(e) { "Failed to parse MQTT JSON: ${e.shortMessage} (path: ${e.path})" } @@ -204,10 +231,15 @@ class MQTTRepositoryImpl( override fun publish(topic: String, data: ByteArray, retained: Boolean) { val currentClient = client if (currentClient == null) { - Logger.w { "MQTT publish to $topic dropped: client not connected" } + Logger.w { + if (buildConfigProvider.isDebug) { + "MQTT publish to $topic dropped: client not connected" + } else { + "MQTT publish dropped: client not connected" + } + } return } - Logger.d { "MQTT publishing message to topic $topic (size: ${data.size} bytes, retained: $retained)" } scope.launch { publishSemaphore.withPermit { safeCatching { @@ -215,7 +247,15 @@ class MQTTRepositoryImpl( MqttMessage(topic = topic, payload = data, qos = QoS.AT_LEAST_ONCE, retain = retained), ) } - .onFailure { e -> Logger.w(e) { "MQTT publish to $topic failed" } } + .onFailure { e -> + Logger.w(e) { + if (buildConfigProvider.isDebug) { + "MQTT publish to $topic failed" + } else { + "MQTT publish (${data.size} bytes) failed" + } + } + } } } }