diff --git a/CHANGELOG.md b/CHANGELOG.md index a379f1d4c78..e7d87451e81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +## Features + +- Add slow and frozen frames to spans (#3450) + ## 8.16.1 ### Features diff --git a/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift b/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift index c355508fdc6..d64512a7119 100644 --- a/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift +++ b/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift @@ -66,7 +66,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { scope.setTag(value: "swift", key: "language") let user = User(userId: "1") - user.email = "tony@example.com" + user.email = "philipp@example.com" scope.setUser(user) if let path = Bundle.main.path(forResource: "Tongariro", ofType: "jpg") { diff --git a/Sources/Sentry/SentryBuildAppStartSpans.m b/Sources/Sentry/SentryBuildAppStartSpans.m index 8477c72a970..bfa40a5280f 100644 --- a/Sources/Sentry/SentryBuildAppStartSpans.m +++ b/Sources/Sentry/SentryBuildAppStartSpans.m @@ -22,7 +22,7 @@ origin:SentryTraceOriginAutoAppStart sampled:tracer.sampled]; - return [[SentrySpan alloc] initWithTracer:tracer context:context]; + return [[SentrySpan alloc] initWithTracer:tracer context:context framesTracker:nil]; } NSArray * diff --git a/Sources/Sentry/SentryFramesTracker.m b/Sources/Sentry/SentryFramesTracker.m index d261c976cb7..37074cd2a86 100644 --- a/Sources/Sentry/SentryFramesTracker.m +++ b/Sources/Sentry/SentryFramesTracker.m @@ -242,4 +242,14 @@ - (void)dealloc @end +BOOL +sentryShouldAddSlowFrozenFramesData( + NSInteger totalFrames, NSInteger slowFrames, NSInteger frozenFrames) +{ + BOOL allBiggerThanZero = totalFrames >= 0 && slowFrames >= 0 && frozenFrames >= 0; + BOOL oneBiggerThanZero = totalFrames > 0 || slowFrames > 0 || frozenFrames > 0; + + return allBiggerThanZero && oneBiggerThanZero; +} + #endif // SENTRY_HAS_UIKIT diff --git a/Sources/Sentry/SentrySpan.m b/Sources/Sentry/SentrySpan.m index 95bbbcf5183..3caa32accaa 100644 --- a/Sources/Sentry/SentrySpan.m +++ b/Sources/Sentry/SentrySpan.m @@ -19,6 +19,11 @@ #import "SentryTraceHeader.h" #import "SentryTracer.h" +#if SENTRY_HAS_UIKIT +# import +# import +#endif // SENTRY_HAS_UIKIT + NS_ASSUME_NONNULL_BEGIN @interface @@ -30,9 +35,18 @@ @implementation SentrySpan { NSMutableDictionary *_tags; NSObject *_stateLock; BOOL _isFinished; +#if SENTRY_HAS_UIKIT + NSUInteger initTotalFrames; + NSUInteger initSlowFrames; + NSUInteger initFrozenFrames; + SentryFramesTracker *_framesTracker; +#endif // SENTRY_HAS_UIKIT } - (instancetype)initWithContext:(SentrySpanContext *)context +#if SENTRY_HAS_UIKIT + framesTracker:(nullable SentryFramesTracker *)framesTracker; +#endif // SENTRY_HAS_UIKIT { if (self = [super init]) { self.startTimestamp = [SentryDependencyContainer.sharedInstance.dateProvider date]; @@ -43,6 +57,17 @@ - (instancetype)initWithContext:(SentrySpanContext *)context if ([NSThread isMainThread]) { _data[SPAN_DATA_THREAD_NAME] = @"main"; + +#if SENTRY_HAS_UIKIT + // Only track frames if running on main thread. + _framesTracker = framesTracker; + if (_framesTracker.isRunning) { + SentryScreenFrames *currentFrames = _framesTracker.currentFrames; + initTotalFrames = currentFrames.total; + initSlowFrames = currentFrames.slow; + initFrozenFrames = currentFrames.frozen; + } +#endif // SENTRY_HAS_UIKIT } else { _data[SPAN_DATA_THREAD_NAME] = [SentryDependencyContainer.sharedInstance.threadInspector getThreadName:currentThread]; @@ -64,9 +89,17 @@ - (instancetype)initWithContext:(SentrySpanContext *)context return self; } -- (instancetype)initWithTracer:(SentryTracer *)tracer context:(SentrySpanContext *)context +- (instancetype)initWithTracer:(SentryTracer *)tracer + context:(SentrySpanContext *)context +#if SENTRY_HAS_UIKIT + framesTracker:(nullable SentryFramesTracker *)framesTracker +{ + if (self = [self initWithContext:context framesTracker:framesTracker]) { +#else { if (self = [self initWithContext:context]) { +#endif // SENTRY_HAS_UIKIT + _tracer = tracer; } return self; @@ -171,6 +204,27 @@ - (void)finishWithStatus:(SentrySpanStatus)status SENTRY_LOG_DEBUG(@"Setting span timestamp: %@ at system time %llu", self.timestamp, (unsigned long long)SentryDependencyContainer.sharedInstance.dateProvider.systemTime); } + +#if SENTRY_HAS_UIKIT + if (_framesTracker.isRunning) { + + SentryScreenFrames *currentFrames = _framesTracker.currentFrames; + NSInteger totalFrames = currentFrames.total - initTotalFrames; + NSInteger slowFrames = currentFrames.slow - initSlowFrames; + NSInteger frozenFrames = currentFrames.frozen - initFrozenFrames; + + if (sentryShouldAddSlowFrozenFramesData(totalFrames, slowFrames, frozenFrames)) { + [self setDataValue:@(totalFrames) forKey:@"frames.total"]; + [self setDataValue:@(slowFrames) forKey:@"frames.slow"]; + [self setDataValue:@(frozenFrames) forKey:@"frames.frozen"]; + + SENTRY_LOG_DEBUG(@"Frames for span \"%@\" Total:%ld Slow:%ld Frozen:%ld", + self.operation, (long)totalFrames, (long)slowFrames, (long)frozenFrames); + } + } + +#endif // SENTRY_HAS_UIKIT + if (self.tracer == nil) { SENTRY_LOG_DEBUG( @"No tracer associated with span with id %@", self.spanId.sentrySpanIdString); diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index 04f21900122..39284809123 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -123,7 +123,11 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti hub:(nullable SentryHub *)hub configuration:(SentryTracerConfiguration *)configuration; { - if (!(self = [super initWithContext:transactionContext])) { + if (!(self = [super initWithContext:transactionContext +#if SENTRY_HAS_UIKIT + framesTracker:SentryDependencyContainer.sharedInstance.framesTracker +#endif // SENTRY_HAS_UIKIT + ])) { return nil; } @@ -351,7 +355,13 @@ - (void)cancelDeadlineTimer spanDescription:description sampled:self.sampled]; - SentrySpan *child = [[SentrySpan alloc] initWithTracer:self context:context]; + SentrySpan *child = + [[SentrySpan alloc] initWithTracer:self + context:context +#if SENTRY_HAS_UIKIT + framesTracker:SentryDependencyContainer.sharedInstance.framesTracker +#endif // SENTRY_HAS_UIKIT + ]; child.startTimestamp = [SentryDependencyContainer.sharedInstance.dateProvider date]; SENTRY_LOG_DEBUG(@"Started child span %@ under %@", child.spanId.sentrySpanIdString, parentId.sentrySpanIdString); @@ -741,10 +751,7 @@ - (void)addMeasurements:(SentryTransaction *)transaction NSInteger slowFrames = currentFrames.slow - initSlowFrames; NSInteger frozenFrames = currentFrames.frozen - initFrozenFrames; - BOOL allBiggerThanZero = totalFrames >= 0 && slowFrames >= 0 && frozenFrames >= 0; - BOOL oneBiggerThanZero = totalFrames > 0 || slowFrames > 0 || frozenFrames > 0; - - if (allBiggerThanZero && oneBiggerThanZero) { + if (sentryShouldAddSlowFrozenFramesData(totalFrames, slowFrames, frozenFrames)) { [self setMeasurement:@"frames_total" value:@(totalFrames)]; [self setMeasurement:@"frames_slow" value:@(slowFrames)]; [self setMeasurement:@"frames_frozen" value:@(frozenFrames)]; diff --git a/Sources/Sentry/include/SentryFramesTracker.h b/Sources/Sentry/include/SentryFramesTracker.h index 6724c3562c6..22e5c86e3b5 100644 --- a/Sources/Sentry/include/SentryFramesTracker.h +++ b/Sources/Sentry/include/SentryFramesTracker.h @@ -40,6 +40,9 @@ NS_ASSUME_NONNULL_BEGIN @end +BOOL sentryShouldAddSlowFrozenFramesData( + NSInteger totalFrames, NSInteger slowFrames, NSInteger frozenFrames); + NS_ASSUME_NONNULL_END #endif // SENTRY_HAS_UIKIT diff --git a/Sources/Sentry/include/SentrySpan.h b/Sources/Sentry/include/SentrySpan.h index 72ddce92044..70678b7d67e 100644 --- a/Sources/Sentry/include/SentrySpan.h +++ b/Sources/Sentry/include/SentrySpan.h @@ -6,6 +6,10 @@ NS_ASSUME_NONNULL_BEGIN @class SentryTracer, SentryId, SentrySpanId, SentryFrame, SentrySpanContext; @protocol SentrySerializable; +#if SENTRY_HAS_UIKIT +@class SentryFramesTracker; +#endif // SENTRY_HAS_UIKIT + @interface SentrySpan : NSObject SENTRY_NO_INIT @@ -85,13 +89,22 @@ SENTRY_NO_INIT * @param transaction The @c SentryTracer managing the transaction this span is associated with. * @param context This span context information. */ -- (instancetype)initWithTracer:(SentryTracer *)transaction context:(SentrySpanContext *)context; +- (instancetype)initWithTracer:(SentryTracer *)transaction + context:(SentrySpanContext *)context +#if SENTRY_HAS_UIKIT + framesTracker:(nullable SentryFramesTracker *)framesTracker; +#endif // SENTRY_HAS_UIKIT +; /** * Init a @c SentrySpan with given context. * @param context This span context information. */ -- (instancetype)initWithContext:(SentrySpanContext *)context; +- (instancetype)initWithContext:(SentrySpanContext *)context +#if SENTRY_HAS_UIKIT + framesTracker:(nullable SentryFramesTracker *)framesTracker; +#endif // SENTRY_HAS_UIKIT +; - (void)setExtraValue:(nullable id)value forKey:(NSString *)key DEPRECATED_ATTRIBUTE; @end diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 43e06071e02..a1648cbaa05 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -348,7 +348,7 @@ class SentryClientTest: XCTestCase { func testCaptureTransactionWithoutScreen() { SentryDependencyContainer.sharedInstance().application = TestSentryUIApplication() - let event = Transaction(trace: SentryTracer(context: SpanContext(operation: "test")), children: []) + let event = Transaction(trace: SentryTracer(context: SpanContext(operation: "test"), framesTracker: nil), children: []) fixture.getSut().capture(event: event, scope: fixture.scope) try? assertLastSentEventWithAttachment { event in diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift index 4001f82890a..2efaf90a47d 100644 --- a/Tests/SentryTests/Transaction/SentrySpanTests.swift +++ b/Tests/SentryTests/Transaction/SentrySpanTests.swift @@ -1,3 +1,4 @@ +import Nimble import Sentry import SentryTestUtils import XCTest @@ -14,7 +15,11 @@ class SentrySpanTests: XCTestCase { let extraValue = "extra_value" let options: Options let currentDateProvider = TestCurrentDateProvider() +#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + let tracer = SentryTracer(context: SpanContext(operation: "TEST"), framesTracker: nil) + #else let tracer = SentryTracer(context: SpanContext(operation: "TEST")) + #endif init() { options = Options() @@ -33,6 +38,13 @@ class SentrySpanTests: XCTestCase { return hub.startTransaction(name: someTransaction, operation: someOperation) } + func getSutWithTracer() -> SentrySpan { +#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + return SentrySpan(tracer: tracer, context: SpanContext(operation: someOperation, sampled: .undecided), framesTracker: nil) +#else + return SentrySpan(tracer: tracer, context: SpanContext(operation: someOperation, sampled: .undecided)) +#endif + } } override func setUp() { @@ -142,7 +154,7 @@ class SentrySpanTests: XCTestCase { } func testFinishSpanWithDefaultTimestamp() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: fixture.someOperation, sampled: .undecided)) + let span = fixture.getSutWithTracer() span.finish() XCTAssertEqual(span.startTimestamp, TestData.timestamp) @@ -152,7 +164,7 @@ class SentrySpanTests: XCTestCase { } func testFinishSpanWithCustomTimestamp() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: fixture.someOperation, sampled: .undecided)) + let span = fixture.getSutWithTracer() span.timestamp = Date(timeIntervalSince1970: 123) span.finish() @@ -292,15 +304,15 @@ class SentrySpanTests: XCTestCase { XCTAssertEqual("manual", serialization["origin"] as? String) } - func testSerialization_NoFrames() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: "test")) + func testSerialization_NoStacktraceFrames() { + let span = fixture.getSutWithTracer() let serialization = span.serialize() XCTAssertEqual(2, (serialization["data"] as? [String: Any])?.count, "Only expected thread.name and thread.id in data.") } - func testSerialization_withFrames() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: "test")) + func testSerialization_withStacktraceFrames() { + let span = fixture.getSutWithTracer() span.frames = [TestData.mainFrame, TestData.testFrame] let serialization = span.serialize() @@ -324,7 +336,7 @@ class SentrySpanTests: XCTestCase { } func testSanitizeDataSpan() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: fixture.someOperation, sampled: .undecided)) + let span = fixture.getSutWithTracer() span.setData(value: Date(timeIntervalSince1970: 10), key: "date") span.finish() @@ -365,7 +377,7 @@ class SentrySpanTests: XCTestCase { } func testTraceHeaderUndecided() { - let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: fixture.someOperation, sampled: .undecided)) + let span = fixture.getSutWithTracer() let header = span.toTraceHeader() XCTAssertEqual(header.traceId, span.traceId) @@ -376,7 +388,7 @@ class SentrySpanTests: XCTestCase { @available(*, deprecated) func testSetExtra_ForwardsToSetData() { - let sut = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: "test")) + let sut = fixture.getSutWithTracer() sut.setExtra(value: 0, key: "key") let data = sut.data as [String: Any] @@ -387,8 +399,13 @@ class SentrySpanTests: XCTestCase { // Span has a weak reference to tracer. If we don't keep a reference // to the tracer ARC will deallocate the tracer. let sutGenerator: () -> Span = { +#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + let tracer = SentryTracer(context: SpanContext(operation: "TEST"), framesTracker: nil) + return SentrySpan(tracer: tracer, context: SpanContext(operation: ""), framesTracker: nil) + #else let tracer = SentryTracer(context: SpanContext(operation: "TEST")) return SentrySpan(tracer: tracer, context: SpanContext(operation: "")) + #endif } let sut = sutGenerator() @@ -451,4 +468,79 @@ class SentrySpanTests: XCTestCase { XCTAssertEqual(nameForSentrySpanStatus(.outOfRange), kSentrySpanStatusNameOutOfRange) XCTAssertEqual(nameForSentrySpanStatus(.dataLoss), kSentrySpanStatusNameDataLoss) } + +#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + func testAddSlowFrozenFramesToData() { + let (displayLinkWrapper, framesTracker) = givenFramesTracker() + + let sut = SentrySpan(context: SpanContext(operation: "TEST"), framesTracker: framesTracker) + + let slow = 2 + let frozen = 1 + let normal = 100 + displayLinkWrapper.givenFrames(slow, frozen, normal) + + sut.finish() + + expect(sut.data["frames.total"] as? NSNumber) == NSNumber(value: slow + frozen + normal) + expect(sut.data["frames.slow"] as? NSNumber) == NSNumber(value: slow) + expect(sut.data["frames.frozen"] as? NSNumber) == NSNumber(value: frozen) + } + + func testDontAddAllZeroSlowFrozenFramesToData() { + let (_, framesTracker) = givenFramesTracker() + + let sut = SentrySpan(context: SpanContext(operation: "TEST"), framesTracker: framesTracker) + + sut.finish() + + expect(sut.data["frames.total"]) == nil + expect(sut.data["frames.slow"]) == nil + expect(sut.data["frames.frozen"]) == nil + } + + func testDontAddZeroSlowFrozenFrames_WhenSpanStartedBackgroundThread() { + let (displayLinkWrapper, framesTracker) = givenFramesTracker() + + let dispatchGroup = DispatchGroup() + dispatchGroup.enter() + DispatchQueue.global().async { + let sut = SentrySpan(context: SpanContext(operation: "TEST"), framesTracker: framesTracker) + + let slow = 2 + let frozen = 1 + let normal = 100 + displayLinkWrapper.givenFrames(slow, frozen, normal) + + sut.finish() + + expect(sut.data["frames.total"]) == nil + expect(sut.data["frames.slow"]) == nil + expect(sut.data["frames.frozen"]) == nil + + dispatchGroup.leave() + } + + dispatchGroup.wait() + } + + func testNoFramesTracker_NoFramesAddedToData() { + let sut = SentrySpan(context: SpanContext(operation: "TEST"), framesTracker: nil) + + sut.finish() + + expect(sut.data["frames.total"]) == nil + expect(sut.data["frames.slow"]) == nil + expect(sut.data["frames.frozen"]) == nil + } + + private func givenFramesTracker() -> (TestDisplayLinkWrapper, SentryFramesTracker) { + let displayLinkWrapper = TestDisplayLinkWrapper() + let framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper) + framesTracker.start() + displayLinkWrapper.call() + + return (displayLinkWrapper, framesTracker) + } +#endif }