feat(react-native): support for starting native spans on android #536
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Browser bundle sizeNPM build
CDN build
Code coverageCoverage values did not change👌. Total:
Generated against a9097cf on 2 December 2024 at 14:05:00 UTC |
gingerbenw
approved these changes
Nov 29, 2024
yousif-bugsnag
force-pushed
the
PLAT-12936/start-native-spans-android
branch
from
39925f3 to
dec156d
Compare
lemnik
reviewed
Nov 29, 2024
...tive/android/src/main/java/com/bugsnag/android/performance/NativeBugsnagPerformanceImpl.java
Outdated
Show resolved
Hide resolved
| long nativeSpanId = Long.parseUnsignedLong(spanId, 16); | ||
| UUID nativeTraceId = new UUID(Long.parseUnsignedLong(traceId.substring(0, 16), 16), Long.parseUnsignedLong(traceId.substring(16), 16)); | ||
|
|
||
| SpanContext nativeSpanContext = new SpanContext() { |
There was a problem hiding this comment.
Any reason this shouldn't maybe be a named class (like ReactNativeSpanContext) to remove the noise from this jsSpanContextToNativeSpanContext?
There was a problem hiding this comment.
good idea 👍🏾
yousif-bugsnag
force-pushed
the
PLAT-12936/start-native-spans-android
branch
2 times, most recently
from
8d55f2c to
63f5c9b
Compare
lemnik
reviewed
Dec 2, 2024
...tive/android/src/main/java/com/bugsnag/android/performance/NativeBugsnagPerformanceImpl.java
Show resolved
Hide resolved
Comment on lines
13
to
20
| nativeSpanId = Long.parseUnsignedLong(spanId, 16); | ||
| nativeTraceId = new UUID( | ||
| Long.parseUnsignedLong(traceId.substring(0, 16), 16), | ||
| Long.parseUnsignedLong(traceId.substring(16), 16) | ||
| ); |
There was a problem hiding this comment.
It feels like 16 for the radix should maybe be made into a constant?
private static final int HEX_RADIX = 16;It might also be worth it to have a TRACE_ID_MIDPOINT = 16 constant
...act-native/android/src/main/java/com/bugsnag/android/performance/ReactNativeSpanContext.java
Outdated
Show resolved
Hide resolved
yousif-bugsnag
force-pushed
the
PLAT-12936/start-native-spans-android
branch
from
63f5c9b to
42f08bd
Compare
yousif-bugsnag
force-pushed
the
PLAT-12936/start-native-spans-android
branch
from
42f08bd to
1b8f00e
Compare
lemnik
approved these changes
Dec 2, 2024
yousif-bugsnag
force-pushed
the
PLAT-12936/start-native-spans-android
branch
from
a2b57b6 to
a9097cf
Compare
yousif-bugsnag
merged commit ab5a293
into
integration/rn-native-integration
43 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
Add support for starting native spans on Android
Design
Added a new Turbo Module method called
startNativeSpanwhich creates a span in the Android Performance SDK and returns a JS representation of the native Android span to JS.The Turbo Module method accepts a limited set of span options (
startTimeandparentContext) as native spans should always be first class and should never become the current context in the native SDK.Testing
Tested manually as this is not yet being used