Skip to content

Commit

Permalink
fix tests and location of stop readings
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight committed Sep 9, 2023
1 parent c8d388c commit 359e03a
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 8 deletions.
1 change: 1 addition & 0 deletions SentryTestUtils/TestDispatchSourceWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class TestDispatchSourceWrapper: SentryDispatchSourceWrapper {
public init(eventHandler: @escaping () -> Void) {
self.overrides.eventHandler = eventHandler
super.init()
eventHandler() // SentryDispatchSourceWrapper constructs a timer so that it fires with no initial delay
}

public override func cancel() {
Expand Down
3 changes: 1 addition & 2 deletions Sources/Sentry/SentryDispatchSourceWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ - (instancetype)initTimerWithInterval:(uint64_t)interval
_queueWrapper = queueWrapper;
_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, queueWrapper.queue);
dispatch_source_set_event_handler(_source, eventHandler);
dispatch_source_set_timer(
_source, dispatch_time(DISPATCH_TIME_NOW, interval), interval, leeway);
dispatch_source_set_timer(_source, dispatch_time(DISPATCH_TIME_NOW, 0), interval, leeway);
dispatch_resume(_source);
}
return self;
Expand Down
13 changes: 10 additions & 3 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ + (void)startWithTracer:(SentryId *)traceId
if (_gCurrentProfiler && [_gCurrentProfiler isRunning]) {
SENTRY_LOG_DEBUG(@"A profiler is already running.");
trackProfilerForTracer(_gCurrentProfiler, traceId);
// record a new metric sample for every concurrent span start
[_gCurrentProfiler->_metricProfiler recordMetrics];
return;
}
Expand All @@ -339,6 +340,15 @@ + (BOOL)isCurrentlyProfiling
return [_gCurrentProfiler isRunning];
}

+ (void)recordMetrics
{
std::lock_guard<std::mutex> l(_gProfilerLock);
if (_gCurrentProfiler == nil) {
return;

Check warning on line 347 in Sources/Sentry/SentryProfiler.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfiler.mm#L347

Added line #L347 was not covered by tests
}
[_gCurrentProfiler->_metricProfiler recordMetrics];
}

+ (nullable SentryEnvelopeItem *)createProfilingEnvelopeItemForTransaction:
(SentryTransaction *)transaction
{
Expand Down Expand Up @@ -415,9 +425,6 @@ + (void)updateProfilePayload:(NSMutableDictionary<NSString *, id> *)payload
and:(uint64_t)endSystemTime
onHub:(SentryHub *)hub;
{
if (self.isRunning) {
[_metricProfiler recordMetrics];
}
return serializedProfileData([self._state copyProfilingData], startSystemTime, endSystemTime,
self.profileId, profilerTruncationReasonName(_truncationReason),
[_metricProfiler serializeBetween:startSystemTime and:endSystemTime],
Expand Down
3 changes: 3 additions & 0 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@ - (SentryTransaction *)toTransaction
transaction.transaction = self.transactionContext.name;
#if SENTRY_TARGET_PROFILING_SUPPORTED
transaction.startSystemTime = self.startSystemTime;
if (self.isProfiling) {
[SentryProfiler recordMetrics];
}
transaction.endSystemTime = SentryDependencyContainer.sharedInstance.dateProvider.systemTime;
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

Expand Down
6 changes: 6 additions & 0 deletions Sources/Sentry/include/SentryProfiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ SENTRY_EXTERN_C_END
*/
+ (BOOL)isCurrentlyProfiling;

/**
* Immediately record a sample of profiling metrics. Helps get full coverage of concurrent spans
* when they're ended.
*/
+ (void)recordMetrics;

/**
* Given a transaction, return an envelope item containing any corresponding profile data to be
* attached to the transaction envelope.
Expand Down
13 changes: 10 additions & 3 deletions Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ class SentryProfilerSwiftTests: XCTestCase {
try fixture.gatherMockedMetrics(span: span)
self.fixture.currentDateProvider.advanceBy(nanoseconds: 1.toNanoSeconds())
span.finish()
try self.assertMetricsPayload(expectedUsageReadings: fixture.mockUsageReadingsPerBatch + 1)
try self.assertMetricsPayload(expectedUsageReadings: fixture.mockUsageReadingsPerBatch + 2) // including one sample at the start and the end
}

func testConcurrentProfilingTransactions() throws {
Expand All @@ -272,6 +272,7 @@ class SentryProfilerSwiftTests: XCTestCase {
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(0))

for i in 0 ..< numberOfTransactions {
print("creating new concurrent transaction for test")
let span = try fixture.newTransaction()

// because energy readings are computed as the difference between sequential cumulative readings, we must increment the mock value by the expected result each iteration
Expand All @@ -289,10 +290,16 @@ class SentryProfilerSwiftTests: XCTestCase {
try fixture.gatherMockedMetrics(span: span)
XCTAssertTrue(SentryProfiler.isCurrentlyProfiling())
span.finish()
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(numberOfTransactions - i - 1))
XCTAssertEqual(SentryProfiler.currentProfiledTracers(), UInt(numberOfTransactions - (i + 1)))

try self.assertValidProfileData()
try self.assertMetricsPayload(expectedUsageReadings: fixture.mockUsageReadingsPerBatch * (i + 1) + numberOfTransactions + i)

// this is a complicated number to come up with, see the explanation for each part...
let expectedUsageReadings = fixture.mockUsageReadingsPerBatch * (i + 1) // since we fire mock metrics readings for each concurrent span,
// we need to accumulate across the number of spans each iteration
+ numberOfTransactions // and then, add the number of spans that were created to account for the start reading for each span
+ 1 // and the end reading for this span
try self.assertMetricsPayload(expectedUsageReadings: expectedUsageReadings, oneLessEnergyReading: i == 0)
}

XCTAssertFalse(SentryProfiler.isCurrentlyProfiling())
Expand Down

0 comments on commit 359e03a

Please sign in to comment.