-
Notifications
You must be signed in to change notification settings - Fork 168
[chain-pr 3/10] DatadogCore: CoreMessageBus implementation and rename #2929
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
Changes from all commits
c65b2cd
d71830d
49daeb7
39dd976
351341a
461e59c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ import DatadogInternal | |
| @objcMembers | ||
| @_spi(Internal) | ||
| public final class CrossPlatformExtension: NSObject { | ||
| private static var contextSharingTransformer: ContextSharingTransformer? | ||
| private static var contextSubscription: MessageBusSubscription? | ||
|
|
||
| /// Subscribes to shared context updates. | ||
| /// | ||
|
|
@@ -26,21 +26,20 @@ public final class CrossPlatformExtension: NSObject { | |
| /// - Parameter toSharedContext: A closure that receives `SharedContext` updates. Called on context changes. | ||
| @objc | ||
| public static func subscribe(toSharedContext: @escaping (SharedContext?) -> Void) { | ||
| if Self.contextSharingTransformer == nil { | ||
| let core = CoreRegistry.default | ||
| let contextSharingTransformer = ContextSharingTransformer() | ||
| try? core.register(feature: ContextSharingFeature(messageReceiver: contextSharingTransformer)) | ||
| Self.contextSharingTransformer = contextSharingTransformer | ||
| guard contextSubscription == nil else { | ||
| return | ||
|
Comment on lines
+29
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the bridge calls Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the intended contract here that callers must always unsubscribe before subscribing again? If so, it might be worth documenting that, since the previous implementation allowed a later subscribe call to replace the callback. |
||
| } | ||
| contextSubscription = CoreRegistry.default.messageBus.subscribe { (context: DatadogContext, _) in | ||
| toSharedContext(SharedContext(datadogContext: context)) | ||
| } | ||
| contextSharingTransformer?.publish(to: toSharedContext) | ||
| } | ||
|
|
||
| /// Drops the subscription to `SharedContext`, and removes the static refrence. | ||
| /// Drops the subscription to `SharedContext`. | ||
| /// | ||
| /// Note: that it doesn't remove the registered feature. | ||
| @objc | ||
| public static func unsubscribeFromSharedContext() { | ||
| contextSharingTransformer?.cancel() | ||
| contextSharingTransformer = nil | ||
| contextSubscription.map { CoreRegistry.default.messageBus.unsubscribe($0) } | ||
| contextSubscription = nil | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forwards only future context writes to typed subscribers, but registration still replays the current context only through
add(messageReceiver:), which sends.contextto the feature's legacymessageReceiver. The two migrated subscribers are now registered withNOPFeatureMessageReceiver()inCrossPlatformExtensionandregister(urlSessionHandler:), so if shared-context or network instrumentation is registered after user/RUM/trace context already exists, their typed receivers start withnil/stale context until another context mutation occurs; early shared-context callbacks and intercepted requests will miss that context.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initial context replay for typed bus subscribers will be tackled in a follow-up PR.