Skip to content
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 - Request Timelines #1054

Merged
merged 5 commits into from
Feb 7, 2016
Merged

Feature - Request Timelines #1054

merged 5 commits into from
Feb 7, 2016

Conversation

cnoon
Copy link
Member

@cnoon cnoon commented Feb 7, 2016

These changes started with me experimenting with the ideas in #1029, then became something pretty awesome!

This PR adds the ability for Alamofire to track the timings of a Request throughout it's entire lifecycle with virtually zero overhead. The Timeline object stores all the timing info about the Request and is delivered as part of the Response object. The timings are collected at various points throughout the lifecycle of the request.

Alamofire can now report the latency, request duration and total duration of any Request. I also added CustomStringConvertible and CustomDebugStringConvertible conformance to the Timeline to make it really easy to print out the info.

@cnoon cnoon added this to the 3.2.0 milestone Feb 7, 2016
@cnoon cnoon self-assigned this Feb 7, 2016
@cnoon
Copy link
Member Author

cnoon commented Feb 7, 2016

cc @jshier, @kcharwood, @kylef for review

@cnoon
Copy link
Member Author

cnoon commented Feb 7, 2016

I'll add a section to the README about this once everyone is on board that this is a good set of changes to put in.

@cnoon cnoon mentioned this pull request Feb 7, 2016
@@ -77,6 +88,7 @@ extension Response: CustomDebugStringConvertible {
output.append(response != nil ? "[Response]: \(response!)" : "[Response]: nil")
output.append("[Data]: \(data?.length ?? 0) bytes")
output.append("[Result]: \(result.debugDescription)")
output.append("[Timeline]: \(timeline.debugDescription)")
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a new line between this and the [Result] output. Perhaps there's an extra newline in result.debugDescription? Trivial, but didn't quite match the rest of the output.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah bummer. I'll check it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is possible if the value stored in the result contains newlines. There's nothing to fix here.

@alimoeeny
Copy link

Wow Dude @cnoon , this is much more complete than what I had in mind, thank you.
I need to try it and see how I can collect all the metrics (timelines I guess) in one location to log or send back to a log aggregator.

@cnoon
Copy link
Member Author

cnoon commented Feb 7, 2016

Thanks @alimoeeny! It was definitely fun to put together.

@jshier
Copy link
Contributor

jshier commented Feb 7, 2016

👍

@alimoeeny I do logging in my custom .responseObject extension, taking the closure that comes in, adding my logging call, and passing the result the actual serializer. I imagine you could do the same, especially if you have a custom response closure already.

cnoon added a commit that referenced this pull request Feb 7, 2016
@cnoon cnoon merged commit 73a6706 into master Feb 7, 2016
@cnoon cnoon deleted the feature/request_timeline branch February 7, 2016 22:47
@alimoeeny
Copy link

thanks @jshier I think I'd do the same.

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