Skip to content

[RUM-8098] Support for configuration schema updates for time based strategy of TNS and INV metrics#2505

Merged
satween merged 6 commits into
developfrom
tvaleev/RUM-8098/inv-tns-configurations-support
Feb 14, 2025
Merged

[RUM-8098] Support for configuration schema updates for time based strategy of TNS and INV metrics#2505
satween merged 6 commits into
developfrom
tvaleev/RUM-8098/inv-tns-configurations-support

Conversation

@satween

@satween satween commented Feb 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds support for INV and TNS time-based thresholds if they used for metric collection
Small side effect on LocalAttribute - making string field private

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 February 10, 2025 17:52
@satween
satween force-pushed the tvaleev/RUM-8098/inv-tns-configurations-support branch from 7b04104 to a6144c6 Compare February 12, 2025 14:17
@satween
satween force-pushed the tvaleev/RUM-8098/inv-tns-configurations-support branch from a6144c6 to 3a55504 Compare February 14, 2025 14:51
@codecov-commenter

codecov-commenter commented Feb 14, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.02%. Comparing base (b0f79d2) to head (3a55504).
Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
...nternal/attributes/ViewScopeInstrumentationType.kt 0.00% 4 Missing ⚠️
...ndroid/telemetry/internal/TelemetryEventHandler.kt 66.67% 0 Missing and 2 partials ⚠️
.../android/rum/internal/domain/scope/RumViewScope.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2505      +/-   ##
===========================================
+ Coverage    69.97%   70.02%   +0.04%     
===========================================
  Files          794      794              
  Lines        29857    29875      +18     
  Branches      4988     4991       +3     
===========================================
+ Hits         20892    20918      +26     
- Misses        7574     7578       +4     
+ Partials      1391     1379      -12
Files with missing lines Coverage Δ
...android/core/internal/attributes/LocalAttribute.kt 66.67% <100.00%> (+3.03%) ⬆️
...actiontonextview/TimeBasedInteractionIdentifier.kt 91.67% <100.00%> (ø)
...tworksettled/TimeBasedInitialResourceIdentifier.kt 91.67% <100.00%> (ø)
.../android/rum/internal/domain/scope/RumViewScope.kt 94.54% <0.00%> (-0.74%) ⬇️
...ndroid/telemetry/internal/TelemetryEventHandler.kt 88.52% <66.67%> (+0.97%) ⬆️
...nternal/attributes/ViewScopeInstrumentationType.kt 0.00% <0.00%> (ø)

... and 33 files with indirect coverage changes

ambushwork
ambushwork reviewed Feb 14, 2025
View reviewed changes
Comment thread ...-android-core/src/main/kotlin/com/datadog/android/core/internal/attributes/LocalAttribute.kt
@InternalApi
enum class Key(
val string: String
private val string: String

@ambushwork ambushwork Feb 14, 2025

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.

Can you elaborate the purpose of make it private and expose it by toString() function? it seems the main change of this PR, but I don't read the explanation in the description

@satween satween Feb 14, 2025 •
edited
Loading

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.

Yep, this change shadowed main purpose a bit. The idea behind this change is to reduce duplicity of the LocalProperty.Key object's propee. As all java/kotlin objects has toString method by default. I just missed this fact in the previous PR and noticed that making sting property public makes no sense.

@satween
satween merged commit 85637ab into develop Feb 14, 2025
@satween
satween deleted the tvaleev/RUM-8098/inv-tns-configurations-support branch February 14, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ambushwork ambushwork ambushwork approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@satween @codecov-commenter @ambushwork