sfreiberg / gotwilio

Twilio library for Go (golang).
BSD 2-Clause "Simplified" License
342 stars 136 forks source link

OpenTracing / Context Propagation #65

Open andrewpage opened 4 years ago

andrewpage commented 4 years ago

Wanted to start a discussion about the possible integration of OpenTracing and context.Context propagation in the gotwilio project.

OpenTracing is an API specification/collection of libraries which allow for distributed tracing of an individual request as it propagates across a system. For instance, one could view a trace of a particular request and identify how long certain components of the request took (interaction with database, message queue, internal processing, external service calls, etc.), as well as view logs and tags associated with that particular sub-request (HTTP status codes, errors, etc.)

These recorded traces are viewed through a tool such as Jaeger. Example below: image

For those of us who are developing Twilio-based applications, the HTTP requests to Twilio (& resulting attributes, such as SIDs and errors) are an important part of our request lifecycle, and it would be great to have first-class support integrated into the gotwilio library.

The great part about this is that OpenTracing instrumentation code piggybacks off of the Golang context.Context package, and the instrumentation code is a no-op if the application has not configured a remote tracing server to report to. This means that nobody is required to configure OpenTracing, but if they have configured it, the gotwilio package would seamlessly integrate.

We would use the opentracing-go package.

Proposed API

To maintain backwards compatibility, I propose keeping the current methods, but added Context-suffixed methods to support context propagation. This is similar to the approach taken by the database/sql package (e.g. Query vs QueryContext.

type Twilio interface {
    func CallWithUrlCallbacks(string, string, *CallbackParameters) (*VoiceResponse, *Exception, error)
    func CallWithUrlCallbacksContext(context.Context, string, string, *CallbackParameters) (*VoiceResponse, *Exception, error)
}

What do you think @sfreiberg?

sfreiberg commented 4 years ago

@andrewpage, this looks like a great proposal and would love to see it implemented. It's a huge win to get it implemented via context so we can get that for free. The proposed API looks great.