Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Too many navigation breadcrumbs for Session Replay #4480

Merged
merged 4 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Add option to report uncaught NSExceptions on macOS (#4471)
- Build visionOS project with static Sentry SDK (#4462)
- Too many navigation breadcrumbs for Session Replay (#4480)
- Capture all touches with session replay (#4477)

### Improvements
Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,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 @@ -2013,6 +2014,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 @@ -3458,6 +3460,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 @@ -5131,6 +5134,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
}
}
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 {
brustolin marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -327,6 +327,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")
}

func testCaptureAllTouches() {
let fixture = Fixture()
let touchTracker = TestTouchTracker(dateProvider: fixture.dateProvider, scale: 1)
Expand Down
Loading