Skip to content

Commit

Permalink
Merge pull request #380 from bugsnag/PLAT-13438-tri-state-type
Browse files Browse the repository at this point in the history
Replace tri-state types with a single type, deprecating their uses in public APIs
  • Loading branch information
kstenerud authored Jan 27, 2025
2 parents 4b488d4 + 27c404c commit 7cdc1f6
Show file tree
Hide file tree
Showing 28 changed files with 330 additions and 141 deletions.
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
source 'https://rubygems.org'

gem 'bugsnag-maze-runner', '~>9.0'
gem 'bugsnag-maze-runner', '~>9.22.0'
gem 'cocoapods'
gem 'xcpretty'
gem 'xcpretty', '~>0.3.0'

#gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner', branch: 'integration/v8'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ - (BugsnagPerformanceSpan * _Nullable)startSpanV1:(NSString * _Nonnull)name opti
}

auto options = SpanOptions(optionsIn);
auto span = tracer->startSpan(name, options, BSGFirstClassUnset);
auto span = tracer->startSpan(name, options, BSGTriStateUnset);
return (BugsnagPerformanceSpan *)[BugsnagPerformanceCrossTalkProxiedObject proxied:span];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#import "BugsnagPerformanceSpanContext+Private.h"
#import "SpanKind.h"
#import "FrameRateMetrics/FrameMetricsSnapshot.h"
#import "SpanOptions.h"

#import <memory>

Expand All @@ -35,6 +36,12 @@ NS_ASSUME_NONNULL_BEGIN

@property(nonatomic, copy) void (^onDumped)(BugsnagPerformanceSpan *);

// These mark the actual times that the span was instantiated and ended,
// irrespective of any time values this span will report.
// We need this because we're recording metrics data in spans instead of metrics.
@property (nonatomic) CFAbsoluteTime actuallyStartedAt;
@property (nonatomic) CFAbsoluteTime actuallyEndedAt;

@property (nonatomic) CFAbsoluteTime startAbsTime;
@property (nonatomic) CFAbsoluteTime endAbsTime;
@property (nonatomic) OnSpanDestroyAction onSpanDestroyAction;
Expand All @@ -44,13 +51,13 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic) SpanLifecycleCallback onSpanClosed;
@property (nonatomic,readwrite) SpanId parentId;
@property (nonatomic) double samplingProbability;
@property (nonatomic) BSGFirstClass firstClass;
@property (nonatomic) BSGTriState firstClass;
@property (nonatomic) SpanKind kind;
@property (nonatomic,readwrite) BOOL isMutable;
@property (nonatomic,readwrite) BOOL hasBeenProcessed;
@property (nonatomic,readonly) NSUInteger attributeCountLimit;
@property (nonatomic,readwrite) BOOL wasStartOrEndTimeProvided;
@property (nonatomic) BSGInstrumentRendering instrumentRendering;
@property (nonatomic) MetricsOptions metricsOptions;
@property (nonatomic, strong) FrameMetricsSnapshot *startFramerateSnapshot;
@property (nonatomic, strong) FrameMetricsSnapshot *endFramerateSnapshot;

Expand All @@ -65,9 +72,9 @@ NS_ASSUME_NONNULL_BEGIN
spanId:(SpanId) spanId
parentId:(SpanId) parentId
startTime:(CFAbsoluteTime) startTime
firstClass:(BSGFirstClass) firstClass
firstClass:(BSGTriState) firstClass
attributeCountLimit:(NSUInteger)attributeCountLimit
instrumentRendering:(BSGInstrumentRendering)instrumentRendering
metricsOptions:(MetricsOptions) metricsOptions
onSpanEndSet:(SpanLifecycleCallback) onSpanEndSet
onSpanClosed:(SpanLifecycleCallback) onSpanEnded NS_DESIGNATED_INITIALIZER;

Expand All @@ -87,6 +94,8 @@ NS_ASSUME_NONNULL_BEGIN
- (void)updateStartTime:(NSDate *)startTime;
- (void)updateSamplingProbability:(double) value;

- (void)forceMutate:(void (^)())block;

@end

NS_ASSUME_NONNULL_END
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ - (void)earlyConfigure:(BSGEarlyConfiguration *)config {}
- (void)earlySetup {}

- (void)configure:(BugsnagPerformanceConfiguration *)config {
self.autoInstrumentRendering = config.autoInstrumentRendering;
self.autoInstrumentRendering = config.enabledMetrics.rendering;
}

- (void)start {
Expand Down
28 changes: 28 additions & 0 deletions Sources/BugsnagPerformance/Private/Metrics.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//
// Metrics.h
// BugsnagPerformance
//
// Created by Karl Stenerud on 20.01.25.
// Copyright © 2025 Bugsnag. All rights reserved.
//

#pragma once

#import <BugsnagPerformance/BugsnagPerformanceConfiguration.h>

namespace bugsnag {

class MetricsOptions {
public:
MetricsOptions() {}

MetricsOptions(BugsnagPerformanceSpanMetricsOptions *metrics)
: rendering(metrics.rendering)
, cpu(metrics.cpu)
{}

BSGTriState rendering{BSGTriStateUnset};
BSGTriState cpu{BSGTriStateUnset};
};

};
17 changes: 9 additions & 8 deletions Sources/BugsnagPerformance/Private/SpanOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#import <BugsnagPerformance/BugsnagPerformanceSpan.h>
#import <BugsnagPerformance/BugsnagPerformanceSpanOptions.h>
#import "Utils.h"
#import "Metrics.h"

namespace bugsnag {

Expand All @@ -17,37 +18,37 @@ class SpanOptions {
SpanOptions(BugsnagPerformanceSpanContext *parentContext,
CFAbsoluteTime startTime,
bool makeCurrentContext,
BSGFirstClass firstClass,
BSGInstrumentRendering instrumentRendering)
BSGTriState firstClass,
MetricsOptions metricsOptions)
: parentContext(parentContext)
, startTime(startTime)
, makeCurrentContext(makeCurrentContext)
, firstClass(firstClass)
, instrumentRendering(instrumentRendering)
, metricsOptions(metricsOptions)
{}

SpanOptions(BugsnagPerformanceSpanOptions *options)
: SpanOptions(options.parentContext,
dateToAbsoluteTime(options.startTime),
options.makeCurrentContext,
options.firstClass,
options.instrumentRendering)
options.metricsOptions)
{}

SpanOptions()
// These defaults must match the defaults in BugsnagPerformanceSpanOptions.m
: SpanOptions(nil,
CFABSOLUTETIME_INVALID,
true,
BSGFirstClassUnset,
BSGInstrumentRenderingUnset)
BSGTriStateUnset,
MetricsOptions())
{}

BugsnagPerformanceSpanContext *parentContext{nil};
CFAbsoluteTime startTime{CFABSOLUTETIME_INVALID};
bool makeCurrentContext{false};
BSGFirstClass firstClass{BSGFirstClassUnset};
BSGInstrumentRendering instrumentRendering{BSGInstrumentRenderingUnset};
BSGTriState firstClass{BSGTriStateUnset};
MetricsOptions metricsOptions;
};

}
8 changes: 4 additions & 4 deletions Sources/BugsnagPerformance/Private/Tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Tracer: public PhasedStartup {
void configure(BugsnagPerformanceConfiguration *config) noexcept {
onSpanEndCallbacks_ = config.onSpanEndCallbacks;
attributeCountLimit_ = config.attributeCountLimit;
autoInstrumentRendering_ = config.autoInstrumentRendering;
enabledMetrics_ = [config.enabledMetrics clone];
};
void preStartSetup() noexcept;
void start() noexcept {}
Expand All @@ -49,8 +49,8 @@ class Tracer: public PhasedStartup {
onViewLoadSpanStarted_ = onViewLoadSpanStarted;
}

BugsnagPerformanceSpan *startSpan(NSString *name, SpanOptions options, BSGFirstClass defaultFirstClass) noexcept;
BugsnagPerformanceSpan *startSpan(NSString *name, SpanOptions options, BSGTriState defaultFirstClass) noexcept;

BugsnagPerformanceSpan *startAppStartSpan(NSString *name, SpanOptions options) noexcept;

BugsnagPerformanceSpan *startCustomSpan(NSString *name, SpanOptions options) noexcept;
Expand Down Expand Up @@ -83,7 +83,7 @@ class Tracer: public PhasedStartup {
FrameMetricsCollector *frameMetricsCollector_;

std::atomic<bool> willDiscardPrewarmSpans_{false};
std::atomic<bool> autoInstrumentRendering_{false};
BugsnagPerformanceEnabledMetrics *enabledMetrics_{nil};
std::mutex prewarmSpansMutex_;
NSMutableArray<BugsnagPerformanceSpan *> *prewarmSpans_;
NSArray<BugsnagPerformanceSpanEndCallback> *onSpanEndCallbacks_;
Expand Down
48 changes: 25 additions & 23 deletions Sources/BugsnagPerformance/Private/Tracer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#import "Instrumentation/ViewLoadInstrumentation.h"
#import "BugsnagPerformanceLibrary.h"
#import "FrameRateMetrics/FrameMetricsCollector.h"
#import <algorithm>

using namespace bugsnag;

Expand Down Expand Up @@ -59,10 +60,10 @@
[span abortUnconditionally];
continue;
}
span.isMutable = true;
[span updateSamplingProbability:sampler_->getProbability()];
callOnSpanEndCallbacks(span);
span.isMutable = false;
[span forceMutate:^() {
[span updateSamplingProbability:sampler_->getProbability()];
callOnSpanEndCallbacks(span);
}];
if (span.state == SpanStateAborted) {
BSGLogDebug(@"Tracer::reprocessEarlySpans: span %@ was rejected in the OnEnd callbacks, so dropping", span.name);
[span abortUnconditionally];
Expand All @@ -89,7 +90,7 @@
}

BugsnagPerformanceSpan *
Tracer::startSpan(NSString *name, SpanOptions options, BSGFirstClass defaultFirstClass) noexcept {
Tracer::startSpan(NSString *name, SpanOptions options, BSGTriState defaultFirstClass) noexcept {
BSGLogDebug(@"Tracer::startSpan(%@, opts, %d)", name, defaultFirstClass);
__block auto blockThis = this;
auto parentSpan = options.parentContext;
Expand All @@ -103,8 +104,8 @@
BSGLogTrace(@"Tracer::startSpan: No parent traceId; generating one");
traceId = IdGenerator::generateTraceId();
}
BSGFirstClass firstClass = options.firstClass;
if (firstClass == BSGFirstClassUnset) {
BSGTriState firstClass = options.firstClass;
if (firstClass == BSGTriStateUnset) {
BSGLogTrace(@"Tracer::startSpan: firstClass not specified; using default of %d", defaultFirstClass);
firstClass = defaultFirstClass;
}
Expand All @@ -115,14 +116,15 @@
auto onSpanClosed = ^(BugsnagPerformanceSpan * _Nonnull endedSpan) {
blockThis->onSpanClosed(endedSpan);
};

BugsnagPerformanceSpan *span = [[BugsnagPerformanceSpan alloc] initWithName:name
traceId:traceId
spanId:spanId
parentId:parentSpan.spanId
startTime:options.startTime
firstClass:firstClass
attributeCountLimit:attributeCountLimit_
instrumentRendering: options.instrumentRendering
metricsOptions:options.metricsOptions
onSpanEndSet:onSpanEndSet
onSpanClosed:onSpanClosed];
if (shouldInstrumentRendering(span)) {
Expand Down Expand Up @@ -238,13 +240,13 @@
BugsnagPerformanceSpan *
Tracer::startAppStartSpan(NSString *name,
SpanOptions options) noexcept {
return startSpan(name, options, BSGFirstClassUnset);
return startSpan(name, options, BSGTriStateUnset);
}

BugsnagPerformanceSpan *
Tracer::startCustomSpan(NSString *name,
SpanOptions options) noexcept {
return startSpan(name, options, BSGFirstClassYes);
return startSpan(name, options, BSGTriStateYes);
}

BugsnagPerformanceSpan *
Expand All @@ -256,12 +258,12 @@
NSString *type = getBugsnagPerformanceViewTypeName(viewType);
onViewLoadSpanStarted_(className);
NSString *name = [NSString stringWithFormat:@"[ViewLoad/%@]/%@", type, className];
if (options.firstClass == BSGFirstClassUnset) {
if (options.firstClass == BSGTriStateUnset) {
if (spanStackingHandler_->hasSpanWithAttribute(@"bugsnag.span.category", @"view_load")) {
options.firstClass = BSGFirstClassNo;
options.firstClass = BSGTriStateNo;
}
}
auto span = startSpan(name, options, BSGFirstClassNo);
auto span = startSpan(name, options, BSGTriStateNo);
if (willDiscardPrewarmSpans_) {
markPrewarmSpan(span);
}
Expand All @@ -271,7 +273,7 @@
BugsnagPerformanceSpan *
Tracer::startNetworkSpan(NSString *httpMethod, SpanOptions options) noexcept {
auto name = [NSString stringWithFormat:@"[HTTP/%@]", httpMethod];
auto span = startSpan(name, options, BSGFirstClassUnset);
auto span = startSpan(name, options, BSGTriStateUnset);
span.kind = SPAN_KIND_CLIENT;
return span;
}
Expand All @@ -283,7 +285,7 @@
NSString *name = [NSString stringWithFormat:@"[ViewLoadPhase/%@]/%@", phase, className];
SpanOptions options;
options.parentContext = parentContext;
auto span = startSpan(name, options, BSGFirstClassUnset);
auto span = startSpan(name, options, BSGTriStateUnset);
if (willDiscardPrewarmSpans_) {
markPrewarmSpan(span);
}
Expand Down Expand Up @@ -314,7 +316,7 @@
options.startTime = startTime;
options.parentContext = parentContext;
options.makeCurrentContext = false;
auto span = startSpan(@"FrozenFrame", options, BSGFirstClassNo);
auto span = startSpan(@"FrozenFrame", options, BSGTriStateNo);
[span endWithAbsoluteTime:endTime];
}

Expand All @@ -334,14 +336,14 @@

bool
Tracer::shouldInstrumentRendering(BugsnagPerformanceSpan *span) noexcept {
switch (span.instrumentRendering) {
case BSGInstrumentRenderingYes:
return autoInstrumentRendering_;
case BSGInstrumentRenderingNo:
switch (span.metricsOptions.rendering) {
case BSGTriStateYes:
return enabledMetrics_.rendering;
case BSGTriStateNo:
return false;
case BSGInstrumentRenderingUnset:
return autoInstrumentRendering_ &&
case BSGTriStateUnset:
return enabledMetrics_.rendering &&
!span.wasStartOrEndTimeProvided &&
span.firstClass == BSGFirstClassYes;
span.firstClass == BSGTriStateYes;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@
#define MAX_ATTRIBUTE_COUNT_LIMIT 1000
#define DEFAULT_ATTRIBUTE_COUNT_LIMIT 128

@implementation BugsnagPerformanceEnabledMetrics

- (instancetype) initWithRendering:(BOOL)rendering cpu:(BOOL)cpu {
if ((self = [super init])) {
_rendering = rendering;
_cpu = cpu;
}
return self;
}

- (instancetype) init {
return [self initWithRendering:NO cpu:NO];
}

- (instancetype) clone {
return [[BugsnagPerformanceEnabledMetrics alloc] initWithRendering:self.rendering cpu:self.cpu];
}

@end

@implementation BugsnagPerformanceConfiguration

- (instancetype)initWithApiKey:(NSString *)apiKey {
Expand All @@ -34,7 +54,7 @@ - (instancetype)initWithApiKey:(NSString *)apiKey {
_autoInstrumentAppStarts = YES;
_autoInstrumentViewControllers = YES;
_autoInstrumentNetworkRequests = YES;
_autoInstrumentRendering = NO;
_enabledMetrics = [BugsnagPerformanceEnabledMetrics new];
_onSpanEndCallbacks = [NSMutableArray array];
_attributeArrayLengthLimit = DEFAULT_ATTRIBUTE_ARRAY_LENGTH_LIMIT;
_attributeStringValueLimit = DEFAULT_ATTRIBUTE_STRING_VALUE_LIMIT;
Expand All @@ -48,6 +68,14 @@ - (instancetype)initWithApiKey:(NSString *)apiKey {
return self;
}

- (void)setAutoInstrumentRendering:(BOOL)autoInstrumentRendering {
self.enabledMetrics.rendering = autoInstrumentRendering;
}

- (BOOL)autoInstrumentRendering {
return self.enabledMetrics.rendering;
}

static inline NSUInteger minMaxDefault(NSUInteger value, NSUInteger min, NSUInteger max, NSUInteger def) {
if(value < min) {
return def;
Expand Down Expand Up @@ -146,7 +174,7 @@ + (instancetype)loadConfigWithInfoDictionary:(NSDictionary * _Nullable)infoDicti
configuration.autoInstrumentNetworkRequests = [autoInstrumentNetworkRequests boolValue];
}
if (autoInstrumentRendering != nil) {
configuration.autoInstrumentRendering = [autoInstrumentRendering boolValue];
configuration.enabledMetrics.rendering = [autoInstrumentRendering boolValue];
}
if (samplingProbability != nil) {
configuration.samplingProbability = samplingProbability;
Expand Down
Loading

0 comments on commit 7cdc1f6

Please sign in to comment.