Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react-native): support for starting native spans on ios #540

Open
wants to merge 5 commits into
base: integration/rn-native-integration
Choose a base branch
from

Conversation

yousif-bugsnag
Copy link
Contributor

@yousif-bugsnag yousif-bugsnag commented Nov 29, 2024

Goal

Add support for starting native spans on iOS - this is the iOS implementation of the startNativeSpan Turbo Module method added in #536

Design

The Turbo Module method startNativeSpan starts a span in Cocoa Performance via the CrossTalk API method startSpanV1 - this method accepts a span name as an NSString and a set of span options as an NSDictionary

Updated the Turbo Module Spec to use the UnsafeObject type for native span options - this is required for backwards compatibility with the Old Architecture (otherwise codegen will generate a struct for the typed function parameter instead of using NSDictionary)

Testing

Tested manually as this isn't used yet

Copy link

github-actions bot commented Nov 29, 2024

Browser bundle size

NPM build

Package
Before 210.63 kB
After 210.63 kB
± No change

CDN build

Unminified Minfied Minified + gzipped
Before 107.59 kB 40.60 kB 11.89 kB
After 107.59 kB 40.60 kB 11.89 kB
± No change No change No change

Code coverage

Coverage values did not change👌.

Total:

Lines Branches Functions Statements
90.21%(+0%) 80.68%(+0%) 87.89%(+0.02%) 88.96%(+0.01%)

Generated against 5979437 on 4 December 2024 at 12:22:19 UTC

@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12937/start-native-spans-cocoa branch from 85ddc0d to 1b366b1 Compare December 2, 2024 09:44
@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12936/start-native-spans-android branch 2 times, most recently from 42f08bd to 1b8f00e Compare December 2, 2024 12:00
Base automatically changed from PLAT-12936/start-native-spans-android to integration/rn-native-integration December 2, 2024 14:56
An error occurred while trying to automatically change base from PLAT-12936/start-native-spans-android to integration/rn-native-integration December 2, 2024 14:56
@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12937/start-native-spans-cocoa branch from 1b366b1 to 7373858 Compare December 2, 2024 14:59
@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12937/start-native-spans-cocoa branch from adef590 to e84642f Compare December 2, 2024 17:31
@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12937/start-native-spans-cocoa branch from ce38a1b to 6cc9a9d Compare December 4, 2024 12:02
@yousif-bugsnag yousif-bugsnag force-pushed the PLAT-12937/start-native-spans-cocoa branch from 6cc9a9d to 5979437 Compare December 4, 2024 12:18
Comment on lines +152 to +156
NSMutableDictionary *span = [NSMutableDictionary new];
span[@"name"] = nativeSpan.name;
span[@"id"] = [NSString stringWithFormat:@"%llx", nativeSpan.spanId];
span[@"traceId"] = [NSString stringWithFormat:@"%llx%llx", nativeSpan.traceId.hi, nativeSpan.traceId.lo];
span[@"startTime"] = [NSNumber numberWithDouble: [nativeSpan.startTime timeIntervalSince1970] * NSEC_PER_SEC];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also need to add the first class property, or does that only matter for the native side?

@@ -381,7 +381,7 @@ function installCocoaPerformance() {
const podfilePath = resolve(fixtureDir, 'ios/Podfile')
let podfile = fs.readFileSync(podfilePath, 'utf8')

const performancePod = `pod 'BugsnagPerformance', :git => 'https://github.com/bugsnag/bugsnag-cocoa-performance.git', :branch => 'integration/react-native-integration'`
const performancePod = `pod 'BugsnagPerformance', :git => 'https://github.com/bugsnag/bugsnag-cocoa-performance.git', :branch => 'ya/crosstalk-api-startspan-improvements'`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure you won't miss it, but just a reminder to update this branch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants