refactor: logging using string interpolation - WPB-14876 #2480

caldrian · 2025-02-05T09:25:17Z

WPB-14876 [iOS] Create a new WireLogger interface

Issue

This is the third PR in a series,

the previous one is: feat: create new logger API - WPB-14297 #3849

This PR adjusts WireBackup to use the new logging module.

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

Make sure you use the API for UI elements that support large fonts.
All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
New UI elements have Accessibility strings for VoiceOver.

…o chore/move-WireLogger-into-WireFoundation-WPB-10941

…abstract-logging

…o chore/abstract-logging

…10 (#2207)

…o chore/move-WireLogger-into-WireFoundation-WPB-10941

…o chore/abstract-logging

# Conflicts: # WireDomain/Sources/WireDomain/Repositories/UserClients/Models/UserClientInfo.swift # WireFoundation/Tests/WireLoggingTests/PlaceholderTests.swift # wire-ios-system/Source/Logging/WireLogger.swift # wire-ios/Wire-iOS Tests/WireAnalytics_DatadogTests.swift # wire-ios/Wire-iOS.xcodeproj/project.pbxproj # wire-ios/WireCommonComponents/Analytics/WireAnalytics.swift # wire-ios/WireCommonComponents/Logging/CocoaLumberjackLogger.swift

…stract-logging

…ract-logging

…ging-using-string-interpolation-WPB-14297

…ging-using-string-interpolation-WPB-14297 # Conflicts: # WireAnalytics/Package.swift # WireNetwork/Package.swift # WireUI/Sources/WireSettingsUI/Account/BackupImportExport/BackupImportExportBuilder.swift # wire-ios/Wire-iOS/Sources/UserInterface/Settings/CellDescriptors/SettingsCellDescriptorFactory+Account.swift

…ging-using-string-interpolation-WPB-14297 # Conflicts: # WireLogging/Sources/WireLogging/LogMessage/WireLogAttribute.swift

…ging-using-string-interpolation-WPB-14297

Copilot

Pull request overview

This PR continues the migration from the legacy logging system to the new WireLogger interface with string interpolation support, specifically refactoring the WireBackup module and related components. The changes introduce safer logging practices through explicit safePublic labels and custom error interpolation to prevent accidental exposure of sensitive data.

Key Changes

Migrated WireBackup module (CreateBackupUseCase, ImportBackupUseCase, BackupLocalStore) to use new WireTaggedLogger with string interpolation
Introduced BackupProgress struct to encapsulate progress values, improving type safety
Added custom WireLogInterpolation extensions for BackupProgress, Error types, and basic types requiring explicit safePublic labeling
Removed WireLegacyLogging dependency from WireBackup, WireAnalytics, and WireDatadog packages

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
WireLogging/Sources/WireLogging/TaggedLogger/WireLogger+Singletons.swift	Added WireLogger singleton instances and setup mechanism with lazy initialization
WireLogging/Sources/WireLogging/TaggedLogger/WireTaggedLogger.swift	Enhanced WireTaggedLogger with additionalAttributes support
WireLogging/Sources/WireLogging/LogMessage/WireLogInterpolation+*.swift	Added interpolation extensions for errors, basic types, and obfuscation control
WireBackup/Sources/WireBackup/UseCases/*.swift	Migrated backup use cases to use WireTaggedLogger with new logging patterns
WireBackup/Sources/WireBackup/Extensions/WireLogging/*.swift	Added WireLogger extensions for createBackup and importBackup tags
WireFoundation/Sources/WireFoundation/Protocols/BackupRestore/*.swift	Introduced BackupProgress struct and updated progress enums to use it
wire-ios/WireCommonComponents/Analytics/*.swift	Setup new multiplex log handler combining OSLog, CocoaLumberjack, and Datadog
wire-ios/WireCommonComponents/Logging/CocoaLumberjackLogger.swift	Added NewCocoaLumberjackLogger adapter for new logging interface
wire-ios/Wire-iOS/Sources/UserInterface/Settings/*.swift	Updated to use new logger singleton names (createBackup, importBackup)
WireUI/Sources/WireSettingsUI/Account/BackupImportExport/*.swift	Migrated UI components to WireTaggedLoggerProtocol
WireAnalytics/Sources/WireDatadog/*.swift	Renamed WireLogLevel to WireLogType for consistency
WireNetwork/Sources/WireNetwork/Extensions/*.swift	Added WireLogAttribute extensions for push channel versions
WireUI/Package.swift	Added WireLogging and WireLoggingSupport dependencies
WireBackup/Package.swift	Removed WireLegacyLogging dependency
WireAnalytics/Package.swift	Removed WireLegacyLogging dependency

Comments suppressed due to low confidence (2)

WireLogging/Sources/WireLogging/TaggedLogger/WireTaggedLogger.swift:50

The additionalAttributes property is documented (lines 24-25) as having the least priority and being merged with attributes passed to log(). However, the log() method (lines 39-50) only passes the method parameter's additionalAttributes to the handler, ignoring the instance property self.additionalAttributes. The instance property should be merged with the method parameter before passing to the handler, with the method parameter taking precedence for duplicate keys as documented.

    private func log(
        _ type: WireLogType,
        _ message: WireLogMessage,
        _ additionalAttributes: [WireLogAttribute]
    ) {
        handler.log(
            tag: tag,
            type: type,
            message: message,
            additionalAttributes: additionalAttributes
        )
    }

WireBackup/Sources/WireBackup/Extensions/WireLogging/WireLogger+instances.swift:25

These static logger properties directly access logHandler during initialization. If these properties are accessed before WireLogger.setup() is called, logHandler will be nil, causing a runtime crash. Consider adding a precondition check in the private WireTaggedLogger() helper function in WireLogger+Singletons.swift or making these properties lazy to ensure they're only created after setup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-28T11:16:58Z

WireFoundation/Sources/WireFoundation/Protocols/BackupRestore/CreateBackupProgress.swift

    static func progress(_ current: Int, _ total: Int) -> Self {
        .progress(current: current, total: total)
    }



The two progress convenience methods are duplicates with different parameter labels. Line 32-33 uses unlabeled parameters (_:_:), while line 36-37 uses named parameters (current:total:), but both create the same BackupProgress instance. This creates ambiguity for callers and is redundant. Consider keeping only one variant, preferably the one with named parameters for better code clarity.

Suggested change

static func progress(_ current: Int, _ total: Int) -> Self {

.progress(current: current, total: total)

}

Copilot · 2025-11-28T11:16:59Z

WireAnalytics/Sources/WireDatadog/WireDatadog.swift

        finalAttributes["version"] = buildVersion

-        let level = level.mapToDatadogLogLevel()
+        let level = logType.mapToDatadogLogLevel()


The log method receives a type parameter (line 110) but on line 119 it uses the instance property logType instead. This means all log messages will always use the same level (.debug) regardless of what was passed as the type parameter. This should be let level = type.mapToDatadogLogLevel().

Suggested change

let level = logType.mapToDatadogLogLevel()

let level = type.mapToDatadogLogLevel()

Copilot · 2025-11-28T11:16:59Z

wire-ios/WireCommonComponents/Analytics/Datadog/WireDatadog+LoggerProtocol.swift

+
+struct NewWireDatadogLogger: WireLogHandlerProtocol {
+
+    let additionalAttributes = LogAttributes()


The additionalAttributes property is declared but never used in the NewWireDatadogLogger struct. It should either be removed or integrated into the logging logic if it was intended to provide default attributes for all log messages.

Copilot · 2025-11-28T11:17:00Z

WireNetwork/Sources/WireNetwork/Extensions/WireLogAttribute+pushChannel.swift

+
+    static var pushChannelV1: WireLogAttribute { .init("push_channel", "v1") }
+
+    /// PushChannelV2 (consumable notifications sync)


Inconsistent spacing in documentation comments. Line 31 has "PushChannelV2" without spacing, while lines 23 and 27 use proper spacing ("Legacy pushChannel" and "PushChannel V1"). For consistency, line 31 should be "PushChannel V2" (with space before V2).

Suggested change

/// PushChannelV2 (consumable notifications sync)

/// PushChannel V2 (consumable notifications sync)

Copilot · 2025-11-28T11:17:00Z

wire-ios/WireCommonComponents/Logging/CocoaLumberjackLogger.swift

+    func log(
+        tag: WireLogTag,
+        type: WireLogType,
+        message: WireLogMessage,
+        additionalAttributes: [WireLogAttribute]
+    ) {
+        switch type {
+        case .debug:
+            logger.debug(message.content, attributes: [.tag: tag.rawValue])
+        case .info:
+            logger.info(message.content, attributes: [.tag: tag.rawValue])
+        case .notice:
+            logger.notice(message.content, attributes: [.tag: tag.rawValue])
+        case .warn:
+            logger.warn(message.content, attributes: [.tag: tag.rawValue])
+        case .error:
+            logger.error(message.content, attributes: [.tag: tag.rawValue])
+        case .critical:
+            logger.critical(message.content, attributes: [.tag: tag.rawValue])
+        }
+    }


The additionalAttributes parameter is not used in the log method. The method only passes the tag to the underlying logger but ignores any additional attributes that might be provided. If additional attributes should be logged, they need to be merged with the tag attribute and passed to the logger. Otherwise, this parameter should be documented as currently unused.

Copilot · 2025-11-28T11:17:00Z

WireFoundation/Sources/WireFoundation/Protocols/BackupRestore/ImportBackupProgress.swift

+    static func progress(_ current: Int, _ total: Int) -> Self {
+        .progress(current: current, total: total)
    }


The two progress convenience methods are duplicates with different parameter labels. Line 30-31 uses named parameters (current:total:), while line 34-35 uses unlabeled parameters (_:_:), but both call the same underlying method. This creates ambiguity for callers and is redundant. Consider keeping only one variant, preferably the one with named parameters for better code clarity.

Suggested change

static func progress(_ current: Int, _ total: Int) -> Self {

.progress(current: current, total: total)

}

Copilot · 2025-11-28T11:17:01Z

WireLogging/Tests/WireLoggingTests/WireLogMessage/WireLogInterpolation+anyErrorTests.swift

+        var sensibleInformation: String
+        init(stringLiteral value: StringLiteralType) {
+            sensibleInformation = value


The property name sensibleInformation appears to be a typo. It should likely be sensitiveInformation (meaning private/secret data) rather than sensibleInformation (meaning reasonable/practical).

Suggested change

var sensibleInformation: String

init(stringLiteral value: StringLiteralType) {

sensibleInformation = value

var sensitiveInformation: String

init(stringLiteral value: StringLiteralType) {

sensitiveInformation = value

netbe

left some questions

netbe · 2025-12-15T09:18:43Z

wire-ios/WireCommonComponents/Analytics/Datadog/WireDatadog+LoggerProtocol.swift

+            if let key = LogAttributesKey(rawValue: additionalAttribute.key) {
+                attributes[key] = additionalAttribute.value
+            } else {
+                assertionFailure(additionalAttribute.key)


suggestion:

Suggested change

assertionFailure(additionalAttribute.key)

assertionFailure("unsupported attribute: \(additionalAttribute.key)")

netbe · 2025-12-15T09:21:44Z

wire-ios/WireCommonComponents/Logging/CocoaLumberjackLogger.swift

 import WireLogging
 import WireSystem

+// TODO: the new implementation currently just wraps the old one, rewrite!


question: @caldrian do you plan to do it in this PR?

netbe · 2025-12-15T09:23:15Z

WireBackup/Sources/WireBackup/Extensions/WireLogging/WireLogger+instances.swift

+    static let createBackup = WireTaggedLogger(tag: "create-backup", handler: logHandler)
+
+    static let importBackup = WireTaggedLogger(tag: "import-backup", handler: logHandler)


question: so each framework we'll get different instances?

netbe · 2025-12-15T09:27:02Z

WireLogging/Tests/WireLoggingTests/WireLogMessage/WireLogInterpolation+anyErrorTests.swift

+
+@testable import WireLogging
+
+struct WireLogInterpolationAnyErrorTests {


praise: nice to have this tested

github-actions · 2026-01-15T00:38:12Z

This PR is stale because it has been open 30 days with no activity. Please update it or close it in case is not relevant anymore.

…ging-using-string-interpolation-WPB-14297 # Conflicts: # WireAnalytics/Sources/WireDatadog/WireDatadog.swift # WireAnalytics/Sources/WireDatadog/WireFakeDatadog.swift # WireLogging/Sources/WireLogging/TaggedLogger/WireLogger+Singletons.swift # WireUI/Sources/WireSettingsUI/Account/BackupImportExport/Import/ImportBackupViewModel.swift

caldrian and others added 30 commits November 25, 2024 17:16

Merge branch 'release/cycle-3.114' of github.com:wireapp/wire-ios int…

12a4c89

…o chore/move-WireLogger-into-WireFoundation-WPB-10941

brainstorming

bbbf99f

WireAnalytics.setup()

1ad532d

add Sendable conformance

95a3ff9

Merge branch 'fix/make-WireLogger-Swift6-ready-WPB-11890' into chore/…

0b6fa8b

…abstract-logging

create targets WireLogging etc.

069d93b

move file

6324a5d

Merge branch 'release/cycle-3.114' of github.com:wireapp/wire-ios int…

674d335

…o chore/abstract-logging

attempt to extend WireLogInterpolation from outside

488c20b

fix: Crash unwrapping MainCoordinator.splitViewController - WPB-145…

30efda2

…10 (#2207)

fix: Profile color tappable area - WPB-14525 (#2208)

c365522

chore: delete redundant LICENSE and README files - WPB-14567 (#2214)

272750f

fix: back button style - WPB-14478 (#2213)

c58319e

chore: bump avs to 9.10.16 - WPB-11904 (#2118)

805268b

feat: Add scroll view to the passcode setup view - WPB-11836 (#2211)

fd6d0d3

Merge branch 'release/cycle-3.114' of github.com:wireapp/wire-ios int…

96e3350

…o chore/move-WireLogger-into-WireFoundation-WPB-10941

fix: remove debug logging of credentials - WPB-14251 (#2205)

73c5368

add missing imports

ecb13d4

chore: unify schemes and test plans - WPB-11406 (#2217)

d2f00e8

fix: crash when registering with a registered email - WPB-14566 (#2223)

ae81f4e

Merge branch 'release/cycle-3.114' of github.com:wireapp/wire-ios int…

e16329f

…o chore/abstract-logging

add notes

b066604

Merge commit 'b0666045b3a660df08cc667bb0b5497154303bc2' into chore/ab…

033c4b5

…stract-logging

fix merge conflicts

321bc1b

add debug output

77e0625

Merge branch 'develop' of github.com:wireapp/wire-ios into chore/abst…

a2eb720

…ract-logging

some progress

4c87e8b

restructuring

2c1e835

StaticString

78f9d80

caldrian marked this pull request as ready for review November 7, 2025 14:41

caldrian added 15 commits November 10, 2025 17:27

Merge branch 'feat/create-new-logger-WPB-14297' into refactor/WireLog…

907ec18

…ging-using-string-interpolation-WPB-14297

SafePublicLoggable

b090ed4

revert WireNetwork changes

54f7bcc

fix build errors in WireBackup

93ae00b

fix build errors

6e31998

refactoring

64200e9

log process id and name with system tag

8adb34c

refactoring

b30475f

minor corrections

4b716b0

more cleanup

30ccb08

add tests

c67f04c

Merge branch 'feat/create-new-logger-WPB-14297' into refactor/WireLog…

fbe4a5b

…ging-using-string-interpolation-WPB-14297 # Conflicts: # WireLogging/Sources/WireLogging/LogMessage/WireLogAttribute.swift

cleanup

952bd01

minor fix

8fb7f53

caldrian mentioned this pull request Nov 28, 2025

feat: create new logger API - WPB-14297 #3849

Open

6 tasks

caldrian changed the title ~~refactor: logging using string interpolation - WPB-14876~~ refactor: logging using string interpolation - WPB-14876x Nov 28, 2025

caldrian changed the title ~~refactor: logging using string interpolation - WPB-14876x~~ refactor: logging using string interpolation - WPB-14876 Nov 28, 2025

caldrian requested review from Copilot and netbe November 28, 2025 11:10

Copilot started reviewing on behalf of caldrian November 28, 2025 11:10 View session

Merge branch 'feat/create-new-logger-WPB-14297' into refactor/WireLog…

c8774a7

…ging-using-string-interpolation-WPB-14297

Copilot finished reviewing on behalf of caldrian November 28, 2025 11:15

Copilot AI reviewed Nov 28, 2025

View reviewed changes

netbe reviewed Dec 15, 2025

View reviewed changes

github-actions bot added the stale label Jan 15, 2026

caldrian marked this pull request as draft January 19, 2026 09:07

	static func progress(_ current: Int, _ total: Int) -> Self {
	.progress(current: current, total: total)
	}

	let level = logType.mapToDatadogLogLevel()
	let level = type.mapToDatadogLogLevel()


		struct NewWireDatadogLogger: WireLogHandlerProtocol {

		let additionalAttributes = LogAttributes()


		static var pushChannelV1: WireLogAttribute { .init("push_channel", "v1") }

		/// PushChannelV2 (consumable notifications sync)

	/// PushChannelV2 (consumable notifications sync)
	/// PushChannel V2 (consumable notifications sync)

	assertionFailure(additionalAttribute.key)
	assertionFailure("unsupported attribute: \(additionalAttribute.key)")

		static let createBackup = WireTaggedLogger(tag: "create-backup", handler: logHandler)

		static let importBackup = WireTaggedLogger(tag: "import-backup", handler: logHandler)


		@testable import WireLogging

		struct WireLogInterpolationAnyErrorTests {

refactor: logging using string interpolation - WPB-14876 #2480

Are you sure you want to change the base?

refactor: logging using string interpolation - WPB-14876 #2480

Uh oh!

Conversation

caldrian commented Feb 5, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Checklist

UI accessibility checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

netbe left a comment

Choose a reason for hiding this comment

Uh oh!

netbe Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

netbe Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

netbe Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

netbe Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

caldrian commented Feb 5, 2025 •

edited by github-actions bot

Loading