Skip to content

RUM-6394: Changing logApiUsage method signature - making event parameter computation lazy#2433

Merged
satween merged 1 commit into
developfrom
tvaleev/rum-6394/lazy_metrics
Dec 5, 2024
Merged

RUM-6394: Changing logApiUsage method signature - making event parameter computation lazy#2433
satween merged 1 commit into
developfrom
tvaleev/rum-6394/lazy_metrics

Conversation

@satween

@satween satween commented Dec 5, 2024

Copy link
Copy Markdown
Contributor

What does this PR do?

Changing signature of logApiUsage method of InternalLogger a bit - replacing InternalTelemetryEvent.ApiUsage event parameter with lambda function that will called only if sampling happens. This should reduce useless amount of work made by sdk and reduce impact to the customer's application resources consumption

Motivation

Some times running logApiUsage is useless because actual logging wont happened because it not sampled. But in such cases parameter computation still happened so we will waste device's resources for the useless work. To avoid that we use lambda instead

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

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)

@codecov-commenter

codecov-commenter commented Dec 5, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.10%. Comparing base (1f84948) to head (da8d6e7).
Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
...n/kotlin/com/datadog/android/api/InternalLogger.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2433      +/-   ##
===========================================
- Coverage    70.14%   70.10%   -0.04%     
===========================================
  Files          769      769              
  Lines        28508    28510       +2     
  Branches      4777     4777              
===========================================
- Hits         19996    19986      -10     
- Misses        7176     7188      +12     
  Partials      1336     1336              
Files with missing lines Coverage Δ
.../android/core/internal/logger/SdkInternalLogger.kt 87.23% <100.00%> (-1.06%) ⬇️
...d/rum/internal/domain/scope/RumViewManagerScope.kt 91.30% <100.00%> (ø)
.../android/rum/internal/domain/scope/RumViewScope.kt 94.50% <100.00%> (ø)
...n/kotlin/com/datadog/android/api/InternalLogger.kt 93.75% <0.00%> (ø)

... and 25 files with indirect coverage changes

@satween
satween force-pushed the tvaleev/rum-6394/lazy_metrics branch from 6609ef8 to da8d6e7 Compare December 5, 2024 15:55
@satween
satween marked this pull request as ready for review December 5, 2024 15:55
@satween
satween requested review from a team as code owners December 5, 2024 15:55

@ambushwork ambushwork 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.

Pipeline to fix, otherwise LGTM!

@satween
satween merged commit db3e1d6 into develop Dec 5, 2024
@satween
satween deleted the tvaleev/rum-6394/lazy_metrics branch December 5, 2024 16:40
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.

3 participants