search

sourcegraph / appdash

Application tracing system for Go, based on Google's Dapper.
https://sourcegraph.com
Other
1.72k stars 137 forks source link

Added timeout feature for httptrace.Transport #93

Closed evalphobia closed 8 years ago

evalphobia commented 8 years ago

This PR enables httptrace.Transport to use timeout parameter and cancel request.

slimsag commented 8 years ago

Thank you for sending this @evalphobia! :) I left some minor comments inline, but overall this looks like a good change.

Though, it looks like the CI build is failing (https://travis-ci.org/sourcegraph/appdash/jobs/99083732) and I'm not sure why yet.

--- FAIL: TestCollectorServer-2 (0.00s)
    collector_test.go:33: collector listening on [::]:50774
    collector_test.go:243: Collect(0000000000000001/0000000000000002/0000000000000003, [{k1 [118 49]}]): dial tcp [::]:50774: network is unreachable
=== RUN TestCollectorServer_stress-2
--- FAIL: TestCollectorServer_stress-2 (0.00s)
    collector_test.go:243: Collect(ac0b06e949183cc1/41c8901084939448, []): dial tcp [::]:39322: network is unreachable

Do these tests pass for you locally?

evalphobia commented 8 years ago

@slimsag Thank you for your comments. I fixed some codes followed by your advices.

Though, it looks like the CI build is failing (https://travis-ci.org/sourcegraph/appdash/jobs/99083732) and I'm not sure why yet. Do these tests pass for you locally?

Yes, I passed the tests both of my branch and original master on my local OSX, but not passed on TravisCI.

For the testing, I set TravisCI on my repo, and resulted in pass. I sought the comment of This job ran on our new platform for Legacy Precise builds. Please read our blog post for more information. and blog article link in top of TravisCI build console. This might be effected?

slimsag commented 8 years ago

Code looks good to me :+1:

Following the blog link, it looks like we might fix the issue just by opting into their new GCE instances. Could you push a second commit to add this to the .travis.yml so we can see if it passes on CI?:

# Most explicit routing to GCE Precise
sudo: required
dist: precise
evalphobia commented 8 years ago

I changed travis.yml , and both of original repo and mine TravisCI build is failed. See the document of the Build Environment, STANDARD virtualization seems to be failed now. So I gonna try CONTAINER-BASED one to check tests will be passed or not.

# Container-based Precise due to `sudo: false`
sudo: false
evalphobia commented 8 years ago

Green finally!

slimsag commented 8 years ago

Awesome! Thanks a ton for tracking this down @evalphobia :)