Update the Method Call metric usage #2040
Conversation
xgouchet
force-pushed
the
xgouchet/sr_perf
branch
2 times, most recently
from
9e9c2ac to
06f622e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2040 +/- ##
===========================================
- Coverage 83.23% 83.03% -0.20%
===========================================
Files 491 493 +2
Lines 17685 17701 +16
Branches 2685 2685
===========================================
- Hits 14719 14697 -22
- Misses 2227 2271 +44
+ Partials 739 733 -6
|
| @@ -47,11 +47,13 @@ interface FeatureScope { | |||
| * @param callerClass name of the class calling the performance measurement. | |||
| * @param metric name of the metric that we want to measure. | |||
| * @param samplingRate value between 0-100 for sampling the event. | |||
| * @param operationName the name of the operation being measured | |||
| * @return a PerformanceMetric object that can later be used to send telemetry, or null if sampled out | |||
| */ | |||
| fun startPerformanceMeasure( | |||
There was a problem hiding this comment.
side note: I added some comments to the original PR #1940 (review), but they were too late. I think in the future we should refactor this a bit a remove this method from the FeatureScope.
There was a problem hiding this comment.
Good point, I'll take your other comments here 👍
| * @param samplingRate the sampling rate for the metric | ||
| * @param operation the operation to report | ||
| */ | ||
| fun <R : Any?> FeatureScope?.measureMethodCallPerf( |
There was a problem hiding this comment.
| fun <R : Any?> FeatureScope?.measureMethodCallPerf( | |
| @InternalApi | |
| fun <R : Any?> FeatureScope?.measureMethodCallPerf( |
|
|
||
| val result = operation() | ||
|
|
||
| val isSuccessful = (result != null) && ((result !is Collection<*>) || result.isNotEmpty()) |
There was a problem hiding this comment.
why generally speaking empty collection shouldn't be considered as a successful result? but probably for the methods we are going to cover with this call it makes sense.
There was a problem hiding this comment.
I was trying to keep the existing behavior consistent
| @@ -52,6 +52,7 @@ android { | |||
|
|
|||
| dependencies { | |||
| api(project(":dd-sdk-android-core")) | |||
| api(project(":features:dd-sdk-android-trace")) | |||
There was a problem hiding this comment.
blocking: seems like unnecessary change? I don't see any com.datadog.android.trace or com.datadog.trace usages in this PR
There was a problem hiding this comment.
Indeed, I was using some traces in my tests, forgot to remove this
| val nodes = views | ||
| .mapNotNull { | ||
| val nodes = sessionReplayFeature.measureMethodCallPerf( | ||
| javaClass, |
There was a problem hiding this comment.
should this be in the consumer rules to prevent name obfuscation?
xgouchet
force-pushed
the
xgouchet/sr_perf
branch
from
06f622e to
7618a02
Compare
xgouchet
force-pushed
the
xgouchet/sr_perf
branch
from
7618a02 to
f4f8507
Compare
What does this PR do?
Update the MethodCall metric code to allow multiple usage with different operation names + cleanup some SR code