openzipkin / zipkin-aws

Reporters and collectors for use in Amazon's cloud
Apache License 2.0
69 stars 34 forks source link

AWS SDK Instrumentation #85

Closed devinsba closed 5 years ago

devinsba commented 6 years ago

Still WIP as I fix the integration tests to handle the model where 2 spans are created instead of 1

codefromthecrypt commented 6 years ago

I spiked another way to test things, abusing the internals of amazon's SDK. most tests are passing except the error handling ones.. I think we need to look carefully at those..

codefromthecrypt commented 6 years ago

@devinsba so to prevent me from unhooking/rehooking :) what do you think about the direction here? Is it aligned? If this is worth cutting with a disclaimer about the error handling you can feel free to merge (or I can rebase etc)

lemme know

devinsba commented 6 years ago

I've been made aware that AmazonDaxClientBuilder is not a standard builder and does not support the interceptor as it exists now. I'll look into this at some point

codefromthecrypt commented 6 years ago

@devinsba where should we go then.. Should we merge knowing errors/redirects aren't handled, with some notes, or do we wait, or do we write it differently? I can help with any drift fixes from last till now

codefromthecrypt commented 5 years ago

FYI I rebased this into a single commit off latest master

devinsba commented 5 years ago

OK, assuming the new test passes. We should be good to go on this.

devinsba commented 5 years ago

cc @ryangardner I know you were waiting for this