Skip to content

Expose more performance apis#232

Merged
wzieba merged 7 commits intotrunkfrom
expose_more_performance_apis
Aug 14, 2024
Merged

Expose more performance apis#232
wzieba merged 7 commits intotrunkfrom
expose_more_performance_apis

Conversation

@wzieba
Copy link
Member

@wzieba wzieba commented Jul 29, 2024

Description

This PR adds or exposes several new features for performance measurments:

  • PerformanceMetricsTag for measuring performance of compose
  • start/finish a span during perfomence transaction
  • allow for a custom transaction name

Related internal request: pdnsEh-1Lf-p2

image

@wzieba wzieba marked this pull request as ready for review August 9, 2024 09:19
@wzieba wzieba requested a review from ParaskP7 August 9, 2024 09:19
Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @wzieba !

I have reviewed (and also tested this PR), everything looks and work as expected, good job! 🌟

FYI: Although I (think) understand why you are not adding testing instruction, me having a Sentry test account, just like you, makes it "easy" to test should PRs. Consider in the future to add some testing instructions, even if that's "optional". This will save me time and effort to try and figure out what to test myself, or whether I tested at least what the PR author wanted me to test.

PS: Let me know when BPAndroid#2826 is ready.


I have left one suggestion (💡) for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.


object PerformanceMetricsTag {
@JvmStatic
fun Modifier.tagForPerformanceMetrics(tag: String) = this then sentryTag(tag)
Copy link
Contributor

Choose a reason for hiding this comment

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

Praise (❤️): TIL about the then infix function on a modifier!

@mattylase
Copy link

Just reiterating my support here - did a run through of the APIs on the consumer side and everything is looking good 👍

@wzieba wzieba enabled auto-merge August 14, 2024 07:18
@wzieba wzieba merged commit 743b2a4 into trunk Aug 14, 2024
@wzieba wzieba deleted the expose_more_performance_apis branch August 14, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants