Skip to content

Commit

Permalink
fix: Too many navigation breadcrumbs for Session Replay
Browse files Browse the repository at this point in the history
  • Loading branch information
brustolin committed Oct 29, 2024
1 parent 27bdedd commit 9419c9f
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 5 deletions.
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@
D8F6A24B2885515C00320515 /* SentryPredicateDescriptor.h in Headers */ = {isa = PBXBuildFile; fileRef = D8F6A24A2885515B00320515 /* SentryPredicateDescriptor.h */; };
D8F6A24E288553A800320515 /* SentryPredicateDescriptorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8F6A24C2885534E00320515 /* SentryPredicateDescriptorTests.swift */; };
D8F8F5572B835BC600AC5465 /* SentryMsgPackSerializerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D8F8F5562B835BC600AC5465 /* SentryMsgPackSerializerTests.m */; };
D8FC98AB2CD0DAB30009824C /* BreadcrumbExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8FC98AA2CD0DAAC0009824C /* BreadcrumbExtension.swift */; };
D8FFE50C2703DBB400607131 /* SwizzlingCallTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8FFE50B2703DAAE00607131 /* SwizzlingCallTests.swift */; };
/* End PBXBuildFile section */

Expand Down Expand Up @@ -2007,6 +2008,7 @@
D8F6A24A2885515B00320515 /* SentryPredicateDescriptor.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryPredicateDescriptor.h; path = include/SentryPredicateDescriptor.h; sourceTree = "<group>"; };
D8F6A24C2885534E00320515 /* SentryPredicateDescriptorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryPredicateDescriptorTests.swift; sourceTree = "<group>"; };
D8F8F5562B835BC600AC5465 /* SentryMsgPackSerializerTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryMsgPackSerializerTests.m; sourceTree = "<group>"; };
D8FC98AA2CD0DAAC0009824C /* BreadcrumbExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BreadcrumbExtension.swift; sourceTree = "<group>"; };
D8FFE50B2703DAAE00607131 /* SwizzlingCallTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwizzlingCallTests.swift; sourceTree = "<group>"; };
/* End PBXFileReference section */

Expand Down Expand Up @@ -3449,6 +3451,7 @@
isa = PBXGroup;
children = (
84AEB4682C2F9673007E46E1 /* ArrayAccesses.swift */,
D8FC98AA2CD0DAAC0009824C /* BreadcrumbExtension.swift */,
841325DE2BFED0510029228F /* TestFramesTracker.swift */,
841325C42BF49EC40029228F /* SentryLaunchProfiling+Tests.h */,
7B4C817124D1BC2B0076ACE4 /* SentryFileManager+Test.h */,
Expand Down Expand Up @@ -5119,6 +5122,7 @@
84AC61DB29F7654A009EEF61 /* TestDispatchSourceWrapper.swift in Sources */,
8431F01729B2851500D8DC56 /* TestSentrySystemWrapper.swift in Sources */,
84281C632A579D0700EE88F2 /* SentryProfilerMocks.mm in Sources */,
D8FC98AB2CD0DAB30009824C /* BreadcrumbExtension.swift in Sources */,
84B7FA4129B28CD200AD93B1 /* TestSentryDispatchQueueWrapper.swift in Sources */,
84B7FA3E29B28ADD00AD93B1 /* TestClient.swift in Sources */,
);
Expand Down
20 changes: 20 additions & 0 deletions SentryTestUtils/BreadcrumbExtension.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import Sentry
public extension Breadcrumb {
static func navigation(screen: String, date: Date? = nil) -> Breadcrumb {
let result = Breadcrumb(level: .info, category: "navigation")

result.type = "navigation"
result.timestamp = date
result.data = ["screen": screen]

return result
}

static func custom(date: Date? = nil) -> Breadcrumb {
let result = Breadcrumb(level: .info, category: "custom")

result.timestamp = date

return result
}
}
22 changes: 17 additions & 5 deletions Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,25 @@ class SentrySessionReplay: NSObject {
SentryLog.debug("Could not delete replay segment from disk: \(error.localizedDescription)")
}
}

private func convertBreadcrumbs(breadcrumbs: [Breadcrumb], from: Date, until: Date) -> [any SentryRRWebEventProtocol] {
return breadcrumbs.filter {
guard let time = $0.timestamp, time >= from && time < until else { return false }
return true
var filteredResult: [Breadcrumb] = []
var lastNavigationTime: Date = from.addingTimeInterval(-1)

for breadcrumb in breadcrumbs {
guard let time = breadcrumb.timestamp, time >= from && time < until else { continue }

// If it's a "navigation" breadcrumb, check the timestamp difference from the previous breadcrumb.
// Skip any breadcrumbs that have occurred within 50ms of the last one,
// as these represent child view controllers that don’t need their own navigation breadcrumb.
if breadcrumb.type == "navigation" {
if time.timeIntervalSince(lastNavigationTime) < 0.05 { continue }
lastNavigationTime = time
}
filteredResult.append(breadcrumb)
}
.compactMap(breadcrumbConverter.convert(from:))

return filteredResult.compactMap(breadcrumbConverter.convert(from:))
}

private func takeScreenshot() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,40 @@ class SentrySessionReplayTests: XCTestCase {
XCTAssertNil(fixture.screenshotProvider.lastImageCall)
}

func testFilterCloseNavigationBreadcrumbs() {
let fixture = Fixture()

let sut = fixture.getSut(options: SentryReplayOptions(sessionSampleRate: 1, onErrorSampleRate: 1))
sut.start(rootView: fixture.rootView, fullSession: true)
XCTAssertEqual(fixture.lastReplayId, sut.sessionReplayId)

fixture.dateProvider.advance(by: 1)
let startEvent = fixture.dateProvider.date()

fixture.breadcrumbs = [
.navigation(screen: "Some Screen", date: startEvent.addingTimeInterval(0.1)), // This should not filter out
.custom(date: startEvent.addingTimeInterval(0.11)), // This should not filter out
.navigation(screen: "Child VC 1", date: startEvent.addingTimeInterval(0.11)), // Dont keep this one
.navigation(screen: "Child VC 2", date: startEvent.addingTimeInterval(0.12)), // Dont keep this one
.navigation(screen: "Child VC 3", date: startEvent.addingTimeInterval(0.15)), // Dont keep this one
.navigation(screen: "Another Screen", date: startEvent.addingTimeInterval(0.16)) // This should not filter out
]

Dynamic(sut).newFrame(nil)
fixture.dateProvider.advance(by: 5)
Dynamic(sut).newFrame(nil)

let event = Event(error: NSError(domain: "Some error", code: 1))
sut.captureReplayFor(event: event)

let breadCrumbRREvents = fixture.lastReplayRecording?.events.compactMap( { $0 as? SentryRRWebBreadcrumbEvent }) ?? []

XCTAssertEqual(breadCrumbRREvents.count, 3)
XCTAssertEqual((breadCrumbRREvents[0].data?["payload"] as? [String: Any])?["message"] as? String, "Some Screen")
XCTAssertEqual((breadCrumbRREvents[1].data?["payload"] as? [String: Any])?["category"] as? String, "custom")
XCTAssertEqual((breadCrumbRREvents[2].data?["payload"] as? [String: Any])?["message"] as? String, "Another Screen")
}

@available(iOS 16.0, tvOS 16, *)
func testDealloc_CallsStop() {
let fixture = Fixture()
Expand Down

0 comments on commit 9419c9f

Please sign in to comment.