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

caldrian · 2025-11-06T14:52:34Z

WPB-14297 Reduce number of logs in production

Issue

This is the second PR in a series,

the previous one is: chore: rename WireLogging to WireLegacyLogging - WPB-14876 #3813
the next one is: refactor: logging using string interpolation - WPB-14876 #2480

This PR creates a new interface for logging, which forces calling code to explicitly specify how types are converted to a string.

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.

github-actions · 2025-11-06T16:04:07Z

Test Results

1 902 tests 1 875 ✅ 2m 16s ⏱️
304 suites 27 💤
3 files 0 ❌

Results for commit a52d55e.

♻️ This comment has been updated with latest results.

…' into feat/create-new-logger-WPB-14297

Copilot

Pull request overview

This PR introduces a new type-safe logging API (WireLogging) that prevents accidental logging of sensitive data by restricting automatic string interpolation to StaticString values only. The design forces developers to explicitly define how custom types are converted to log messages through WireLogInterpolation extensions.

Key changes:

New logging infrastructure with WireLogMessage, WireLogInterpolation, and WireLogAttribute for structured, secure logging
WireTaggedLogger and protocol-based API with log level methods (debug, info, notice, warn, error, critical)
OSLogHandler implementation with logger caching and tag-based categorization

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
WireLogInterpolationTests.swift	Basic tests for static string interpolation
sourcery.yml	Configuration update to import WireLogging in generated mocks
AutoMockable.manual.swift	Manual mock for WireTaggedLoggerProtocol
WireTaggedLoggerProtocol.swift	Protocol defining tagged logger interface
WireTaggedLogger.swift	Tagged logger implementation with log level methods
WireLogger+Singletons.swift	Static logger instances initialized with shared handler
WireLogType.swift	Enum defining log levels (renamed from WireLogLevel)
WireLogTag.swift	String-based tag type for categorizing logs
WireLogMessage.swift	Log message type supporting custom interpolation
WireLogInterpolation.swift	Custom string interpolation restricting to StaticString by default
WireLogAttribute.swift	Key-value pair for structured log metadata
WireLogAttribute+predefined.swift	Predefined attribute helpers
WireLogHandlerProtocol.swift	Protocol for log handlers
OSLogHandler.swift	os.Logger-based handler with caching
Documentation.md	Comprehensive documentation for the logging API
WireLogger+Instances.swift	Minor formatting change in legacy logger
Package.swift	Added WireLogging dependency to WireLegacyLogging

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

Copilot · 2025-11-28T11:13:21Z

WireLogging/Sources/WireLogging/LogHandler/OSLogHandler/OSLogHandler.swift

+
+        func logger(for tag: WireLogTag) -> Logger {
+            // Synchronously get or create logger and update access time
+            let logger = queue.sync {


Every log call triggers an async eviction task (line 107-109), which creates many unnecessary async dispatches. Consider adding a throttling mechanism (e.g., only evict every N calls or after a time interval) or moving eviction to a background timer to reduce overhead.

Maybe I can replace it with locking.

Copilot · 2025-11-28T11:13:21Z

WireLogging/Sources/WireLogging/LogHandler/OSLogHandler/OSLogHandler.swift

+            queue.async {
+                self.evictStaleEntries()
+            }


Every log call triggers an async eviction task (line 107-109), which creates many unnecessary async dispatches. Consider adding a throttling mechanism (e.g., only evict every N calls or after a time interval) or moving eviction to a background timer to reduce overhead.

A valid point, I'll think about it.

Copilot · 2025-11-28T11:13:21Z

WireLogging/Sources/WireLogging/LogHandler/OSLogHandler/OSLogHandler.swift

+        var attributesString = ""
+        for attributesKey in attributes.keys.sorted() {
+            attributesString += "[\(attributesKey)=\(attributes[attributesKey]!)]"
+        }
+


String concatenation in a loop (line 46) creates multiple intermediate String objects. Use a String builder pattern or join the array for better performance, especially when many attributes are present.

Suggested change

var attributesString = ""

for attributesKey in attributes.keys.sorted() {

attributesString += "[\(attributesKey)=\(attributes[attributesKey]!)]"

}

let attributesString = attributes.keys.sorted()

.map { "[\($0)=\(attributes[$0]!)]" }

.joined()

Copilot · 2025-11-28T11:13:21Z

WireLogging/Tests/WireLoggingTests/WireLogMessage/WireLogInterpolationTests.swift

+    @Test
+    func staticStringIsNotObfuscated() async throws {


The new logging infrastructure lacks comprehensive test coverage. Missing tests include: WireLogAttribute handling, OSLogHandler behavior, logger caching in OSLogHandler, WireTaggedLogger methods, and the setup/singleton pattern in WireLogger. Consider adding tests for these critical components.

Copilot · 2025-11-28T11:13:22Z

WireLogging/Tests/WireLoggingTests/WireLogMessage/WireLogInterpolationTests.swift

+    @Test
+    func staticStringInterpolationIsNotObfuscated() async throws {


The new logging infrastructure lacks comprehensive test coverage. Missing tests include: WireLogAttribute handling, OSLogHandler behavior, logger caching in OSLogHandler, WireTaggedLogger methods, and the setup/singleton pattern in WireLogger. Consider adding tests for these critical components.

Copilot · 2025-11-28T11:13:22Z

WireLogging/Sources/WireLogging/TaggedLogger/WireLogger+Singletons.swift

+
+    public static let analytics = WireTaggedLogger(tag: "analytics", handler: logHandler)
+    public static let apiMigration = WireTaggedLogger(tag: "api-migration", handler: logHandler)
+    public static let appVersionMigration = WireTaggedLogger(tag: "api-version-migration", handler: logHandler)


The tag "api-version-migration" does not match the logger name appVersionMigration. This inconsistency could cause confusion. Consider renaming the tag to "app-version-migration" for consistency.

Suggested change

public static let appVersionMigration = WireTaggedLogger(tag: "api-version-migration", handler: logHandler)

public static let appVersionMigration = WireTaggedLogger(tag: "app-version-migration", handler: logHandler)

Copilot · 2025-11-28T11:13:22Z

WireLogging/Sources/WireLogging/LogMessage/WireLogInterpolation.swift

+    /// ```
+
+    public mutating func writeAttribute(_ attribute: WireLogAttribute) {
+        attributes += [attribute]


Using += with a single-element array creates unnecessary array allocations. Use attributes.append(attribute) for better performance.

Suggested change

attributes += [attribute]

attributes.append(attribute)

netbe

left a couple of questions before approving

netbe · 2025-12-15T08:53:00Z

WireLogging/Sources/WireLogging/Documentation.docc/Documentation.md

@@ -0,0 +1,86 @@
+# ``WireLogging``


praise: really clear documentation 🙏

netbe · 2025-12-15T09:05:20Z

WireLogging/Sources/WireLogging/TaggedLogger/WireLogger+Singletons.swift

+    /// This method must be called very early on app start, before any other interaction with ``WireLogger``.
+
+    public static func setup(_ logHandler: any WireLogHandlerProtocol) {
+        precondition(Thread.isMainThread)


question: why is this check necessary?

netbe · 2025-12-15T09:06:05Z

WireLogging/Sources/WireLogging/TaggedLogger/WireLogger+Singletons.swift

+    public static func setup(_ logHandler: any WireLogHandlerProtocol) {
+        precondition(Thread.isMainThread)
+        guard self.logHandler == nil else {
+            fatalError("WireLogger is already set up")


question: should we just return ? or be assertionFailure?

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

WireLogging/Sources/WireLogging/LogHandler/OSLogHandler/OSLogHandler.swift

+//
+
+import Foundation
+import os


question: So this is only the osLog implementation given by Apple right? What about Cocoalumberjack (to write to filesystem)?

create new logger API

41df6da

github-actions bot assigned caldrian Nov 6, 2025

caldrian added 8 commits November 6, 2025 17:30

format code

615eed0

add documentation

91d5c8a

cleanup

7afd857

cache os.Logger instances

14044fa

add explanation

1c8fa7d

add empty lines

2d83cfd

predefined attributes

abdbe34

sort

6061f61

caldrian marked this pull request as draft November 7, 2025 12:30

caldrian added 2 commits November 7, 2025 13:38

use UUID for selfUserID attribute

f4c7f69

adjust docs

9687c31

caldrian marked this pull request as ready for review November 7, 2025 12:40

caldrian added 4 commits November 10, 2025 17:26

Merge branch 'chore/rename-wirelogging-to-wirelegacylogging-WPB-14876…

6ec2355

…' into feat/create-new-logger-WPB-14297

Merge branch 'chore/rename-wirelogging-to-wirelegacylogging-WPB-14876…

b05277a

…' into feat/create-new-logger-WPB-14297

Merge branch 'chore/rename-wirelogging-to-wirelegacylogging-WPB-14876…

560e6c8

…' into feat/create-new-logger-WPB-14297

separate log attributes

8918320

caldrian mentioned this pull request Nov 28, 2025

chore: rename WireLogging to WireLegacyLogging - WPB-14876 #3813

Open

6 tasks

Merge branch 'chore/rename-wirelogging-to-wirelegacylogging-WPB-14876…

898ced4

…' into feat/create-new-logger-WPB-14297

caldrian changed the title ~~feat: create new logger API - WPB-14297~~ feat: create new logger API - WPB-14297x Nov 28, 2025

caldrian changed the title ~~feat: create new logger API - WPB-14297x~~ feat: create new logger API - WPB-14297 Nov 28, 2025

caldrian mentioned this pull request Nov 28, 2025

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

Open

6 tasks

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

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

Copilot AI reviewed Nov 28, 2025

View reviewed changes

a few fixes

a52d55e

wireapp deleted a comment from Copilot AI Nov 28, 2025

netbe reviewed Dec 15, 2025

View reviewed changes

		@Test
		func staticStringInterpolationIsNotObfuscated() async throws {

	public static let appVersionMigration = WireTaggedLogger(tag: "api-version-migration", handler: logHandler)
	public static let appVersionMigration = WireTaggedLogger(tag: "app-version-migration", handler: logHandler)

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

caldrian commented Nov 6, 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!

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

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

caldrian 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!

caldrian 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

caldrian commented Nov 6, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Nov 6, 2025 •

edited

Loading