Skip to content

RUMM-2331: Simple message bus for cross-feature communication#1087

Merged
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2331/message-bus-for-cross-feature-communication
Oct 17, 2022
Merged

RUMM-2331: Simple message bus for cross-feature communication#1087
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2331/message-bus-for-cross-feature-communication

Conversation

@0xnm

@0xnm 0xnm commented Oct 14, 2022

Copy link
Copy Markdown
Member

What does this PR do?

This change introduces the way to send messages to the particular feature which is useful in case if this feature is an optional dependency.

This is achieved by using FeatureScope#sendEvent method which just dispatches the event to the registered receiver for the given feature.

I didn't find the need for the broadcast message and also since events are always targeting the particular feature the host of the sendEvent method is not SdkCore, but FeatureScope. This removes the need for the target argument of the sendEvent method, because we always target the feature by doing SdkCore#getFeature which returns FeatureScope.

Message bus in the current implementation doesn't add any thread model: event will be received on the same thread as it was sent.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@0xnm
0xnm requested a review from a team as a code owner October 14, 2022 15:03
@xgouchet xgouchet added the size-small This PR is small sized label Oct 14, 2022
@codecov-commenter

codecov-commenter commented Oct 14, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1087 (72fe800) into feature/sdkv2 (ef329c2) will increase coverage by 0.01%.
The diff coverage is 80.00%.

@@                Coverage Diff                @@
##           feature/sdkv2    #1087      +/-   ##
=================================================
+ Coverage          82.53%   82.54%   +0.01%     
=================================================
  Files                347      347              
  Lines              11183    11196      +13     
  Branches            1834     1838       +4     
=================================================
+ Hits                9229     9241      +12     
- Misses              1391     1392       +1     
  Partials             563      563              
Impacted Files Coverage Δ
.../kotlin/com/datadog/android/v2/core/DatadogCore.kt 87.77% <66.67%> (-0.08%) ⬇️
...in/com/datadog/android/core/internal/SdkFeature.kt 88.07% <100.00%> (+0.69%) ⬆️
...android/log/internal/logger/TelemetryLogHandler.kt 75.00% <0.00%> (-25.00%) ⬇️
...rsistence/file/batch/PlainBatchFileReaderWriter.kt 86.27% <0.00%> (-4.90%) ⬇️
...g/internal/domain/event/DdSpanToSpanEventMapper.kt 95.92% <0.00%> (-4.08%) ⬇️
...src/main/kotlin/com/datadog/android/DatadogSite.kt 86.67% <0.00%> (ø)
...android/v2/core/internal/DatadogContextProvider.kt 82.81% <0.00%> (ø)
...android/v2/core/internal/net/DataOkHttpUploader.kt 94.52% <0.00%> (ø)
...android/rum/internal/ndk/DatadogNdkCrashHandler.kt 87.30% <0.00%> (+0.53%) ⬆️
.../android/rum/internal/monitor/DatadogRumMonitor.kt 92.49% <0.00%> (+0.58%) ⬆️
... and 2 more

Comment thread dd-sdk-android/src/main/kotlin/com/datadog/android/v2/core/DatadogCore.kt Outdated

@plousada plousada left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@0xnm
0xnm force-pushed the nogorodnikov/rumm-2331/message-bus-for-cross-feature-communication branch from 72fe800 to c3b4712 Compare October 17, 2022 12:09
@0xnm
0xnm merged commit 54cf7f5 into feature/sdkv2 Oct 17, 2022
@0xnm
0xnm deleted the nogorodnikov/rumm-2331/message-bus-for-cross-feature-communication branch October 17, 2022 12:41
@xgouchet xgouchet added this to the 1.16.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size-small This PR is small sized

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants