-
Notifications
You must be signed in to change notification settings - Fork 142
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
♻️ [RUM-2445] align rum and logs common context implementation #2564
♻️ [RUM-2445] align rum and logs common context implementation #2564
Conversation
/to-staging |
🚂 Branch Integration: starting soon, merge in < 7m Commit bfbbf3222b will soon be integrated into staging-02. This build is going to start soon! (estimated merge in less than 7m) you can cancel this operation by commenting your pull request with |
🚨 Branch Integration: The build pipeline contains failing jobs for this merge request We couldn't automatically merge the commit bfbbf3222b into staging-02. Since those jobs are not marked as being allowed to fail, the pipeline will most likely fail. You should have a look at the pipeline, wait for the build to finish and investigate the failures.
|
Align both implementations by: * move the logs `buildCommonContext` in its own module (similar to RUM) * craft a `getCommonContext` based on `buildCommonContext` in both rumPublicApi and logsPublicApi, and pass it to `startRum` and `startLogs`. This slightly refactors how common contexts are computed.
bfbbf32
to
37fd6d9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2564 +/- ##
=======================================
Coverage 92.85% 92.85%
=======================================
Files 228 229 +1
Lines 6744 6745 +1
Branches 1480 1480
=======================================
+ Hits 6262 6263 +1
Misses 482 482 ☔ View full report in Codecov by Sentry. |
/to-staging |
🚂 Branch Integration: starting soon, merge in < 7m Commit 37fd6d92c0 will soon be integrated into staging-02. This build is going to start soon! (estimated merge in less than 7m) you can cancel this operation by commenting your pull request with |
🚂 Branch Integration: This commit was successfully integrated Commit 37fd6d92c0 has been merged into staging-02 in merge commit 386eae285f. Check out the triggered pipeline on Gitlab 🦊 |
Motivation
This PR slightly refactors how common contexts are computed.
Changes
Align both implementations by:
buildCommonContext
in its own module (similar to RUM)getCommonContext
based onbuildCommonContext
in both rumPublicApi and logsPublicApi, and pass it tostartRum
andstartLogs
.Testing
I have gone over the contributing documentation.