From 3d315981bcd1d27818df39e80de2ed91782673ee Mon Sep 17 00:00:00 2001 From: Ariel Demarco Date: Thu, 20 Feb 2025 15:08:12 -0300 Subject: [PATCH 1/6] Finished functionallity + tests regarding log stacktrace behavior --- .../Enums/StackTraceBehavior.swift | 14 ---- .../EmbraceStackTraceError.swift | 38 +++++++++++ .../Models/EmbraceStackTrace.swift | 56 +++++++++++++++ .../{Enums => Models}/LogSeverity.swift | 0 .../{Enums => Models}/SessionState.swift | 0 .../Models/StackTraceBehavior.swift | 30 ++++++++ .../Internal/Logs/LogController.swift | 13 +++- .../Internal/Logs/LogControllerTests.swift | 68 ++++++++++++++++++- 8 files changed, 200 insertions(+), 19 deletions(-) delete mode 100644 Sources/EmbraceCommonInternal/Enums/StackTraceBehavior.swift create mode 100644 Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift create mode 100644 Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift rename Sources/EmbraceCommonInternal/{Enums => Models}/LogSeverity.swift (100%) rename Sources/EmbraceCommonInternal/{Enums => Models}/SessionState.swift (100%) create mode 100644 Sources/EmbraceCommonInternal/Models/StackTraceBehavior.swift diff --git a/Sources/EmbraceCommonInternal/Enums/StackTraceBehavior.swift b/Sources/EmbraceCommonInternal/Enums/StackTraceBehavior.swift deleted file mode 100644 index dffa4362..00000000 --- a/Sources/EmbraceCommonInternal/Enums/StackTraceBehavior.swift +++ /dev/null @@ -1,14 +0,0 @@ -// -// Copyright © 2024 Embrace Mobile, Inc. All rights reserved. -// - -import Foundation - -/// Describes the behavior for automatically capturing stack traces. -public enum StackTraceBehavior: Int { - /// Stack traces are not automatically captured. - case notIncluded - - /// The default behavior for automatically capturing stack traces. - case `default` -} diff --git a/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift b/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift new file mode 100644 index 00000000..b434ce4c --- /dev/null +++ b/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift @@ -0,0 +1,38 @@ +// +// Copyright © 2025 Embrace Mobile, Inc. All rights reserved. +// + +import Foundation + +public enum EmbraceStackTraceError: Error { + case invalidFormat +} + +extension EmbraceStackTraceError: LocalizedError, CustomNSError { + + public static var errorDomain: String { + return "Embrace" + } + + public var errorCode: Int { + switch self { + case .invalidFormat: + return -1 + } + } + + public var errorDescription: String? { + switch self { + case .invalidFormat: + return """ + Invalid stack trace format. Each frame should follow this format: + [ + ] + The "+ " part is optional. + """ + } + } + + public var localizedDescription: String { + return self.errorDescription ?? "No Matching Error" + } +} diff --git a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift new file mode 100644 index 00000000..dbc8357c --- /dev/null +++ b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift @@ -0,0 +1,56 @@ +// +// Copyright © 2025 Embrace Mobile, Inc. All rights reserved. +// + +import Foundation + +public struct EmbraceStackTrace: Equatable { + /// The captured stack frames, following the format of `Thread.callStackSymbols`. + public private(set) var frames: [String] + + /// Initializes a `EmbraceStackTrace` with a given list of stack frames. + /// + /// Each frame is represented as a `String`, containing: + /// - The frame index. + /// - The binary name. + /// - The memory address. + /// - The symbol. + /// - The offset within the symbol. + /// + /// Example format of _a single frame_: + /// ``` + /// 2 EmbraceApp 0x0000000001234abc -[MyClass myMethod] + 48 + /// ``` + /// + /// - Parameter frames: An array of frames strings, following the format of `Thread.callStackSymbols`. + /// - Throws: An `EmbraceStackTraceError.invalidFormat` if any of the frames are not in the expected format. + public init(frames: [String]) throws { + guard frames.allSatisfy(EmbraceStackTrace.isValidStackFrame) else { + throw EmbraceStackTraceError.invalidFormat + } + self.frames = frames + } + + /// Validates if a given frame string follows the required format. + /// - Parameter frame: a stack trace frame. + /// - Returns: whether the format is valid or invalid. + private static func isValidStackFrame(_ frame: String) -> Bool { + /* + Regular expression pattern breakdown: + + ^\s* → Allows optional leading spaces at the beginning + (\d+) → Captures the frame index (a sequence of digits) + \s+ → One or more whitespaces + (.*?) → Captures the module name (non-greedy/lazy to allow spaces) + \s+ → One or more whitespaces + (0x[0-9A-Fa-f]+) → Captures the memory address hex (must start with `0x`) + \s+ → One or more whitespaces + (.+?) → Captures the function/method symbol (non-greedy/lazy) + (?:\s+\+\s+(\d+))? → Optionally captures the slide offset as it might not always be present (`+ `) + */ + let pattern = #"^\s*(\d+)\s+(.*?)\s+(0x[0-9A-Fa-f]+)\s+(.+?)(?:\s+\+\s+(\d+))?$"# + + return frame.range(of: pattern, options: .regularExpression) != nil + } +} + diff --git a/Sources/EmbraceCommonInternal/Enums/LogSeverity.swift b/Sources/EmbraceCommonInternal/Models/LogSeverity.swift similarity index 100% rename from Sources/EmbraceCommonInternal/Enums/LogSeverity.swift rename to Sources/EmbraceCommonInternal/Models/LogSeverity.swift diff --git a/Sources/EmbraceCommonInternal/Enums/SessionState.swift b/Sources/EmbraceCommonInternal/Models/SessionState.swift similarity index 100% rename from Sources/EmbraceCommonInternal/Enums/SessionState.swift rename to Sources/EmbraceCommonInternal/Models/SessionState.swift diff --git a/Sources/EmbraceCommonInternal/Models/StackTraceBehavior.swift b/Sources/EmbraceCommonInternal/Models/StackTraceBehavior.swift new file mode 100644 index 00000000..53b6841e --- /dev/null +++ b/Sources/EmbraceCommonInternal/Models/StackTraceBehavior.swift @@ -0,0 +1,30 @@ +// +// Copyright © 2024 Embrace Mobile, Inc. All rights reserved. +// + +import Foundation + +/// Describes the behavior for automatically capturing stack traces. +public enum StackTraceBehavior { + /// Stack traces are not automatically captured. + case notIncluded + + /// The default behavior for automatically capturing stack traces. + case `default` + + /// A custom stack trace provided. + case custom(_ value: EmbraceStackTrace) +} + +extension StackTraceBehavior: Equatable { + public static func == (lhs: StackTraceBehavior, rhs: StackTraceBehavior) -> Bool { + switch (lhs, rhs) { + case (.notIncluded, .notIncluded), (.default, .default): + return true + case let (.custom(lhsValue), .custom(rhsValue)): + return lhsValue == rhsValue + default: + return false + } + } +} diff --git a/Sources/EmbraceCore/Internal/Logs/LogController.swift b/Sources/EmbraceCore/Internal/Logs/LogController.swift index edf46571..b8880f43 100644 --- a/Sources/EmbraceCore/Internal/Logs/LogController.swift +++ b/Sources/EmbraceCore/Internal/Logs/LogController.swift @@ -90,9 +90,16 @@ class LogController: LogControllable { If we want to keep this method cleaner, we could move this log to `EmbraceLogAttributesBuilder` However that would cause to always add a frame to the stacktrace. */ - if stackTraceBehavior == .default && (severity == .warn || severity == .error) { - let stackTrace: [String] = Thread.callStackSymbols - attributesBuilder.addStackTrace(stackTrace) + switch stackTraceBehavior { + case .default: + if severity == .warn || severity == .error { + let stackTrace: [String] = Thread.callStackSymbols + attributesBuilder.addStackTrace(stackTrace) + } + case .custom(let customStackTrace): + attributesBuilder.addStackTrace(customStackTrace.frames) + case .notIncluded: + break } var finalAttributes = attributesBuilder diff --git a/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift b/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift index c1bc73c8..ddb742a8 100644 --- a/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift +++ b/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift @@ -244,9 +244,54 @@ class LogControllerTests: XCTestCase { whenCreatingLogWithPreUploadedAttachment() thenLogWithPreuploadedAttachmentIsCreatedCorrectly() } + + func testInfoLog_createLogByDefault_doesntAddStackTraceToAttributes() throws { + givenLogController() + whenCreatingLog(severity: .info) + thenLogHasntGotAnEmbbededStackTraceInTheAttributes() + } + + func testWarningLog_createLogByDefault_addsStackTraceToAttributes() throws { + givenLogController() + whenCreatingLog(severity: .warn) + thenLogHasAnEmbbededStackTraceInTheAttributes() + } + + func testErrorLog_createLogByDefault_addsStackTraceToAttributes() throws { + givenLogController() + whenCreatingLog(severity: .error) + thenLogHasAnEmbbededStackTraceInTheAttributes() + } + + func testWarningLog_createLogByWithNotIncludedStacktrace_doesntAddStackTraceToAttributes() throws { + givenLogController() + whenCreatingLog(severity: .warn, stackTraceBehavior: .notIncluded) + thenLogHasntGotAnEmbbededStackTraceInTheAttributes() + } + + + func testErrorLog_createLogByWithNotIncludedStacktrace_doesntAddStackTraceToAttributes() throws { + givenLogController() + whenCreatingLog(severity: .error, stackTraceBehavior: .notIncluded) + thenLogHasntGotAnEmbbededStackTraceInTheAttributes() + } + + func testAnyLog_createLogByWithCustomStacktrace_alwaysAddStackTraceToAttributes() throws { + givenLogController() + let customStackTrace = try EmbraceStackTrace(frames: Thread.callStackSymbols) + whenCreatingLog( + severity: randomSeverity(), + stackTraceBehavior: .custom(customStackTrace) + ) + thenLogHasAnEmbbededStackTraceInTheAttributes() + } } private extension LogControllerTests { + func randomSeverity() -> LogSeverity { + [LogSeverity.error, LogSeverity.warn, LogSeverity.info].randomElement()! + } + func givenLogControllerWithNoStorage() { sut = .init( storage: nil, @@ -330,8 +375,11 @@ private extension LogControllerTests { sut.sessionController?.increaseAttachmentCount() } - func whenCreatingLog() { - sut.createLog("test", severity: .info) + func whenCreatingLog( + severity: LogSeverity = .info, + stackTraceBehavior: StackTraceBehavior = .default + ) { + sut.createLog("test", severity: severity, stackTraceBehavior: stackTraceBehavior) } func whenCreatingLogWithAttachment() { @@ -420,6 +468,22 @@ private extension LogControllerTests { XCTAssertEqual(log!.attributes["emb.type"]!.description, "sys.log") } + func thenLogHasAnEmbbededStackTraceInTheAttributes() { + wait { + let log = self.otelBridge.otel.logs.first + + return log!.attributes["emb.stacktrace.ios"] != nil + } + } + + func thenLogHasntGotAnEmbbededStackTraceInTheAttributes() { + wait { + let log = self.otelBridge.otel.logs.first + + return log!.attributes["emb.stacktrace.ios"] == nil + } + } + func thenLogWithSuccessfulAttachmentIsCreatedCorrectly() { wait { let log = self.otelBridge.otel.logs.first From b419903a6a69b9aef14f41d067f36e28259d1556 Mon Sep 17 00:00:00 2001 From: Ariel Demarco Date: Thu, 20 Feb 2025 16:53:51 -0300 Subject: [PATCH 2/6] Finished with tests on regex for valid stacktrace; made fixes to it found while testing --- .../Models/EmbraceStackTrace.swift | 21 +-- .../Models/EmbraceStackTraceTests.swift | 178 ++++++++++++++++++ .../{Enums => Models}/LogSeverityTests.swift | 0 3 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift rename Tests/EmbraceCommonInternalTests/{Enums => Models}/LogSeverityTests.swift (100%) diff --git a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift index dbc8357c..13d94c78 100644 --- a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift +++ b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift @@ -38,18 +38,17 @@ public struct EmbraceStackTrace: Equatable { /* Regular expression pattern breakdown: - ^\s* → Allows optional leading spaces at the beginning - (\d+) → Captures the frame index (a sequence of digits) - \s+ → One or more whitespaces - (.*?) → Captures the module name (non-greedy/lazy to allow spaces) - \s+ → One or more whitespaces - (0x[0-9A-Fa-f]+) → Captures the memory address hex (must start with `0x`) - \s+ → One or more whitespaces - (.+?) → Captures the function/method symbol (non-greedy/lazy) - (?:\s+\+\s+(\d+))? → Optionally captures the slide offset as it might not always be present (`+ `) + ^\s* -> Allows optional leading spaces at the beginning + (\d+) -> Captures the frame index (a sequence of digits) + \s+ -> One or more whitespaces + ([^\s]+(?:\s+[^\s]+)*) -> Captures the module name, allowing spaces between words but not at the edges + \s+ -> One or more whitespaces + (0x[0-9A-Fa-f]+) -> Captures the memory address hex (must start with `0x`) + \s+ -> One or more whitespaces + (\S.+?) -> Captures the function/method symbol ensuring it's not empty (non-greedy/lazy) + (?:\s+\+\s+(\d+))? -> Optionally captures the slide offset as it might not always be present (`+ `) */ - let pattern = #"^\s*(\d+)\s+(.*?)\s+(0x[0-9A-Fa-f]+)\s+(.+?)(?:\s+\+\s+(\d+))?$"# - + let pattern = #"^\s*(\d+)\s+([^\s]+(?:\s+[^\s]+)*)\s+(0x[0-9A-Fa-f]+)\s+(\S.+?)(?:\s+\+\s+(\d+))?$"# return frame.range(of: pattern, options: .regularExpression) != nil } } diff --git a/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift b/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift new file mode 100644 index 00000000..d3865c2e --- /dev/null +++ b/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift @@ -0,0 +1,178 @@ +// +// Copyright © 2025 Embrace Mobile, Inc. All rights reserved. +// + +import XCTest + +@testable import EmbraceCommonInternal + +class EmbraceStackTraceTests: XCTestCase { + // MARK: - Overall behaviour + + func test_init_withEmptyFramesShouldntThrow() { + XCTAssertNoThrow(try EmbraceStackTrace(frames: [])) + } + + func test_init_withRandomFramesShouldThrow() { + XCTAssertThrowsError(try EmbraceStackTrace(frames: [UUID().uuidString])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_init_shouldBeAbleToBeGeneratedWithThreadCallStackAndKeepInformation() throws { + let threadStackTrace = Thread.callStackSymbols + // Implicit `XCTAssertNoThrow` + let embraceStackTrace = try EmbraceStackTrace(frames: Thread.callStackSymbols) + XCTAssertEqual(embraceStackTrace.frames.count, threadStackTrace.count) + } + + func test_init_withCustomStackTrace_shouldBeAbleToBeGeneratedAndKeepInformation() throws { + // Thread.callStackSymbols from other platform (e.g. Playgrounds app) + let customStackTrace = [ + "0 Page_Contents 0x000000010af45dec main + 136", + "1 ExecutionExtension 0x00000001002a7e24 ExecutionExtension + 32292", + "2 CoreFoundation 0x000000018a965070 __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 28", + "3 CoreFoundation 0x000000018a964f84 __CFRunLoopDoBlocks + 356", + "4 CoreFoundation 0x000000018a963ddc __CFRunLoopRun + 848", + "5 CoreFoundation 0x000000018a963434 CFRunLoopRunSpecific + 608", + "6 HIToolbox 0x000000019510d19c RunCurrentEventLoopInMode + 292", + "7 HIToolbox 0x000000019510cfd8 ReceiveNextEventCommon + 648", + "8 HIToolbox 0x000000019510cd30 _BlockUntilNextEventMatchingListInModeWithFilter + 76", + "9 AppKit 0x000000018e1c2cc8 _DPSNextEvent + 660", + "10 AppKit 0x000000018e9b94d0 -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 700", + "11 ViewBridge 0x00000001930cc6dc __77-[NSViewServiceApplication vbNextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke + 136", + "12 ViewBridge 0x00000001930cc43c -[NSViewServiceApplication _withToxicEventMonitorPerform:] + 152", + "13 ViewBridge 0x00000001930cc63c -[NSViewServiceApplication vbNextEventMatchingMask:untilDate:inMode:dequeue:] + 168", + "14 ViewBridge 0x00000001930b89c0 -[NSViewServiceApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 100", + "15 AppKit 0x000000018e1b5ffc -[NSApplication run] + 476", + "16 AppKit 0x000000018e18d240 NSApplicationMain + 880", + "17 AppKit 0x000000018e3e0654 +[NSWindow _savedFrameFromString:] + 0", + "18 UIKitMacHelper 0x00000001a3c41f50 UINSApplicationMain + 972", + "19 UIKitCore 0x00000001b9e197bc UIApplicationMain + 148", + "20 libxpc.dylib 0x000000018a5ab870 _xpc_objc_uimain + 224", + "21 libxpc.dylib 0x000000018a59d2d0 _xpc_objc_main + 276", + "22 libxpc.dylib 0x000000018a5ace58 _xpc_main + 324", + "23 libxpc.dylib 0x000000018a59d014 _xpc_copy_xpcservice_dictionary + 0", + "24 Foundation 0x000000018bab6048 +[NSXPCListener serviceListener] + 0", + "25 PlugInKit 0x0000000198ccddb0 pkIsServiceAccount + 35664", + "26 PlugInKit 0x0000000198ccdc20 pkIsServiceAccount + 35264", + "27 PlugInKit 0x0000000198ccd88c pkIsServiceAccount + 34348", + "28 PlugInKit 0x0000000198cce180 pkIsServiceAccount + 36640", + "29 ExtensionFoundation 0x00000001e6485280 EXExtensionMain + 304", + "30 Foundation 0x000000018bb13668 NSExtensionMain + 204", + "31 dyld 0x000000018a4fb154 start + 2476" + ] + let embraceStackTrace = try EmbraceStackTrace(frames: customStackTrace) + XCTAssertEqual(embraceStackTrace.frames.count, customStackTrace.count) + } + + func test_init_ifOneFrameIsInvalid_shouldThrow() { + let invalidStackTrace = [ + "0 Page_Contents 0x000000010af45dec main + 136", // valid frame + "a", // invalid frame + "2 CoreFoundation 0x000000018a965070 __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 28" // valid frame + ] + XCTAssertThrowsError(try EmbraceStackTrace(frames: invalidStackTrace)) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + // MARK: - Regex + + func test_stackFrame_shouldStartWithNumber() { + let invalidFrame = " EmbraceApp 0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldHaveSpaceAfterFrameNumber() { + let invalidFrame = "0EmbraceApp 0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldContainModuleName() { + let invalidFrame = "0 0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldAllowSpacesInModuleName() { + let validFrame = "0 Embrace SDK 0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertNoThrow(try EmbraceStackTrace(frames: [validFrame])) + } + + func test_stackFrame_shouldHaveSpaceBeforeMemoryAddress() { + let invalidFrame = "0 EmbraceApp0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldHaveHexAddress() { + let invalidFrame = "0 EmbraceApp 1234abc -[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldHaveSpaceBeforeSymbol() { + let invalidFrame = "0 EmbraceApp 0x0000000001234abc-[MyClass myMethod] + 48" + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_shouldHaveFunctionSymbol() { + let invalidFrame = "0 EmbraceApp 0x0000000001234abc " + XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .invalidFormat) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } + + func test_stackFrame_couldHaveSlideOffsetForSymbol() { + let symbolWithSlideOffsetFrame = "0 EmbraceApp 0x0000000001234abc + 48" + let symbolWithoutSlideOffsetFrame = "0 EmbraceApp 0x0000000001234abc + 48" + XCTAssertNoThrow(try EmbraceStackTrace(frames: [symbolWithSlideOffsetFrame])) + XCTAssertNoThrow(try EmbraceStackTrace(frames: [symbolWithoutSlideOffsetFrame])) + } +} diff --git a/Tests/EmbraceCommonInternalTests/Enums/LogSeverityTests.swift b/Tests/EmbraceCommonInternalTests/Models/LogSeverityTests.swift similarity index 100% rename from Tests/EmbraceCommonInternalTests/Enums/LogSeverityTests.swift rename to Tests/EmbraceCommonInternalTests/Models/LogSeverityTests.swift From a230ae8c8a3662b817f773c3ee576a8006c4f44c Mon Sep 17 00:00:00 2001 From: Ariel Demarco Date: Fri, 21 Feb 2025 11:57:49 -0300 Subject: [PATCH 3/6] Finished with changes after manual and unit testing --- .../BrandGame.xcodeproj/project.pbxproj | 4 -- .../StressTests/Logging/LoggingView.swift | 66 +++++++++++++++++-- .../Logging/StackTraceBehavior+AsString.swift | 17 ----- .../Internal/Logs/LogController.swift | 4 +- Sources/EmbraceCore/Public/Embrace+OTel.swift | 8 +++ .../Internal/Logs/LogControllerTests.swift | 18 +++-- 6 files changed, 85 insertions(+), 32 deletions(-) delete mode 100644 Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/StackTraceBehavior+AsString.swift diff --git a/Examples/BrandGame/BrandGame.xcodeproj/project.pbxproj b/Examples/BrandGame/BrandGame.xcodeproj/project.pbxproj index 9c85c6e9..96eb05b4 100644 --- a/Examples/BrandGame/BrandGame.xcodeproj/project.pbxproj +++ b/Examples/BrandGame/BrandGame.xcodeproj/project.pbxproj @@ -43,7 +43,6 @@ 7BCB66282AC4742800ABD33A /* MenuList.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BCB66272AC4742800ABD33A /* MenuList.swift */; }; 7BCB662A2AC4767B00ABD33A /* MainMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BCB66292AC4767B00ABD33A /* MainMenu.swift */; }; 7BCB662E2AC476BC00ABD33A /* NetworkStressTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7BCB662D2AC476BC00ABD33A /* NetworkStressTest.swift */; }; - FA1114462C8A4EE30026499D /* StackTraceBehavior+AsString.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA1114452C8A4EE30026499D /* StackTraceBehavior+AsString.swift */; }; FA3B0A112C3C2A5600315578 /* EmbraceCore in Frameworks */ = {isa = PBXBuildFile; productRef = FA3B0A102C3C2A5600315578 /* EmbraceCore */; }; FA3B0A132C3C2A5600315578 /* EmbraceCrash in Frameworks */ = {isa = PBXBuildFile; productRef = FA3B0A122C3C2A5600315578 /* EmbraceCrash */; }; FA3B0A152C3C2A5600315578 /* EmbraceCrashlyticsSupport in Frameworks */ = {isa = PBXBuildFile; productRef = FA3B0A142C3C2A5600315578 /* EmbraceCrashlyticsSupport */; }; @@ -135,7 +134,6 @@ 7BCB66272AC4742800ABD33A /* MenuList.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuList.swift; sourceTree = ""; }; 7BCB66292AC4767B00ABD33A /* MainMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MainMenu.swift; sourceTree = ""; }; 7BCB662D2AC476BC00ABD33A /* NetworkStressTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkStressTest.swift; sourceTree = ""; }; - FA1114452C8A4EE30026499D /* StackTraceBehavior+AsString.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "StackTraceBehavior+AsString.swift"; sourceTree = ""; }; FA4C5F322C486E13005C0371 /* CreateSpanView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateSpanView.swift; sourceTree = ""; }; FA4C5F342C487B45005C0371 /* AttributesView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttributesView.swift; sourceTree = ""; }; FA4FA4F92C62DBB600E66300 /* NoSpacesTextField.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoSpacesTextField.swift; sourceTree = ""; }; @@ -454,7 +452,6 @@ isa = PBXGroup; children = ( FA9505152B86640F00562A4C /* LoggingView.swift */, - FA1114452C8A4EE30026499D /* StackTraceBehavior+AsString.swift */, ); path = Logging; sourceTree = ""; @@ -677,7 +674,6 @@ 7B3143C62AFF1E7D00BA0D2F /* ReflexGameModel.swift in Sources */, FAFDA3D72C34273100AD17CF /* MemoryPressureSimulatorView.swift in Sources */, 7B5630A52AC1331000E0DF39 /* Color+Random.swift in Sources */, - FA1114462C8A4EE30026499D /* StackTraceBehavior+AsString.swift in Sources */, 7B62AD952B0040FE0087C2AE /* Minigame.swift in Sources */, 7BBBCD582BD0205C00BC289A /* StdoutLogExporter.swift in Sources */, 7BCB66282AC4742800ABD33A /* MenuList.swift in Sources */, diff --git a/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/LoggingView.swift b/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/LoggingView.swift index 9e91f145..0fd95764 100644 --- a/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/LoggingView.swift +++ b/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/LoggingView.swift @@ -9,7 +9,7 @@ import EmbraceCommonInternal struct LoggingView: View { @State private var logMessage: String = "This is the log message..." @State private var severity: Int = LogSeverity.info.rawValue - @State private var behavior: Int = StackTraceBehavior.default.rawValue + @State private var behavior: Int = Behavior.default.rawValue @State private var key: String = "" @State private var value: String = "" @State private var attributes: [String: String] = [:] @@ -19,8 +19,8 @@ struct LoggingView: View { [.info, .warn, .error] }() - private let behaviors: [StackTraceBehavior] = { - [.default, .notIncluded] + private let behaviors: [Behavior] = { + [.default, .notIncluded, .custom] }() var body: some View { @@ -86,14 +86,38 @@ private extension LoggingView { print("Wrong severity number") return } - guard let behavior = StackTraceBehavior(rawValue: behavior) else { - print("Wrong behavior") + guard let stackTraceBehavior = try? getStackTraceBehavior() else { + print("Wrong stacktrace behavior") return } - Embrace.client?.log(logMessage, severity: severity, attributes: attributes, stackTraceBehavior: behavior) + Embrace.client?.log( + logMessage, + severity: severity, + attributes: attributes, + stackTraceBehavior: stackTraceBehavior + ) cleanUpFields() } + func getStackTraceBehavior() throws -> StackTraceBehavior { + switch Behavior(rawValue: behavior) { + case .default: + return .default + case .notIncluded: + return .notIncluded + case .custom: + return .custom(try EmbraceStackTrace( + frames: [ + "0 BrandGame 0x0000000005678def [SomeClass method] + 48", + "1 Random Library 0x0000000001234abc [Random init]", + "2 \(UUID().uuidString) 0x0000000001234abc [\(UUID().uuidString) \(UUID().uuidString))]" + ]) + ) + case .none: + throw NSError(domain: "BrandGame", code: -1, userInfo: [:]) + } + } + func cleanUpFields() { guard shouldCleanUp else { return @@ -105,6 +129,36 @@ private extension LoggingView { } } +extension LoggingView { + enum Behavior: Int { + case notIncluded + case `default` + case custom + + static func from(_ stackTraceBehavior: StackTraceBehavior) -> Self { + switch stackTraceBehavior { + case .notIncluded: + return .notIncluded + case .default: + return .default + case .custom(_): + return .custom + } + } + + func asString() -> String { + return switch self { + case .notIncluded: + "Not included" + case .default: + "Default" + case .custom: + "Custom" + } + } + } +} + #Preview { LoggingView() } diff --git a/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/StackTraceBehavior+AsString.swift b/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/StackTraceBehavior+AsString.swift deleted file mode 100644 index bdd02f41..00000000 --- a/Examples/BrandGame/BrandGame/View/Menu/StressTests/Logging/StackTraceBehavior+AsString.swift +++ /dev/null @@ -1,17 +0,0 @@ -// -// Copyright © 2023 Embrace Mobile, Inc. All rights reserved. -// - -import Foundation -import EmbraceCommonInternal - -extension StackTraceBehavior { - func asString() -> String { - switch self { - case .default: - return "Default" - case .notIncluded: - return "Not Included" - } - } -} diff --git a/Sources/EmbraceCore/Internal/Logs/LogController.swift b/Sources/EmbraceCore/Internal/Logs/LogController.swift index b8880f43..4fb7f3ec 100644 --- a/Sources/EmbraceCore/Internal/Logs/LogController.swift +++ b/Sources/EmbraceCore/Internal/Logs/LogController.swift @@ -97,7 +97,9 @@ class LogController: LogControllable { attributesBuilder.addStackTrace(stackTrace) } case .custom(let customStackTrace): - attributesBuilder.addStackTrace(customStackTrace.frames) + if severity == .warn || severity == .error { + attributesBuilder.addStackTrace(customStackTrace.frames) + } case .notIncluded: break } diff --git a/Sources/EmbraceCore/Public/Embrace+OTel.swift b/Sources/EmbraceCore/Public/Embrace+OTel.swift index 66407f64..edc74cd8 100644 --- a/Sources/EmbraceCore/Public/Embrace+OTel.swift +++ b/Sources/EmbraceCore/Public/Embrace+OTel.swift @@ -119,6 +119,8 @@ extension Embrace: EmbraceOpenTelemetry { /// - severity: `LogSeverity` for the log. /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log + /// + /// - Important: `.info` logs will _never_ have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -143,6 +145,8 @@ extension Embrace: EmbraceOpenTelemetry { /// - timestamp: Timestamp for the log. /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log + /// + /// - Important: `.info` logs will _never_ have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -173,6 +177,8 @@ extension Embrace: EmbraceOpenTelemetry { /// - attachment: Data of the attachment /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log + /// + /// - Important: `.info` logs will _never_ have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -205,6 +211,8 @@ extension Embrace: EmbraceOpenTelemetry { /// - attachmentUrl: URL to dowload the attachment data /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log + /// + /// - Important: `.info` logs will _never_ have stacktraces. public func log( _ message: String, severity: LogSeverity, diff --git a/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift b/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift index ddb742a8..7a259dc9 100644 --- a/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift +++ b/Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift @@ -276,20 +276,30 @@ class LogControllerTests: XCTestCase { thenLogHasntGotAnEmbbededStackTraceInTheAttributes() } - func testAnyLog_createLogByWithCustomStacktrace_alwaysAddStackTraceToAttributes() throws { + func testWarnAndErrorLogs_createLogByWithCustomStacktrace_alwaysAddStackTraceToAttributes() throws { givenLogController() let customStackTrace = try EmbraceStackTrace(frames: Thread.callStackSymbols) whenCreatingLog( - severity: randomSeverity(), + severity: [.warn, .error].randomElement()!, stackTraceBehavior: .custom(customStackTrace) ) thenLogHasAnEmbbededStackTraceInTheAttributes() } + + func testInfoLogs_createLogByWithCustomStacktrace_wontAddStackTraceToAttributes() throws { + givenLogController() + let customStackTrace = try EmbraceStackTrace(frames: Thread.callStackSymbols) + whenCreatingLog( + severity: .info, + stackTraceBehavior: .custom(customStackTrace) + ) + thenLogHasntGotAnEmbbededStackTraceInTheAttributes() + } } private extension LogControllerTests { - func randomSeverity() -> LogSeverity { - [LogSeverity.error, LogSeverity.warn, LogSeverity.info].randomElement()! + func randomSeverity(from severities: [LogSeverity]) -> LogSeverity { + severities.randomElement()! } func givenLogControllerWithNoStorage() { From 756a8ee966773ffc1375072c78c5f44d0a438f05 Mon Sep 17 00:00:00 2001 From: Ariel Demarco Date: Mon, 24 Feb 2025 13:21:28 -0300 Subject: [PATCH 4/6] Added limits to frames --- .../EmbraceStackTraceError.swift | 5 +++ .../Models/EmbraceStackTrace.swift | 38 ++++++++++++++++--- .../Models/EmbraceStackTraceTests.swift | 35 +++++++++++++++++ 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift b/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift index b434ce4c..9ef0016e 100644 --- a/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift +++ b/Sources/EmbraceCommonInternal/Error Management/EmbraceStackTraceError.swift @@ -6,6 +6,7 @@ import Foundation public enum EmbraceStackTraceError: Error { case invalidFormat + case frameIsTooLong } extension EmbraceStackTraceError: LocalizedError, CustomNSError { @@ -18,6 +19,8 @@ extension EmbraceStackTraceError: LocalizedError, CustomNSError { switch self { case .invalidFormat: return -1 + case .frameIsTooLong: + return -2 } } @@ -29,6 +32,8 @@ extension EmbraceStackTraceError: LocalizedError, CustomNSError { [ + ] The "+ " part is optional. """ + case .frameIsTooLong: + return "The stacktrace contains frames that are longer than 10.000 characters." } } diff --git a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift index 13d94c78..9e51d5df 100644 --- a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift +++ b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift @@ -5,6 +5,12 @@ import Foundation public struct EmbraceStackTrace: Equatable { + /// The maximum amount of characters a stack trace frame can have. + private static let maximumFrameLength = 10000 + + /// The maximum amount of frames a stacktrace we support. + private static let maximumAmountOfFrames = 200 + /// The captured stack frames, following the format of `Thread.callStackSymbols`. public private(set) var frames: [String] @@ -23,18 +29,40 @@ public struct EmbraceStackTrace: Equatable { /// ``` /// /// - Parameter frames: An array of frames strings, following the format of `Thread.callStackSymbols`. - /// - Throws: An `EmbraceStackTraceError.invalidFormat` if any of the frames are not in the expected format. + /// - Throws: An `EmbraceStackTraceError.invalidFormat` if any of the frames are not in the expected format + /// - Throws: An `EmbraceStackTraceError.frameIsTooLong` if any of the frames has more than the `maximumFrameLength` (10.000 characters). + /// + /// - Important: a stacktrace can't have more than `maximumAmountOfFrames` (200); if that happens, we'll trim the exceeding frames. public init(frames: [String]) throws { - guard frames.allSatisfy(EmbraceStackTrace.isValidStackFrame) else { - throw EmbraceStackTraceError.invalidFormat + let trimmedStackTrace = EmbraceStackTrace.trimStackTrace(frames) + try EmbraceStackTrace.validateStackFrames(trimmedStackTrace) + self.frames = trimmedStackTrace + } + + private static func validateStackFrames(_ frames: [String]) throws { + try frames.forEach { + if $0.count >= maximumFrameLength { + throw EmbraceStackTraceError.frameIsTooLong + } + if !isValidStackFrameFormat($0) { + throw EmbraceStackTraceError.invalidFormat + } } - self.frames = frames + } + + /// Generates a stack trace with up to `maximumAmountOfFrames` frames. + /// + /// - Important: A stack trace can't have more than `maximumAmountOfFrames` (200); + /// if that happens, we'll trim the exceeding frames. + /// - Returns: An array of stack frames as `String`. + private static func trimStackTrace(_ stackFrames: [String]) -> [String] { + return Array(stackFrames.prefix(maximumAmountOfFrames)) } /// Validates if a given frame string follows the required format. /// - Parameter frame: a stack trace frame. /// - Returns: whether the format is valid or invalid. - private static func isValidStackFrame(_ frame: String) -> Bool { + private static func isValidStackFrameFormat(_ frame: String) -> Bool { /* Regular expression pattern breakdown: diff --git a/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift b/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift index d3865c2e..4987e9f9 100644 --- a/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift +++ b/Tests/EmbraceCommonInternalTests/Models/EmbraceStackTraceTests.swift @@ -85,6 +85,13 @@ class EmbraceStackTraceTests: XCTestCase { } } + func test_initWithHugeStackTrace_shouldTrimToTwoHundredFrames() throws { + let stackTrace = try EmbraceStackTrace(frames: generateRandomStackFrames(numberOfFrames: .random(in: 201...10000))) + XCTAssertEqual(stackTrace.frames.count, 200) + XCTAssertTrue(stackTrace.frames.first!.starts(with: "0")) + XCTAssertTrue(stackTrace.frames.last!.starts(with: "199")) + } + // MARK: - Regex func test_stackFrame_shouldStartWithNumber() { @@ -125,6 +132,11 @@ class EmbraceStackTraceTests: XCTestCase { XCTAssertNoThrow(try EmbraceStackTrace(frames: [validFrame])) } + func test_stackFrame_shouldAllowUnicodeCharsInModuleName() { + let validFrame = "0 xx 0x0000000001234abc -[MyClass myMethod] + 48" + XCTAssertNoThrow(try EmbraceStackTrace(frames: [validFrame])) + } + func test_stackFrame_shouldHaveSpaceBeforeMemoryAddress() { let invalidFrame = "0 EmbraceApp0x0000000001234abc -[MyClass myMethod] + 48" XCTAssertThrowsError(try EmbraceStackTrace(frames: [invalidFrame])) { error in @@ -175,4 +187,27 @@ class EmbraceStackTraceTests: XCTestCase { XCTAssertNoThrow(try EmbraceStackTrace(frames: [symbolWithSlideOffsetFrame])) XCTAssertNoThrow(try EmbraceStackTrace(frames: [symbolWithoutSlideOffsetFrame])) } + + func test_stackFrame_cantHaveMoreThanTenThousandCharacters() { + let longFrame = String(repeating: "A", count: 10001) + XCTAssertThrowsError(try EmbraceStackTrace(frames: [longFrame])) { error in + if let error = error as? EmbraceStackTraceError { + XCTAssertTrue(error == .frameIsTooLong) + } else { + XCTFail("Error should be `EmbraceStackTraceError.invalidFormat`") + } + } + } +} + +private extension EmbraceStackTraceTests { + func generateRandomStackFrames(numberOfFrames: Int = 30) -> [String] { + return (0.. Date: Mon, 24 Feb 2025 13:24:23 -0300 Subject: [PATCH 5/6] Fix typo --- Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift index 9e51d5df..48f87197 100644 --- a/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift +++ b/Sources/EmbraceCommonInternal/Models/EmbraceStackTrace.swift @@ -8,7 +8,7 @@ public struct EmbraceStackTrace: Equatable { /// The maximum amount of characters a stack trace frame can have. private static let maximumFrameLength = 10000 - /// The maximum amount of frames a stacktrace we support. + /// The maximum amount of frames we support in a stacktrace. private static let maximumAmountOfFrames = 200 /// The captured stack frames, following the format of `Thread.callStackSymbols`. From b962204e861d2a8a64b886f5a7e587e3c70a399e Mon Sep 17 00:00:00 2001 From: Ariel Demarco Date: Wed, 26 Feb 2025 17:13:21 -0300 Subject: [PATCH 6/6] Fixed documentation to make it a bit more clear --- Sources/EmbraceCore/Public/Embrace+OTel.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/EmbraceCore/Public/Embrace+OTel.swift b/Sources/EmbraceCore/Public/Embrace+OTel.swift index edc74cd8..8e89a229 100644 --- a/Sources/EmbraceCore/Public/Embrace+OTel.swift +++ b/Sources/EmbraceCore/Public/Embrace+OTel.swift @@ -120,7 +120,7 @@ extension Embrace: EmbraceOpenTelemetry { /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log /// - /// - Important: `.info` logs will _never_ have stacktraces. + /// - Important: Only `warn` and `error` logs will have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -146,7 +146,7 @@ extension Embrace: EmbraceOpenTelemetry { /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log /// - /// - Important: `.info` logs will _never_ have stacktraces. + /// - Important: Only `warn` and `error` logs will have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -178,7 +178,7 @@ extension Embrace: EmbraceOpenTelemetry { /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log /// - /// - Important: `.info` logs will _never_ have stacktraces. + /// - Important: Only `warn` and `error` logs will have stacktraces. public func log( _ message: String, severity: LogSeverity, @@ -212,7 +212,7 @@ extension Embrace: EmbraceOpenTelemetry { /// - attributes: Attributes for the log. /// - stackTraceBehavior: Defines if the stack trace information should be added to the log /// - /// - Important: `.info` logs will _never_ have stacktraces. + /// - Important: Only `warn` and `error` logs will have stacktraces. public func log( _ message: String, severity: LogSeverity,