Add Seek Latency Telemetry #2735
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added the ability to track the latency of `WebMediaPlayer` actions to the existing `MediaMetricsProvider` class, and instrumented `Seek` as the first action. Added basic unit tests for the new code. b/329439521 Change-Id: Icbbfd0da148e48d2137da7662b91e6e2d52de590
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2735 +/- ##
==========================================
- Coverage 59.80% 59.79% -0.02%
==========================================
Files 1831 1831
Lines 88482 88482
==========================================
- Hits 52918 52907 -11
- Misses 35564 35575 +11 ☔ View full report in Codecov by Sentry. |
sideb0ard
approved these changes
Mar 27, 2024
There was a problem hiding this comment.
This looks clean, tests look good.
my only potential worry is any performance impact of calling IsActionCurrentlyTracked for every OnPipelineSeek, depending on how often seek is called.
base::small_map looks efficient though, so let's see!
lgtm if histogram data looks good for Joel
joeltine
reviewed
Apr 1, 2024
Change-Id: I4486d0fa7bcdc31ba12a549040e886044c53eeef
Change-Id: Ife49b0a7d40b809caf0f389a4230038dc8f6042e
Change-Id: If3ef45925049a43ee190e1bd65bdeae638d5d526
joeltine
approved these changes
Apr 1, 2024
Change-Id: I4b1dc77751492846342a1c724f39f1b259d0eaae
cobalt-github-releaser-bot
pushed a commit
that referenced
this pull request
Apr 2, 2024
Added the ability to track the latency of `WebMediaPlayer` actions to the existing `MediaMetricsProvider` class, and instrumented `Seek` as the first action. Added basic unit tests for the new code. b/329439521 Change-Id: Icbbfd0da148e48d2137da7662b91e6e2d52de590 (cherry picked from commit a712047)
at-ninja
added a commit
that referenced
this pull request
Apr 2, 2024
Refer to the original PR: #2735 Added the ability to track the latency of `WebMediaPlayer` actions to the existing `MediaMetricsProvider` class, and instrumented `Seek` as the first action. Added basic unit tests for the new code. b/329439521 Change-Id: Icbbfd0da148e48d2137da7662b91e6e2d52de590 --------- Co-authored-by: Drew Thomas <async@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the ability to track the latency of
WebMediaPlayeractions to the existingMediaMetricsProviderclass, and instrumentedSeekas the first action.Added basic unit tests for the new code.
b/329439521
Change-Id: Icbbfd0da148e48d2137da7662b91e6e2d52de590