From 2124551ae0a2da085c69d3f0838e9be646fa4a70 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Fri, 27 Oct 2023 10:18:25 +0200 Subject: [PATCH] feat: Add thread id and name to span data (#3359) Add current thread name and thread id to every span when the SDK initializes the span. Fixes GH-3355 --- CHANGELOG.md | 6 ++ Sources/Sentry/SentryCoreDataTracker.m | 2 +- Sources/Sentry/SentryDependencyContainer.m | 14 ++++ Sources/Sentry/SentryNSDataTracker.m | 4 +- Sources/Sentry/SentrySpan.m | 17 +++++ .../HybridPublic/SentryDependencyContainer.h | 2 + .../Sentry/include/SentryInternalDefines.h | 4 +- .../Sentry/include/SentryThreadInspector.h | 2 + .../Transaction/SentrySpanTests.swift | 67 ++++++++++++++++--- .../Transaction/SentryTracerTests.swift | 3 +- .../Transaction/SentryTransactionTests.swift | 8 +-- 11 files changed, 111 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6c3d0c25f0..e9fac15162d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Add thread id and name to span data (#3359) + ## 8.14.2 ### Features diff --git a/Sources/Sentry/SentryCoreDataTracker.m b/Sources/Sentry/SentryCoreDataTracker.m index fd77270ff4c..99039d9f66e 100644 --- a/Sources/Sentry/SentryCoreDataTracker.m +++ b/Sources/Sentry/SentryCoreDataTracker.m @@ -114,7 +114,7 @@ - (void)addExtraInfoToSpan:(SentrySpan *)span withContext:(NSManagedObjectContex { BOOL isMainThread = [NSThread isMainThread]; - [span setDataValue:@(isMainThread) forKey:BLOCKED_MAIN_THREAD]; + [span setDataValue:@(isMainThread) forKey:SPAN_DATA_BLOCKED_MAIN_THREAD]; NSMutableArray *systems = [NSMutableArray array]; NSMutableArray *names = [NSMutableArray array]; [context.persistentStoreCoordinator.persistentStores enumerateObjectsUsingBlock:^( diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index a6ce3120a3e..768771f3672 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -11,6 +11,7 @@ #import "SentryRandom.h" #import "SentrySysctl.h" #import "SentrySystemWrapper.h" +#import "SentryThreadInspector.h" #import "SentryUIDeviceWrapper.h" #import #import @@ -132,6 +133,19 @@ - (SentrySysctl *)sysctlWrapper return _sysctlWrapper; } +- (SentryThreadInspector *)threadInspector +{ + if (_threadInspector == nil) { + @synchronized(sentryDependencyContainerLock) { + if (_threadInspector == nil) { + SentryOptions *options = [[[SentrySDK currentHub] getClient] options]; + _threadInspector = [[SentryThreadInspector alloc] initWithOptions:options]; + } + } + } + return _threadInspector; +} + - (SentryExtraContextProvider *)extraContextProvider { if (_extraContextProvider == nil) { diff --git a/Sources/Sentry/SentryNSDataTracker.m b/Sources/Sentry/SentryNSDataTracker.m index 4a90c685eee..6722b11421c 100644 --- a/Sources/Sentry/SentryNSDataTracker.m +++ b/Sources/Sentry/SentryNSDataTracker.m @@ -190,7 +190,7 @@ - (void)mainThreadExtraInfo:(id)span { BOOL isMainThread = [NSThread isMainThread]; - [span setDataValue:@(isMainThread) forKey:BLOCKED_MAIN_THREAD]; + [span setDataValue:@(isMainThread) forKey:SPAN_DATA_BLOCKED_MAIN_THREAD]; if (!isMainThread) { return; @@ -210,7 +210,7 @@ - (void)mainThreadExtraInfo:(id)span // and only the 'main' frame remains in the stack // therefore, there is nothing to do about it // and we should not report it as an issue. - [span setDataValue:@(NO) forKey:BLOCKED_MAIN_THREAD]; + [span setDataValue:@(NO) forKey:SPAN_DATA_BLOCKED_MAIN_THREAD]; } else { [((SentrySpan *)span) setFrames:frames]; } diff --git a/Sources/Sentry/SentrySpan.m b/Sources/Sentry/SentrySpan.m index 7d71f1ad772..3c8e629a2e1 100644 --- a/Sources/Sentry/SentrySpan.m +++ b/Sources/Sentry/SentrySpan.m @@ -1,10 +1,12 @@ #import "SentrySpan.h" #import "NSDate+SentryExtras.h" #import "NSDictionary+SentrySanitize.h" +#import "SentryCrashThread.h" #import "SentryCurrentDateProvider.h" #import "SentryDependencyContainer.h" #import "SentryFrame.h" #import "SentryId.h" +#import "SentryInternalDefines.h" #import "SentryLog.h" #import "SentryMeasurementValue.h" #import "SentryNoOpSpan.h" @@ -12,6 +14,7 @@ #import "SentrySerializable.h" #import "SentrySpanContext.h" #import "SentrySpanId.h" +#import "SentryThreadInspector.h" #import "SentryTime.h" #import "SentryTraceHeader.h" #import "SentryTracer.h" @@ -33,6 +36,20 @@ - (instancetype)initWithContext:(SentrySpanContext *)context if (self = [super init]) { self.startTimestamp = [SentryDependencyContainer.sharedInstance.dateProvider date]; _data = [[NSMutableDictionary alloc] init]; + + SentryCrashThread currentThread = sentrycrashthread_self(); + _data[SPAN_DATA_THREAD_ID] = @(currentThread); + + if ([NSThread isMainThread]) { + _data[SPAN_DATA_THREAD_NAME] = @"main"; + } else { + NSString *threadName = [SentryDependencyContainer.sharedInstance.threadInspector + getThreadName:currentThread]; + if (threadName.length > 0) { + _data[SPAN_DATA_THREAD_NAME] = threadName; + } + } + _tags = [[NSMutableDictionary alloc] init]; _isFinished = NO; diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index 0dce9c6cd99..c8867893ece 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -17,6 +17,7 @@ @class SentrySysctl; @class SentrySystemWrapper; @class SentryThreadWrapper; +@class SentryThreadInspector; @protocol SentryRandom; #if SENTRY_HAS_METRIC_KIT @@ -68,6 +69,7 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryBinaryImageCache *binaryImageCache; @property (nonatomic, strong) SentryExtraContextProvider *extraContextProvider; @property (nonatomic, strong) SentrySysctl *sysctlWrapper; +@property (nonatomic, strong) SentryThreadInspector *threadInspector; #if SENTRY_UIKIT_AVAILABLE @property (nonatomic, strong) SentryFramesTracker *framesTracker; diff --git a/Sources/Sentry/include/SentryInternalDefines.h b/Sources/Sentry/include/SentryInternalDefines.h index a5c8ad9205f..c4718d0f0e0 100644 --- a/Sources/Sentry/include/SentryInternalDefines.h +++ b/Sources/Sentry/include/SentryInternalDefines.h @@ -59,4 +59,6 @@ static NSString *const SentryPlatformName = @"cocoa"; (__cond_result); \ }) -#define BLOCKED_MAIN_THREAD @"blocked_main_thread" +#define SPAN_DATA_BLOCKED_MAIN_THREAD @"blocked_main_thread" +#define SPAN_DATA_THREAD_ID @"thread.id" +#define SPAN_DATA_THREAD_NAME @"thread.name" diff --git a/Sources/Sentry/include/SentryThreadInspector.h b/Sources/Sentry/include/SentryThreadInspector.h index 216aac1ba46..fd028fb7c36 100644 --- a/Sources/Sentry/include/SentryThreadInspector.h +++ b/Sources/Sentry/include/SentryThreadInspector.h @@ -31,6 +31,8 @@ SENTRY_NO_INIT */ - (NSArray *)getCurrentThreadsWithStackTrace; +- (NSString *)getThreadName:(SentryCrashThread)thread; + @end NS_ASSUME_NONNULL_END diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift index 92285ee70d6..2b1c6c8bd24 100644 --- a/Tests/SentryTests/Transaction/SentrySpanTests.swift +++ b/Tests/SentryTests/Transaction/SentrySpanTests.swift @@ -53,6 +53,49 @@ class SentrySpanTests: XCTestCase { XCTAssertFalse(span.isFinished) } + func testInit_SetsMainThreadInfoAsSpanData() { + let span = fixture.getSut() + XCTAssertEqual("main", span.data["thread.name"] as! String) + + let threadId = sentrycrashthread_self() + XCTAssertEqual(NSNumber(value: threadId), span.data["thread.id"] as! NSNumber) + } + + func testInit_SetsThreadInfoAsSpanData_FromBackgroundThread() { + let expect = expectation(description: "Thread must be called.") + + Thread.detachNewThread { + let threadName = "test-thread-name" + Thread.current.name = threadName + + let span = self.fixture.getSut() + XCTAssertEqual(threadName, span.data["thread.name"] as! String) + let threadId = sentrycrashthread_self() + XCTAssertEqual(NSNumber(value: threadId), span.data["thread.id"] as! NSNumber) + + expect.fulfill() + } + + wait(for: [expect], timeout: 0.1) + } + + func testInit_SetsThreadInfoAsSpanData_FromBackgroundThreadWithNoName() { + let expect = expectation(description: "Thread must be called.") + + Thread.detachNewThread { + Thread.current.name = "" + + let span = self.fixture.getSut() + XCTAssertNil(span.data["thread.name"]) + let threadId = sentrycrashthread_self() + XCTAssertEqual(NSNumber(value: threadId), span.data["thread.id"] as! NSNumber) + + expect.fulfill() + } + + wait(for: [expect], timeout: 0.1) + } + func testFinish() { let client = TestClient(options: fixture.options)! let span = fixture.getSut(client: client) @@ -191,11 +234,11 @@ class SentrySpanTests: XCTestCase { span.setData(value: fixture.extraValue, key: fixture.extraKey) - XCTAssertEqual(span.data.count, 1) + XCTAssertEqual(span.data.count, 3) XCTAssertEqual(span.data[fixture.extraKey] as! String, fixture.extraValue) span.removeData(key: fixture.extraKey) - XCTAssertEqual(span.data.count, 0) + XCTAssertEqual(span.data.count, 2, "Only expected thread.name and thread.id in data.") XCTAssertNil(span.data[fixture.extraKey]) } @@ -237,7 +280,9 @@ class SentrySpanTests: XCTestCase { XCTAssertEqual(serialization["sampled"] as? NSNumber, true) XCTAssertNotNil(serialization["data"]) XCTAssertNotNil(serialization["tags"]) - XCTAssertEqual((serialization["data"] as! Dictionary)[fixture.extraKey], fixture.extraValue) + + let data = serialization["data"] as? [String: Any] + XCTAssertEqual(data?[fixture.extraKey] as! String, fixture.extraValue) XCTAssertEqual((serialization["tags"] as! Dictionary)[fixture.extraKey], fixture.extraValue) XCTAssertEqual("manual", serialization["origin"] as? String) } @@ -246,7 +291,7 @@ class SentrySpanTests: XCTestCase { let span = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: "test")) let serialization = span.serialize() - XCTAssertNil(serialization["data"]) + XCTAssertEqual(2, (serialization["data"] as? [String: Any])?.count, "Only expected thread.name and thread.id in data.") } func testSerialization_withFrames() { @@ -269,7 +314,8 @@ class SentrySpanTests: XCTestCase { span.finish() let serialization = span.serialize() - XCTAssertEqual((serialization["data"] as! Dictionary)["date"], "1970-01-01T00:00:10.000Z") + let data = serialization["data"] as? [String: Any] + XCTAssertEqual(data?["date"] as? String, "1970-01-01T00:00:10.000Z") } func testSanitizeDataSpan() { @@ -279,14 +325,15 @@ class SentrySpanTests: XCTestCase { span.finish() let serialization = span.serialize() - XCTAssertEqual((serialization["data"] as! Dictionary)["date"], "1970-01-01T00:00:10.000Z") + let data = serialization["data"] as? [String: Any] + XCTAssertEqual(data?["date"] as? String, "1970-01-01T00:00:10.000Z") } func testSerialization_WithNoDataAndTag() { let span = fixture.getSut() let serialization = span.serialize() - XCTAssertNil(serialization["data"]) + XCTAssertEqual(2, (serialization["data"] as? [String: Any])?.count, "Only expected thread.name and thread.id in data.") XCTAssertNil(serialization["tag"]) } @@ -327,7 +374,8 @@ class SentrySpanTests: XCTestCase { let sut = SentrySpan(tracer: fixture.tracer, context: SpanContext(operation: "test")) sut.setExtra(value: 0, key: "key") - XCTAssertEqual(["key": 0], sut.data as! [String: Int]) + let data = sut.data as [String: Any] + XCTAssertEqual(0, data["key"] as? Int) } func testSpanWithoutTracer_StartChild_ReturnsNoOpSpan() { @@ -374,7 +422,8 @@ class SentrySpanTests: XCTestCase { queue.activate() group.wait() - XCTAssertEqual(span.data.count, outerLoop * innerLoop) + let threadDataItemCount = 2 + XCTAssertEqual(span.data.count, outerLoop * innerLoop + threadDataItemCount) } func testSpanStatusNames() { diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift index 45ce7d2fde0..aed308c3297 100644 --- a/Tests/SentryTests/Transaction/SentryTracerTests.swift +++ b/Tests/SentryTests/Transaction/SentryTracerTests.swift @@ -1097,7 +1097,8 @@ class SentryTracerTests: XCTestCase { let sut = fixture.getSut() sut.setExtra(value: 0, key: "key") - XCTAssertEqual(["key": 0], sut.data as! [String: Int]) + let data = sut.data as [String: Any] + XCTAssertEqual(0, data["key"] as? Int) } private func advanceTime(bySeconds: TimeInterval) { diff --git a/Tests/SentryTests/Transaction/SentryTransactionTests.swift b/Tests/SentryTests/Transaction/SentryTransactionTests.swift index 1796a6ca0de..29964809bea 100644 --- a/Tests/SentryTests/Transaction/SentryTransactionTests.swift +++ b/Tests/SentryTests/Transaction/SentryTransactionTests.swift @@ -139,10 +139,10 @@ class SentryTransactionTests: XCTestCase { // when let serializedTransaction = sut.serialize() - let serializedTransactionExtra = try! XCTUnwrap(serializedTransaction["extra"] as? [String: String]) + let serializedTransactionExtra = try! XCTUnwrap(serializedTransaction["extra"] as? [String: Any]) // then - XCTAssertEqual(serializedTransactionExtra, [fixture.testKey: fixture.testValue]) + XCTAssertEqual(serializedTransactionExtra[fixture.testKey] as! String, fixture.testValue) } func testSerialize_shouldPreserveExtraFromScope() { @@ -156,10 +156,10 @@ class SentryTransactionTests: XCTestCase { // when let serializedTransaction = sut.serialize() - let serializedTransactionExtra = try! XCTUnwrap(serializedTransaction["extra"] as? [String: String]) + let serializedTransactionExtra = try! XCTUnwrap(serializedTransaction["extra"] as? [String: Any]) // then - XCTAssertEqual(serializedTransactionExtra, [fixture.testKey: fixture.testValue]) + XCTAssertEqual(serializedTransactionExtra[fixture.testKey] as! String, fixture.testValue) } func testSerializeOrigin() throws {