only need to observe profile starts, and only if there wasnt one runn…

…ing when a span starts
getsentry · May 28, 2024 · 4db756b · 4db756b
1 parent 3a4fa35
commit 4db756b
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 24 deletions.
diff --git a/Sources/Sentry/Profiling/SentryContinuousProfiler.mm b/Sources/Sentry/Profiling/SentryContinuousProfiler.mm
@@ -116,12 +116,6 @@ + (void)stop
  disableTimer();
  }
 
- [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
- postNotification:[[NSNotification alloc]
- initWithName:kSentryNotificationContinuousProfileStopped
- object:nil
- userInfo:nil]];
-
  {
  std::lock_guard<std::mutex> l(_threadUnsafe_gContinuousProfilerLock);
  [_threadUnsafe_gContinuousCurrentProfiler

diff --git a/Sources/Sentry/SentrySpan.m b/Sources/Sentry/SentrySpan.m
@@ -103,15 +103,12 @@ - (instancetype)initWithContext:(SentrySpanContext *)context
  if (SentryContinuousProfiler.isCurrentlyProfiling) {
  [_profileSessionIDs
  addObject:SentryContinuousProfiler.currentProfilerID.sentryIdString];
+ } else {
+ [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
+ addObserver:self
+ selector:@selector(linkProfiler)
+ name:kSentryNotificationContinuousProfileStarted];
  }
- [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
- addObserver:self
- selector:@selector(linkProfiler)
- name:kSentryNotificationContinuousProfileStarted];
- [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
- addObserver:self
- selector:@selector(linkProfiler)
- name:kSentryNotificationContinuousProfileStopped];
  }
 #endif // SENTRY_TARGET_PROFILING_SUPPORTED
  }
@@ -135,9 +132,6 @@ - (void)stopObservingContinuousProfiling
  [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
  removeObserver:self
  name:kSentryNotificationContinuousProfileStarted];
- [SentryDependencyContainer.sharedInstance.notificationCenterWrapper
- removeObserver:self
- name:kSentryNotificationContinuousProfileStopped];
  }
 }
 #endif // SENTRY_TARGET_PROFILING_SUPPORTED

diff --git a/Sources/Sentry/include/SentryContinuousProfiler.h b/Sources/Sentry/include/SentryContinuousProfiler.h
@@ -10,8 +10,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 static NSString *const kSentryNotificationContinuousProfileStarted
  = @"io.sentry.notification.continuous-profile-started";
-static NSString *const kSentryNotificationContinuousProfileStopped
- = @"io.sentry.notification.continuous-profile-stopped";
 
 /**
  * An interface to the new continuous profiling implementation.

diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift
@@ -67,12 +67,12 @@ class SentrySpanTests: XCTestCase {
  }
 
 #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)
- func testSpanDoesNotIncludeTraceProfilerIDOrSubscribeToNotifications() throws {
+ func testSpanDoesNotIncludeTraceProfilerID() throws {
  fixture.options.profilesSampleRate = 1
  SentrySDK.setStart(fixture.options)
  let span = fixture.getSut()
  let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter {
- $0.name.rawValue == kSentryNotificationContinuousProfileStarted || $0.name.rawValue == kSentryNotificationContinuousProfileStopped
+ $0.name.rawValue == kSentryNotificationContinuousProfileStarted
  }
  XCTAssertEqual(continuousProfileObservations.count, 0)
  XCTAssert(SentryTraceProfiler.isCurrentlyProfiling())
@@ -81,6 +81,38 @@ class SentrySpanTests: XCTestCase {
  let serialized = span.serialize()
  XCTAssertNil(serialized["profile_ids"])
  }
+
+ func testSpanDoesNotSubscribeToNotificationsIfAlreadyCapturedContinuousProfileID() {
+ fixture.options.enableContinuousProfiling = true
+ SentryContinuousProfiler.start()
+ SentrySDK.setStart(fixture.options)
+ let _ = fixture.getSut()
+ let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter {
+ $0.name.rawValue == kSentryNotificationContinuousProfileStarted
+ }
+ XCTAssertEqual(continuousProfileObservations.count, 1)
+ }
+
+ func testSpanDoesNotSubscribeToNotificationsIfContinuousProfilingDisabled() {
+ fixture.options.profilesSampleRate = 1
+ fixture.options.enableContinuousProfiling = false
+ SentrySDK.setStart(fixture.options)
+ let _ = fixture.getSut()
+ let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter {
+ $0.name.rawValue == kSentryNotificationContinuousProfileStarted
+ }
+ XCTAssertEqual(continuousProfileObservations.count, 0)
+ }
+
+ func testSpanDoesSubscribeToNotificationsIfNotAlreadyCapturedContinuousProfileID() {
+ fixture.options.enableContinuousProfiling = true
+ SentrySDK.setStart(fixture.options)
+ let _ = fixture.getSut()
+ let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter {
+ $0.name.rawValue == kSentryNotificationContinuousProfileStarted
+ }
+ XCTAssertEqual(continuousProfileObservations.count, 1)
+ }
 
  /// Test a span that starts before and ends before a continuous profile, includes profile id
  ///
@@ -95,9 +127,6 @@ class SentrySpanTests: XCTestCase {
  XCTAssertEqual(fixture.notificationCenter.addObserverInvocations.invocations.filter {
  $0.name.rawValue == kSentryNotificationContinuousProfileStarted
  }.count, 1)
- XCTAssertEqual(fixture.notificationCenter.addObserverInvocations.invocations.filter {
- $0.name.rawValue == kSentryNotificationContinuousProfileStopped
- }.count, 1)
  SentryContinuousProfiler.start()
  let profileId = try XCTUnwrap(SentryContinuousProfiler.profiler()?.profilerId.sentryIdString)
  span.finish()