AF-1541 additional timing granularity #116
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on HipChat: InfoSec Forum. |
| @@ -0,0 +1,16 @@ | |||
| class StartupTimingSpecifier { | |||
There was a problem hiding this comment.
Not my favorite name.
toddbeckman-wf
changed the title
Codecov Report
@@ Coverage Diff @@
## master #116 +/- ##
==========================================
- Coverage 93.49% 91.74% -1.76%
==========================================
Files 4 5 +1
Lines 369 424 +55
==========================================
+ Hits 345 389 +44
- Misses 24 35 +11
Continue to review full report at Codecov.
|
| _actions.loadData(); | ||
| return new Future.value(); | ||
| return null; |
There was a problem hiding this comment.
This could introduce an issue if a consumer treats the returned Future directly.
// This will throw.
_module.onLoad().then((_) => ...);There was a problem hiding this comment.
I was just toying around with the examples and forgot to put it back. I'll probably do that before pushing for the reviews on this.
| void addTags(Map<String, dynamic> newTags) => tags.addAll(newTags); | ||
|
|
||
| @override | ||
| Duration get duration => _endTime?.difference(startTime); |
There was a problem hiding this comment.
❓Are consumers of Span aware that this may be null?
There was a problem hiding this comment.
I figure it's not important here because it's just in the examples.
…r-specified-timing
…r-specified-timing
…nal-timing-granularity
|
Closing for #115 |
Note
Merges #115. Clean diff: AF-1540_consumer-specified-timing...AF-1541_additional-timing-granularity
It will result in a breaking API change if they are merged separately. Handle this before merging either this or that, or merge together.
Description
There are other useful startup time properties that would be nice to measure. Allow consumers to call a method with a specifier for which timing operation this is.
Testing
See the
DataLoadAsyncexample.