-
-
Notifications
You must be signed in to change notification settings - Fork 329
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
feat: monitor for new metric values via sampling and notifications #2493
Merged
Merged
Changes from 9 commits
Commits
Show all changes
75 commits
Select commit
Hold shift + click to select a range
bd8f158
feat: monitor for new metric values via sampling and notifications
armcknight 1337fd2
changelog
armcknight 8a87e63
fix changelog
armcknight da9e287
register for low power mode notifications; improve notification wrapp…
armcknight 5f7412b
extract syscalls and NSProcess work to wrappers with test mocks; lots…
armcknight 288e631
start writing the test; only add metrics to payload if any were gathered
armcknight bb77d06
get test passing with various fixes
armcknight 92f1c09
typedef system type for better understandability
armcknight 7d0666c
Merge branch '8.0.0' into armcknight/feat/profiling-metrics
armcknight 5c94ce7
add and use a wrapper method around NSProcessInfo to get core count
armcknight de82f6a
fix merge issue
armcknight 01ca6f5
dont need lock around cpu usage gethering since its always on the sam…
armcknight ea2fde9
add NSTimer wrapper
armcknight 6e1aed9
rename paramater target->observer
armcknight 762cd42
add info on where to find doc info
armcknight 246d01b
fix timer wrapper issue and assert number of times it "fired"
armcknight aa47004
fix ci build error
armcknight a032fb0
again
armcknight e323e77
fix macos build
armcknight 5acf5de
fix tvos build
armcknight 9bad80a
fix linker errors
armcknight 039e934
again
armcknight 3c2653e
try to fix possible timing issue in CI
armcknight 58bf05f
add debug log
armcknight 3c0da5e
more logging
armcknight 2492abe
format log
armcknight c11991a
only print metrics
armcknight 4475048
only add slow frame renders if info exists
armcknight 7b6ccf3
logging
armcknight e3f4b10
fixup! logging
armcknight 621d340
logging
armcknight 34a4b94
more logging
armcknight 6cf38cc
override processor count
armcknight cf54864
remove some verbose debug logs for testing
armcknight 8c1a6fc
fix build
armcknight 3f5ec52
Merge remote-tracking branch 'origin/8.0.0' into armcknight/feat/prof…
armcknight 65044c1
add dispatch source wrapper to mock memory pressure notifications
armcknight b40d64a
extract method to start metric profiler; init missing timer wrapper
armcknight feb2055
store string values and put measurements under profile
armcknight 8e58476
rename invalidate->cancel
armcknight 6212b2a
add comment for metric sampling rate
armcknight b9990af
fixup! rename invalidate->cancel
armcknight ca88f04
remove unused ivar
armcknight fe63d02
move test header extensions to test target
armcknight e3ccdb1
set values back to doubles instead of strings; fix test after moving …
armcknight 13134a9
synchronize reads and writes to metric profiler data structures
armcknight d71398a
convert kabob case to snake case
armcknight bd6165c
pass low power mode enabled as float value instead of bool
armcknight 827dd5a
make measurements sibling to profile
armcknight acc122d
split slow/frozen frames; use singular unit names
armcknight 2f6c51d
fixup! make measurements sibling to profile
armcknight 66b4bf0
Merge remote-tracking branch 'origin/8.0.0' into armcknight/feat/prof…
armcknight 075c08f
dont declare abstract fire method
armcknight 071e57d
fix macos build
armcknight 80e5a39
wrap bare nanosecond conversions in function
armcknight 74e2375
use the correct routine to process frame rates, which is different fr…
armcknight 360b2a9
remove all metrics but cpu usage and memory footprint
armcknight 2b727d3
fix test by removing async dispatch
armcknight 6b690bc
add test case for new error function; reenable another test case for …
armcknight 16b2ba3
test errors gathering cpu and memory usages
armcknight 94e8c3b
add missing asserts for memory footprint; generalize logic to perform…
armcknight c940b1a
add slow/frozen frames to test case
armcknight 1cfed1c
fix typo; simplify lazy property init
armcknight 5443b84
fix older swift builds
armcknight acb3cfd
test amount of GPU frame render timestamps recorded for profiling met…
armcknight 6b4a169
fix macos build
armcknight 52a1b38
fixup! fix macos build
armcknight 48f2b5a
add system and process info wrapper tests
armcknight 08c0979
add nstimer wrapper tests
armcknight cf11d7f
avoid possible negative interval between transaction end and profile …
armcknight 9c51294
Merge remote-tracking branch 'origin/8.0.0' into armcknight/feat/prof…
armcknight 483bbe6
Merge remote-tracking branch 'origin/main' into armcknight/feat/profi…
armcknight eea6153
fix changelog
armcknight adef44c
fixup! dont declare abstract fire method
armcknight bccfd3c
fix broken nstimer test
armcknight File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
2 changes: 1 addition & 1 deletion
2
Tests/SentryTests/Integrations/Performance/FramesTracking/TestDisplayLinkWrapper.swift
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
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m
: Why can't you useuseFramesTracker
to test this with mocked values?What you could do instead of this is to just call
processFrameRenders
directly in your tests with your desired values. You could achieve this by using a test category makingprocessFrameRenders
public in tests or useDynamic(profiler).processFrameRenders()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I thought about it but wasn't quite sure how to mock the values so that the original logic won't wind up crashing with negative durations, which is what the early returns in the logic are for. This workaround essentially allows us to ensure that the frame tracker did in fact record frame render info and pass it to the profiler. As I mentioned in #2493 (comment), I will write more tests to ensure the slicing logic in here and added in that PR are tested for correctness. Are you OK kicking that can down the road a bit more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously, otherwise, I wouldn't have approved your PR.