Skip to content

Commit

Permalink
Discard unfinished spans when the app goes into the background
Browse files Browse the repository at this point in the history
  • Loading branch information
kstenerud committed Dec 14, 2023
1 parent b2663ef commit e0c1825
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 0 deletions.
12 changes: 12 additions & 0 deletions BugsnagPerformance.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@
094FA7522B10EEB600112ED4 /* BugsnagPerformance.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */; };
09509B752ADFE9A900A358EC /* TracerTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09509B742ADFE9A900A358EC /* TracerTests.mm */; };
096CBC172B1752F700534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 096CBC152B1752F100534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift */; };
0986B7C02B287C9D00BD2CA3 /* WeakSpansList.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */; };
09B473072B23087D0024CF11 /* WeakSpansList.h in Headers */ = {isa = PBXBuildFile; fileRef = 09B473052B23087D0024CF11 /* WeakSpansList.h */; };
09B4730A2B2313570024CF11 /* WeakSpansListTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09B473092B2313570024CF11 /* WeakSpansListTests.mm */; };
8A80DA8B2966CE940035BDA9 /* BugsnagPerformance.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */; };
8A80DA912966CEB30035BDA9 /* ConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A80DA80296588840035BDA9 /* ConfigurationTests.swift */; };
962F80F229DB03A400BE82BC /* PerformanceMicrobenchmarkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 962F80F129DB03A400BE82BC /* PerformanceMicrobenchmarkTests.swift */; };
Expand Down Expand Up @@ -272,6 +275,9 @@
094FA7422B10EDE700112ED4 /* BugsnagPerformanceSwiftUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BugsnagPerformanceSwiftUITests.swift; sourceTree = "<group>"; };
09509B742ADFE9A900A358EC /* TracerTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TracerTests.mm; sourceTree = "<group>"; };
096CBC152B1752F100534F0C /* BugsnagPerformanceSwiftUIInstrumentation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BugsnagPerformanceSwiftUIInstrumentation.swift; sourceTree = "<group>"; };
0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WeakSpansList.mm; sourceTree = "<group>"; };
09B473052B23087D0024CF11 /* WeakSpansList.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WeakSpansList.h; sourceTree = "<group>"; };
09B473092B2313570024CF11 /* WeakSpansListTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WeakSpansListTests.mm; sourceTree = "<group>"; };
72E4BB63359EA30E80116E2A /* BugsnagPerformance.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = BugsnagPerformance.framework; sourceTree = BUILT_PRODUCTS_DIR; };
8A80DA80296588840035BDA9 /* ConfigurationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationTests.swift; sourceTree = "<group>"; };
8A80DA872966CE940035BDA9 /* BugsnagPerformance-SwiftTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "BugsnagPerformance-SwiftTests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -540,6 +546,8 @@
CBF621052919418F004BEE0B /* Uploader.h */,
01EAF97F291AAF19007AC627 /* Utils.h */,
015836C3291264E0002F54C8 /* Version.h */,
09B473052B23087D0024CF11 /* WeakSpansList.h */,
0986B7BF2B287C9D00BD2CA3 /* WeakSpansList.mm */,
CBE8EA18294B5AB800702950 /* Worker.h */,
CBE8EA19294B5AB800702950 /* Worker.mm */,
);
Expand Down Expand Up @@ -605,6 +613,7 @@
CB2B8A9A2A0924A50054FBBE /* SpanTests.mm */,
CB747D20299E5458003CA1B4 /* TimeTests.mm */,
09509B742ADFE9A900A358EC /* TracerTests.mm */,
09B473092B2313570024CF11 /* WeakSpansListTests.mm */,
CB0AD75A295F27DD002A3FB6 /* WorkerTests.mm */,
);
path = BugsnagPerformanceTests;
Expand Down Expand Up @@ -735,6 +744,7 @@
CBE0873329FA984C007455F2 /* BugsnagPerformanceViewType+Private.h in Headers */,
CBEC51D62976BCAD009C0CE3 /* Filesystem.h in Headers */,
CBEC51C0296DB312009C0CE3 /* JSON.h in Headers */,
09B473072B23087D0024CF11 /* WeakSpansList.h in Headers */,
0921F02B2A67CBD600C764EB /* BugsnagPerformanceNetworkRequestInfo.h in Headers */,
0122C24A29019770002D243C /* AppStartupInstrumentation.h in Headers */,
CB04969B2915194E0097E526 /* OtlpUploader.h in Headers */,
Expand Down Expand Up @@ -1013,6 +1023,7 @@
CB0AD75B295F27DD002A3FB6 /* WorkerTests.mm in Sources */,
CBEC51C3296EC0AC009C0CE3 /* JSONTests.mm in Sources */,
01A414C52912B93F003152A4 /* ResourceAttributesTests.mm in Sources */,
09B4730A2B2313570024CF11 /* WeakSpansListTests.mm in Sources */,
CB2B8A9B2A0924A50054FBBE /* SpanTests.mm in Sources */,
CBEC51CE296EEF52009C0CE3 /* PersistenceTests.mm in Sources */,
96D55C882A1EE26A006D1F29 /* SpanStackingHandlerTests.mm in Sources */,
Expand Down Expand Up @@ -1074,6 +1085,7 @@
CBEC51C1296DB312009C0CE3 /* JSON.mm in Sources */,
CBEBE59B29F671A800BF0B4F /* Instrumentation.mm in Sources */,
0122C24029019770002D243C /* SpanData.mm in Sources */,
0986B7C02B287C9D00BD2CA3 /* WeakSpansList.mm in Sources */,
96D4160C29F276E400AEE435 /* SpanAttributesProvider.mm in Sources */,
CB68FABA2A3C27B9005B2CDB /* PersistentDeviceID.mm in Sources */,
CBF7C5E2297A8E9100D47719 /* Gzip.m in Sources */,
Expand Down
1 change: 1 addition & 0 deletions Sources/BugsnagPerformance/Private/AppStateTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ NS_ASSUME_NONNULL_BEGIN
@property(nonatomic,readonly) BOOL isInForeground;

@property(nonatomic,readwrite,strong) void (^onTransitionToForeground)(void);
@property(nonatomic,readwrite,strong) void (^onTransitionToBackground)(void);

@end

Expand Down
4 changes: 4 additions & 0 deletions Sources/BugsnagPerformance/Private/AppStateTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ - (void) handleAppForegroundEvent {
- (void) handleAppBackgroundEvent {
#pragma clang diagnostic ignored "-Wdirect-ivar-access"
_isInForeground = NO;
void (^callback)(void) = self.onTransitionToBackground;
if (callback != nil) {
callback();
}
}

@end
2 changes: 2 additions & 0 deletions Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class BugsnagPerformanceImpl: public PhasedStartup {
bool sendCurrentBatchTask() noexcept;
bool sendRetriesTask() noexcept;
bool sendPValueRequestTask() noexcept;
bool sweepTracerTask() noexcept;

// Event reactions
void onBatchFull() noexcept;
Expand All @@ -109,6 +110,7 @@ class BugsnagPerformanceImpl: public PhasedStartup {
void onFilesystemError() noexcept;
void onWorkInterval() noexcept;
void onAppEnteredForeground() noexcept;
void onAppEnteredBackground() noexcept;

// Utility
void wakeWorker() noexcept;
Expand Down
14 changes: 14 additions & 0 deletions Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@
appStateTracker_.onTransitionToForeground = ^{
blockThis->onAppEnteredForeground();
};
appStateTracker_.onTransitionToBackground = ^{
blockThis->onAppEnteredBackground();
};

tracer_->start();

Expand Down Expand Up @@ -174,6 +177,7 @@
return @[
^bool() { return blockThis->sendCurrentBatchTask(); },
^bool() { return blockThis->sendRetriesTask(); },
^bool() { return blockThis->sweepTracerTask(); },
];
}

Expand Down Expand Up @@ -212,6 +216,12 @@
return false;
}

bool BugsnagPerformanceImpl::sweepTracerTask() noexcept {
tracer_->sweep();
// Never auto-repeat this task, even if work was done; it can wait.
return false;
}

#pragma mark Event Reactions

void BugsnagPerformanceImpl::onFilesystemError() noexcept {
Expand Down Expand Up @@ -258,6 +268,10 @@
wakeWorker();
}

void BugsnagPerformanceImpl::onAppEnteredBackground() noexcept {
tracer_->abortAllOpenSpans();
}

#pragma mark Utility

void BugsnagPerformanceImpl::wakeWorker() noexcept {
Expand Down
10 changes: 10 additions & 0 deletions Sources/BugsnagPerformance/Private/Tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#import "PhasedStartup.h"
#import "SpanAttributesProvider.h"
#import "SpanStackingHandler.h"
#import "WeakSpansList.h"

#import <memory>

Expand Down Expand Up @@ -59,6 +60,11 @@ class Tracer: public PhasedStartup {
void cancelQueuedSpan(BugsnagPerformanceSpan *span) noexcept;

void onPrewarmPhaseEnded(void) noexcept;

void abortAllOpenSpans() noexcept;

// Sweep must be called periodically to avoid a buildup of dead pointers.
void sweep() noexcept;

private:
Tracer() = delete;
Expand All @@ -73,6 +79,10 @@ class Tracer: public PhasedStartup {
std::mutex prewarmSpansMutex_;
NSMutableArray<BugsnagPerformanceSpan *> *prewarmSpans_;

// Sloppy list of "open" spans. Some spans may have already been closed,
// but span abort/end are idempotent so it doesn't matter.
std::shared_ptr<WeakSpansList> potentiallyOpenSpans_;

std::shared_ptr<Batch> batch_;
void (^onSpanStarted_)(){ ^(){} };
std::function<void(NSString *)> onViewLoadSpanStarted_{ [](NSString *){} };
Expand Down
15 changes: 15 additions & 0 deletions Sources/BugsnagPerformance/Private/Tracer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
, sampler_(sampler)
, earlyNetworkSpans_([NSMutableArray new])
, prewarmSpans_([NSMutableArray new])
, potentiallyOpenSpans_(std::make_shared<WeakSpansList>())
, batch_(batch)
, onSpanStarted_(onSpanStarted)
{}
Expand Down Expand Up @@ -54,6 +55,19 @@
}
}

void
Tracer::abortAllOpenSpans() noexcept {
potentiallyOpenSpans_->abortAll();
}

void
Tracer::sweep() noexcept {
constexpr unsigned minEntriesBeforeCompacting = 10000;
if (potentiallyOpenSpans_->count() >= minEntriesBeforeCompacting) {
potentiallyOpenSpans_->compact();
}
}

BugsnagPerformanceSpan *
Tracer::startSpan(NSString *name, SpanOptions options, BSGFirstClass defaultFirstClass) noexcept {
__block auto blockThis = this;
Expand Down Expand Up @@ -84,6 +98,7 @@
spanStackingHandler_->push(span);
}
[span addAttributes:SpanAttributes::get()];
potentiallyOpenSpans_->add(span);
onSpanStarted_();
return span;
}
Expand Down
74 changes: 74 additions & 0 deletions Sources/BugsnagPerformance/Private/WeakSpansList.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//
// WeakSpansList.h
// BugsnagPerformance-iOS
//
// Created by Karl Stenerud on 08.12.23.
// Copyright © 2023 Bugsnag. All rights reserved.
//

#import <BugsnagPerformance/BugsnagPerformanceSpan.h>
#import <mutex>

@interface BSGWeakSpanPointer: NSObject

// We use a weak wrapper because NSPointerArray.weakObjectsPointerArray's compact method is broken.
@property(nonatomic,readonly,weak) BugsnagPerformanceSpan *span;

- (instancetype) initWithSpan:(BugsnagPerformanceSpan *)span;

+ (instancetype) pointerWithSpan:(BugsnagPerformanceSpan *)span;

@end

namespace bugsnag {

class WeakSpansList {
public:
WeakSpansList()
: spans_([NSMutableArray new])
{}

void add(BugsnagPerformanceSpan *span) noexcept {
auto ptr = [BSGWeakSpanPointer pointerWithSpan:span];
std::lock_guard<std::mutex> guard(mutex_);
[spans_ addObject:ptr];
}

void compact() noexcept {
std::lock_guard<std::mutex> guard(mutex_);
bool canCompact = false;
for (BSGWeakSpanPointer *ptr in spans_) {
if (!ptr.span.isValid) {
canCompact = true;
break;
}
}
if (canCompact) {
auto newSpans = [NSMutableArray arrayWithCapacity:spans_.count];
for (BSGWeakSpanPointer *ptr in spans_) {
if (ptr.span.isValid) {
[newSpans addObject:ptr];
}
}
spans_ = newSpans;
}
}

void abortAll() noexcept {
std::lock_guard<std::mutex> guard(mutex_);
for (BSGWeakSpanPointer *ptr in spans_) {
[ptr.span abort];
}
[spans_ removeAllObjects];
}

NSUInteger count() noexcept {
return spans_.count;
}

private:
std::mutex mutex_;
NSMutableArray *spans_;
};

}
24 changes: 24 additions & 0 deletions Sources/BugsnagPerformance/Private/WeakSpansList.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//
// WeakSpansList.c
// BugsnagPerformance-iOS
//
// Created by Karl Stenerud on 12.12.23.
// Copyright © 2023 Bugsnag. All rights reserved.
//

#import "WeakSpansList.h"

@implementation BSGWeakSpanPointer

- (instancetype) initWithSpan:(BugsnagPerformanceSpan *)span {
if ((self = [super init])) {
_span = span;
}
return self;
}

+ (instancetype) pointerWithSpan:(BugsnagPerformanceSpan *)span {
return [[self alloc] initWithSpan:span];
}

@end
70 changes: 70 additions & 0 deletions Tests/BugsnagPerformanceTests/WeakSpansListTests.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//
// WeakSpansListTests.m
// BugsnagPerformance-iOSTests
//
// Created by Karl Stenerud on 08.12.23.
// Copyright © 2023 Bugsnag. All rights reserved.
//

#import <XCTest/XCTest.h>
#import "WeakSpansList.h"
#import "BugsnagPerformanceSpan+Private.h"
#import "SpanOptions.h"

using namespace bugsnag;

@interface WeakSpansListTests : XCTestCase

@end

static BugsnagPerformanceSpan *createSpan() {
return [[BugsnagPerformanceSpan alloc]
initWithSpan:std::make_unique<Span>(@"test",
IdGenerator::generateTraceId(),
IdGenerator::generateSpanId(),
IdGenerator::generateSpanId(),
SpanOptions().startTime,
BSGFirstClassNo,
^void(std::shared_ptr<SpanData>) {
})];
}

@implementation WeakSpansListTests

- (void)testAllReleased {
WeakSpansList list;
@autoreleasepool {
list.add(createSpan());
list.add(createSpan());
list.add(createSpan());
}
XCTAssertEqual(3U, list.count());
list.compact();
XCTAssertEqual(0U, list.count());
}

- (void)testNoneReleased {
WeakSpansList list;
list.add(createSpan());
list.add(createSpan());
list.add(createSpan());
XCTAssertEqual(3U, list.count());
list.compact();
XCTAssertEqual(3U, list.count());
}

- (void)testSomeReleased {
WeakSpansList list;
list.add(createSpan());
@autoreleasepool {
list.add(createSpan());
list.add(createSpan());
list.add(createSpan());
}
list.add(createSpan());
XCTAssertEqual(5U, list.count());
list.compact();
XCTAssertEqual(2U, list.count());
}

@end

0 comments on commit e0c1825

Please sign in to comment.