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

[PLAT-11315] Discard unfinished spans when the app goes into the background #228

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
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
Loading