Skip to content

Commit 533a6e0

Browse files
robert-smartbearRobert Bartoszewski
andauthored
Span creation code refactor (#479)
* Moved span creation code to factories. Made sure all spans have attributes set before onStart callbacks are called * Passing SpanOption by reference * Updated the Changelog --------- Co-authored-by: Robert Bartoszewski <robert.smartbear@gmail.com>
1 parent f52c502 commit 533a6e0

35 files changed

+940
-364
lines changed

BugsnagPerformance.xcodeproj/project.pbxproj

Lines changed: 84 additions & 40 deletions
Large diffs are not rendered by default.

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ Changelog
99

1010
### Bug fixes
1111

12+
* Fixed an issue causing onSpanStart callbacks being called before all initial attributes have been set.
13+
[479](https://github.com/bugsnag/bugsnag-cocoa-performance/pull/479)
14+
1215
* Fixed an issue allowing conditions to change spans endDate to an earlier date.
1316
[478](https://github.com/bugsnag/bugsnag-cocoa-performance/pull/478)
1417

Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
#import "SystemInfoSampler.h"
3333
#import "SpanControl/BSGCompositeSpanControlProvider.h"
3434
#import "BSGPluginManager.h"
35+
#import "SpanFactory/AppStartup/AppStartupSpanFactoryImpl.h"
36+
#import "SpanFactory/ViewLoad/ViewLoadSpanFactoryImpl.h"
37+
#import "SpanFactory/Network/NetworkSpanFactoryImpl.h"
3538

3639
#import <mutex>
3740

@@ -101,6 +104,10 @@ class BugsnagPerformanceImpl: public PhasedStartup {
101104
BSGCompositeSpanControlProvider *spanControlProvider_;
102105
BSGPrioritizedStore<BugsnagPerformanceSpanStartCallback> *spanStartCallbacks_;
103106
BSGPrioritizedStore<BugsnagPerformanceSpanEndCallback> *spanEndCallbacks_;
107+
std::shared_ptr<PlainSpanFactoryImpl> plainSpanFactory_;
108+
std::shared_ptr<AppStartupSpanFactoryImpl> appStartupSpanFactory_;
109+
std::shared_ptr<ViewLoadSpanFactoryImpl> viewLoadSpanFactory_;
110+
std::shared_ptr<NetworkSpanFactoryImpl> networkSpanFactory_;
104111
std::shared_ptr<Tracer> tracer_;
105112
std::unique_ptr<RetryQueue> retryQueue_;
106113
AppStateTracker *appStateTracker_;
@@ -151,7 +158,6 @@ class BugsnagPerformanceImpl: public PhasedStartup {
151158
void wakeWorker() noexcept;
152159
void uploadPValueRequest() noexcept;
153160
void uploadPackage(std::unique_ptr<OtlpPackage> package, bool isRetry) noexcept;
154-
void possiblyMakeSpanCurrent(BugsnagPerformanceSpan *span, SpanOptions &options);
155161
NSMutableArray<BugsnagPerformanceSpan *> *
156162
sendableSpans(NSMutableArray<BugsnagPerformanceSpan *> *spans) noexcept;
157163
bool shouldSampleCPU(BugsnagPerformanceSpan *span) noexcept;

Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,27 @@
4444
, spanStackingHandler_(std::make_shared<SpanStackingHandler>())
4545
, reachability_(reachability)
4646
, batch_(std::make_shared<Batch>())
47-
, spanAttributesProvider_(std::make_shared<SpanAttributesProvider>())
4847
, sampler_(std::make_shared<Sampler>())
48+
, spanAttributesProvider_(std::make_shared<SpanAttributesProvider>())
4949
, networkHeaderInjector_(std::make_shared<NetworkHeaderInjector>(spanAttributesProvider_, spanStackingHandler_, sampler_))
5050
, frameMetricsCollector_([FrameMetricsCollector new])
5151
, conditionTimeoutExecutor_(std::make_shared<ConditionTimeoutExecutor>())
5252
, spanControlProvider_([BSGCompositeSpanControlProvider new])
5353
, spanStartCallbacks_([BSGPrioritizedStore<BugsnagPerformanceSpanStartCallback> new])
5454
, spanEndCallbacks_([BSGPrioritizedStore<BugsnagPerformanceSpanEndCallback> new])
55-
, tracer_(std::make_shared<Tracer>(spanStackingHandler_, sampler_, batch_, frameMetricsCollector_, conditionTimeoutExecutor_, spanAttributesProvider_, spanStartCallbacks_, spanEndCallbacks_, ^{this->onSpanStarted();}))
55+
, plainSpanFactory_(std::make_shared<PlainSpanFactoryImpl>(sampler_, spanStackingHandler_, spanAttributesProvider_))
56+
, appStartupSpanFactory_(std::make_shared<AppStartupSpanFactoryImpl>(plainSpanFactory_, spanAttributesProvider_))
57+
, viewLoadSpanFactory_(std::make_shared<ViewLoadSpanFactoryImpl>(plainSpanFactory_, spanAttributesProvider_))
58+
, networkSpanFactory_(std::make_shared<NetworkSpanFactoryImpl>(plainSpanFactory_, spanAttributesProvider_))
59+
, tracer_(std::make_shared<Tracer>(spanStackingHandler_, sampler_, batch_, frameMetricsCollector_, conditionTimeoutExecutor_, plainSpanFactory_, viewLoadSpanFactory_, networkSpanFactory_, spanStartCallbacks_, spanEndCallbacks_, ^{this->onSpanStarted();}))
5660
, retryQueue_(std::make_unique<RetryQueue>([persistence_->bugsnagPerformanceDir() stringByAppendingPathComponent:@"retry-queue"]))
5761
, appStateTracker_(appStateTracker)
5862
, viewControllersToSpans_([NSMapTable mapTableWithKeyOptions:NSMapTableWeakMemory | NSMapTableObjectPointerPersonality
5963
valueOptions:NSMapTableStrongMemory])
6064
, instrumentation_(std::make_shared<Instrumentation>(tracer_,
65+
appStartupSpanFactory_,
66+
viewLoadSpanFactory_,
67+
networkSpanFactory_,
6168
spanAttributesProvider_,
6269
networkHeaderInjector_))
6370
, worker_([[Worker alloc] initWithInitialTasks:buildInitialTasks() recurringTasks:buildRecurringTasks()])

Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation/Lifecycle/AppStartupLifecycleHandlerImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//
88

99
#import "AppStartupLifecycleHandler.h"
10-
#import "../SpanFactory/AppStartupSpanFactory.h"
10+
#import "../../../SpanFactory/AppStartup/AppStartupSpanFactory.h"
1111
#import "../System/AppStartupInstrumentationSystemUtils.h"
1212
#import "../../../SpanAttributesProvider.h"
1313
#import "../../../BugsnagPerformanceCrossTalkAPI.h"

Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation/Lifecycle/AppStartupLifecycleHandlerImpl.mm

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//
88

99
#import "AppStartupLifecycleHandlerImpl.h"
10-
#import "../SpanFactory/AppStartupSpanFactory.h"
10+
#import "../../../SpanFactory/AppStartup/AppStartupSpanFactory.h"
1111
#import "../../../SpanAttributesProvider.h"
1212
#import "../../../BugsnagPerformanceSpan+Private.h"
1313

@@ -119,9 +119,9 @@ static bool isAppStartInProgress(AppStartupInstrumentationState *state) noexcept
119119
return;
120120
}
121121

122-
state.appStartSpan = spanFactory_->startAppStartSpan(state.didStartProcessAtTime,
123-
state.isColdLaunch,
124-
state.firstViewName);
122+
state.appStartSpan = spanFactory_->startAppStartOverallSpan(state.didStartProcessAtTime,
123+
state.isColdLaunch,
124+
state.firstViewName);
125125
}
126126

127127
void

Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation/SpanFactory/AppStartupSpanFactoryImpl.h

Lines changed: 0 additions & 33 deletions
This file was deleted.

Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation/SpanFactory/AppStartupSpanFactoryImpl.mm

Lines changed: 0 additions & 62 deletions
This file was deleted.

Sources/BugsnagPerformance/Private/Instrumentation/Instrumentation.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,24 @@
1414
#import "AppStartupInstrumentation/AppStartupInstrumentation.h"
1515
#import "NetworkInstrumentation/NetworkInstrumentation.h"
1616
#import "AppStartupInstrumentation/System/AppStartupInstrumentationSystemUtilsImpl.h"
17-
#import "AppStartupInstrumentation/SpanFactory/AppStartupSpanFactoryImpl.h"
17+
#import "../SpanFactory/AppStartup/AppStartupSpanFactoryImpl.h"
1818
#import "AppStartupInstrumentation/Lifecycle/AppStartupLifecycleHandlerImpl.h"
1919
#import "ViewLoadInstrumentation/ViewLoadInstrumentation.h"
2020
#import "NetworkInstrumentation/System/NetworkHeaderInjector.h"
21+
#import "../SpanFactory/AppStartup/AppStartupSpanFactory.h"
22+
#import "../SpanFactory/ViewLoad/ViewLoadSpanFactory.h"
23+
#import "../SpanFactory/Network/NetworkSpanFactory.h"
24+
25+
std::shared_ptr<AppStartupInstrumentation> createAppStartupInstrumentation(std::shared_ptr<Tracer> tracer,
26+
std::shared_ptr<AppStartupSpanFactory> spanFactory,
27+
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider);
2128

2229
std::shared_ptr<ViewLoadInstrumentation> createViewLoadInstrumentation(std::shared_ptr<Tracer> tracer,
30+
std::shared_ptr<ViewLoadSpanFactory> spanFactory,
2331
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider);
2432

2533
std::shared_ptr<NetworkInstrumentation> createNetworkInstrumentation(std::shared_ptr<Tracer> tracer,
34+
std::shared_ptr<NetworkSpanFactory> spanFactory,
2635
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider,
2736
std::shared_ptr<NetworkHeaderInjector> networkHeaderInjector);
2837

@@ -31,11 +40,14 @@ namespace bugsnag {
3140
class Instrumentation: public PhasedStartup {
3241
public:
3342
Instrumentation(std::shared_ptr<Tracer> tracer,
43+
std::shared_ptr<AppStartupSpanFactory> appStartupSpanFactory,
44+
std::shared_ptr<ViewLoadSpanFactory> viewLoadSpanFactory,
45+
std::shared_ptr<NetworkSpanFactory> networkSpanFactory,
3446
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider,
3547
std::shared_ptr<NetworkHeaderInjector> networkHeaderInjector) noexcept
36-
: appStartupInstrumentation_(std::make_shared<AppStartupInstrumentation>(std::make_shared<AppStartupLifecycleHandlerImpl>(std::make_shared<AppStartupSpanFactoryImpl>(tracer, spanAttributesProvider), spanAttributesProvider, tracer, std::make_shared<AppStartupInstrumentationSystemUtilsImpl>(), [BugsnagPerformanceCrossTalkAPI sharedInstance]), std::make_shared<AppStartupInstrumentationSystemUtilsImpl>()))
37-
, viewLoadInstrumentation_(createViewLoadInstrumentation(tracer, spanAttributesProvider))
38-
, networkInstrumentation_(createNetworkInstrumentation(tracer, spanAttributesProvider, networkHeaderInjector))
48+
: appStartupInstrumentation_(createAppStartupInstrumentation(tracer, appStartupSpanFactory, spanAttributesProvider))
49+
, viewLoadInstrumentation_(createViewLoadInstrumentation(tracer, viewLoadSpanFactory, spanAttributesProvider))
50+
, networkInstrumentation_(createNetworkInstrumentation(tracer, networkSpanFactory, spanAttributesProvider, networkHeaderInjector))
3951
{
4052
tracer->setGetAppStartInstrumentationState([=]{ return appStartupInstrumentation_->stateSnapshot(); });
4153
}

Sources/BugsnagPerformance/Private/Instrumentation/Instrumentation.mm

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
#import "Instrumentation.h"
1010

11-
#import "ViewLoadInstrumentation/SpanFactory/ViewLoadSpanFactoryImpl.h"
11+
#import "../SpanFactory/Network/NetworkSpanFactoryImpl.h"
12+
#import "../SpanFactory/ViewLoad/ViewLoadSpanFactoryImpl.h"
1213
#import "ViewLoadInstrumentation/System/ViewLoadInstrumentationSystemUtilsImpl.h"
1314
#import "ViewLoadInstrumentation/System/ViewLoadSwizzlingHandlerImpl.h"
1415
#import "ViewLoadInstrumentation/State/ViewLoadInstrumentationStateRepositoryImpl.h"
@@ -21,7 +22,6 @@
2122
#import "NetworkInstrumentation/System/NetworkSwizzlingHandlerImpl.h"
2223
#import "NetworkInstrumentation/Lifecycle/NetworkEarlyPhaseHandlerImpl.h"
2324
#import "NetworkInstrumentation/Lifecycle/NetworkLifecycleHandlerImpl.h"
24-
#import "NetworkInstrumentation/SpanFactory/NetworkSpanFactoryImpl.h"
2525

2626
using namespace bugsnag;
2727

@@ -61,11 +61,24 @@
6161

6262
#pragma mark - Factory functions
6363

64+
std::shared_ptr<AppStartupInstrumentation> createAppStartupInstrumentation(std::shared_ptr<Tracer> tracer,
65+
std::shared_ptr<AppStartupSpanFactory> spanFactory,
66+
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider) {
67+
auto systemUtils = std::make_shared<AppStartupInstrumentationSystemUtilsImpl>();
68+
auto lifecycleHandler = std::make_shared<AppStartupLifecycleHandlerImpl>(spanFactory,
69+
spanAttributesProvider,
70+
tracer,
71+
systemUtils,
72+
[BugsnagPerformanceCrossTalkAPI sharedInstance]);
73+
74+
return std::make_shared<AppStartupInstrumentation>(lifecycleHandler, systemUtils);
75+
}
76+
6477
std::shared_ptr<ViewLoadInstrumentation> createViewLoadInstrumentation(std::shared_ptr<Tracer> tracer,
78+
std::shared_ptr<ViewLoadSpanFactory> spanFactory,
6579
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider) {
6680
auto systemUtils = std::make_shared<ViewLoadInstrumentationSystemUtilsImpl>();
6781
auto swizzlingHandler = std::make_shared<ViewLoadSwizzlingHandlerImpl>();
68-
auto spanFactory = std::make_shared<ViewLoadSpanFactoryImpl>(tracer, spanAttributesProvider);
6982
auto repository = std::make_shared<ViewLoadInstrumentationStateRepositoryImpl>();
7083
auto earlyPhaseHandler = std::make_shared<ViewLoadEarlyPhaseHandlerImpl>(tracer);
7184
auto loadingIndicatorsHandler = std::make_shared<ViewLoadLoadingIndicatorsHandlerImpl>(repository);
@@ -80,13 +93,13 @@
8093
}
8194

8295
std::shared_ptr<NetworkInstrumentation> createNetworkInstrumentation(std::shared_ptr<Tracer> tracer,
96+
std::shared_ptr<NetworkSpanFactory> spanFactory,
8397
std::shared_ptr<SpanAttributesProvider> spanAttributesProvider,
8498
std::shared_ptr<NetworkHeaderInjector> networkHeaderInjector) {
8599
auto repository = std::make_shared<NetworkInstrumentationStateRepositoryImpl>();
86100
auto systemUtils = std::make_shared<NetworkInstrumentationSystemUtilsImpl>();
87101
auto swizzlingHandler = std::make_shared<NetworkSwizzlingHandlerImpl>();
88102
auto earlyPhaseHandler = std::make_shared<NetworkEarlyPhaseHandlerImpl>(tracer, spanAttributesProvider);
89-
auto spanFactory = std::make_shared<NetworkSpanFactoryImpl>(tracer, spanAttributesProvider);
90103
auto lifecycleHandler = std::make_shared<NetworkLifecycleHandlerImpl>(tracer,
91104
spanAttributesProvider,
92105
spanFactory,

0 commit comments

Comments
 (0)