Skip to content

RUM-10409: fix effective sample rate calculation for SessionEndedMetricDispatcher#2744

Merged
satween merged 3 commits into
developfrom
tvaleev/feature/RUM-10409
Jun 20, 2025
Merged

RUM-10409: fix effective sample rate calculation for SessionEndedMetricDispatcher#2744
satween merged 3 commits into
developfrom
tvaleev/feature/RUM-10409

Conversation

@satween

@satween satween commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes wrong effective sampling rate value computation for SessionEndedMetricDispatcher.
Before this fix, sessionSampling rate was not take into a count for effective sampling rate of such events

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)

@satween
satween requested review from a team as code owners June 19, 2025 12:59
mariusc83
mariusc83 previously approved these changes Jun 19, 2025
verify(mockInternalLogger).logMetric(
any(),
any(),
eq(15.0f),

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.

why is this 15.0f here ? shouldn't be the one provided in constructor ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the value, provided in constructor – is the session sampling rate, that will or will not start entire metric
15.0 - is the metric sampling rate, that determine wil or will not the whole metric will be sent.
So effective sampling rate = sessionSamplingRate * 15%

@codecov-commenter

codecov-commenter commented Jun 19, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.72%. Comparing base (ac02939) to head (cd4ec56).
⚠️ Report is 1964 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2744      +/-   ##
===========================================
- Coverage    69.77%   69.72%   -0.04%     
===========================================
  Files          824      824              
  Lines        30790    30795       +5     
  Branches      5177     5177              
===========================================
- Hits         21481    21471      -10     
- Misses        7844     7859      +15     
  Partials      1465     1465              
Files with missing lines Coverage Δ
...rum/src/main/kotlin/com/datadog/android/rum/Rum.kt 88.14% <100.00%> (+0.42%) ⬆️
...um/internal/metric/SessionEndedMetricDispatcher.kt 67.65% <100.00%> (+3.13%) ⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/Rum.kt Outdated
@satween
satween force-pushed the tvaleev/feature/RUM-10409 branch from e8948c8 to 26a89dc Compare June 19, 2025 14:57
@satween
satween requested review from 0xnm and mariusc83 June 19, 2025 14:57
0xnm
0xnm previously approved these changes Jun 19, 2025
Comment thread features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/Rum.kt Outdated
@satween
satween force-pushed the tvaleev/feature/RUM-10409 branch from 26a89dc to 04c405c Compare June 19, 2025 15:37
@satween
satween requested a review from 0xnm June 19, 2025 15:38
@satween
satween merged commit dfb5968 into develop Jun 20, 2025
25 checks passed
@satween
satween deleted the tvaleev/feature/RUM-10409 branch June 20, 2025 08:56
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.

4 participants