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
Feature - Task Metrics #1492
Feature - Task Metrics #1492
Conversation
@@ -154,6 +183,8 @@ public struct DownloadResponse<Value> { | |||
/// The timeline of the complete lifecycle of the request. | |||
public let timeline: Timeline | |||
|
|||
var _metrics: AnyObject? |
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.
Why is this AnyObject
and not URLSessionTaskMetrics
?
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.
Just so the internal variable didn't need be behind an availability check?
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.
Yep...you can't store a URLSessionTaskMetrics
as a stored property b/c it's not available on the older OS's. This is the only valid workaround I've found to the problem yet. It's nasty, but it does work.
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.
You can't put the whole stored property behind an availability check?
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.
Nope.
Looks good. |
Awesome, thanks @jshier. I'm going to go ahead and move this through. |
This PR adds support for the awesome new
URLSessionTaskMetric
statistical tracking. This is essentially what we always wanted theTimeline
to be, but we didn't have access to the same internal APIs to compute all these data points. Thanks Apple!This shouldn't be a difficult feature to build, but it most definitely was due to the complexity of compiling against 4 platforms with only three of the latest actually supporting new APIs. Therefore, I had to get pretty creative with the new
Response
protocol and extension in combination withAnyObject
abstraction while moving the metrics through the various objects.Overall, I'm VERY happy with how this turned out. I wish it wasn't so complicated to support so many platforms, but unfortunately, it is. I abstracted the complexity away into the single
Response
protocol and implementation, so that certainly helps isolate all the platform complexity to a single area.