yarpc/yarpc-go

A message passing platform for Go

yarpc

A message passing platform for Go that lets you:

• Write servers and clients with various encodings, including JSON, Thrift, and Protobuf.
• Expose servers over many transports simultaneously, including HTTP/1.1, gRPC, and TChannel.
• Migrate outbound calls between transports without any code changes using config.

Installation

We recommend locking to SemVer range ^1 using Glide:

glide get 'go.uber.org/yarpc#^1'

Stability

This library is v1 and follows SemVer strictly.

No breaking changes will be made to exported APIs before v2.0.0 with the exception of experimental packages.

Experimental packages reside within packages named x, and are not stable. This means their APIs can break at any time. The intention here is to validate these APIs and iterate on them by working closely with internal customers. Once stable, their contents will be moved out of the containing x package and their APIs will be locked.

Development

Setup

To start developing with yarpc-go, run the following command to setup your environment:

cd $GOPATH/src
git clone https://github.com/yarpc/yarpc-go.git go.uber.org/yarpc
make

Running Tests

To run tests into a pre-configured docker container, run the following command:

make test

To run tests locally, run the following command:

SUPPRESS_DOCKER=1 make test

Happy development!

yarpc-go's Issues

Rename thrift.TRequest to thrift.Request

Deepends on #172

Expose Headers at the top-level

Users shouldn't have to use transport.Headers.

Decide on HTTP wire format

Let's decide on the wire format for HTTP - this came up from #30, where we @abhinav didn't want to see YARPC-Ttlms.

Implement Crossdock echo behaviors

parent: https://github.com/yarpc/yarpc/issues/17

TODO:

• Integrate with thriftrw-go code generation (blocked on thriftrw/thriftrw-go#61)

Combine encoding's ReqMeta into a single top-level ReqMeta

Currently, we have 3 different ReqMetas:

• raw.ReqMeta
• json.ReqMeta
• thrift.ReqMeta

The only difference between the 3 is that thrift.ReqMeta does not have a Procedure field, since we determine the procedure name based on the idl method they are calling. We chose this path because, in theory, each encoding might have different request metadata. This also makes it really clear that you don't need to set Procedure when making Thrift calls.

I'd like to reconsider that position. Maintaining a separate ReqMeta object per encoding adds mental overhead, makes it a bit awkward to add additional encodings, and is not the most obvious experience. We've received feedback that this is a bit surprising.

Instead, I'd like to see a single top-level yarpc.ReqMeta and yarpc.ResMeta structs:

type ReqMeta struct {
    Context context.Context
    Procedure string
    Headers Headers
    TTL time.Duration
}

type ResMeta struct {
    Headers Headers
}

This produces an experience like so:

// raw

resBody, resMeta, err := rawClient.Call(
    yarpc.ReqMeta{
        Procedure: "yolo-raw",
        Context: context.Background(),
        Headers: yarpc.Headers{"bob": "ishome"},
        TTL: time.Second,
    },
    []bytes{"raw bytes yall"},
) 

// thrift
// note we don't provide a Procedure
// that will get written (or overwritten) by the encoding,
// you could even imagine allowing someone to override at the call-site if provided

resBody, resMeta, err := userServiceClient.Search(
    yarpc.ReqMeta{
        Context: context.Background(),
        Headers: yarpc.Headers{"bob": "ishome"},
        TTL: time.Second,
    }, 
    userservice.Filter{FirstName: "bob"},
    userservice.LastUUID("..."),
)

// protobufs (feels same as thrift, except only 1 respBody struct)

resBody, resMeta, err := emailServiceClient.Send(
    yarpc.ReqMeta{
        Context: context.Background(),
        Headers: yarpc.Headers{"bob": "ishome"},
        TTL: time.Second,
    }, 
    emailservice.To{Email: "[email protected]", From: "Your Friends"},
)

• Every encoding uses the same ReqMeta, but has varying request bodies
• Every encoding uses the same ResMeta, but has varying response bodies

Request.Server should be populated automatically

Instead of by hand as in #30

Run crossdock tests from regular test suite

Move transport.Headers to yarpc.Headers

We don't want some of the primary use-cases to cause users to reach down into the transport package, lets move headers up as a top-level shape instead.

Baggage: Retrieve all entries

We should figure out if we need a function that allows users access to all baggage added to the context.

Couple things to keep in mind:

• opentracing doesn't currently offer a way to do this, so this may require work there after we integrate with it.
• We shouldn't expose a plain map[string]string because the keys are case insensitive (canonicalized to lower case), which is a detail the user is not aware of. We would need either a custom type with accessor functions (Get(key)), or a CanonicalizeBaggageKey(key) function.

TestErrorWaiter is a flaky test

https://travis-ci.org/yarpc/yarpc-go/jobs/126185525#L450

Combine Context & Meta

Let's try this out:

func (h Handler) Get(context Context, request Request) response, context, err

Will look more like node, python, & java.

Request.Caller should be populated automatically

Instead of provided by hand as in #30

Crossdock tests should use errors instead of Result struct

Instead of what we're doing in #30

Rename TTL header to $PREFIX-TTL-MS

Integrate with thriftrw-go code generation

Blocked on thriftrw/thriftrw-go#61

Unexplode Request/Response for Raw

For raw, the request and response body is always []byte so we don't need to explode them into {Req,Res}{Meta,Body} like the other encodings.

Context validation to avoid invalid memory address...

I was able to get a panic: runtime error: invalid memory address or nil pointer dereference [recovered] by making a raw call without assigning a Context: to reqMeta

Rename $encoding.{Request,Response} to $encoding.{Req,Res}Meta

TChannel Transport ownership

Right now, the user has to construct the tchannel.Channel manually and thread it down to the inbound and outbound transports. There's some duplication of information there too (the caller name). It would be nice if we could provide an API that would take care of constructing the Channel for the user and making the TChannel inbound and outbounds share it. It would need to pick up the service name from the channel.

Hide {Local,Remote}{BadRequest,Unexpected}Error in an internal package

Right now, users could use these from the transport package if they wanted. Do we want to disallow that?

Worth noting that moving to an internal package isn't as straightforward because the corresponding types for these functions implement package-local functions of the BadRequest and UnexpectedError interfaces.

crossdock: Log to crossdock.T

We should add the ability to log to crossdock.T similar to testing.T. For the normal case, that would just log to stdout, but for the crossdock test, we can wrap the testing.T into a crossdock.T, and log to the test object. This will make the tests silent except in case of failure or when we use -v.

Add go 1.6 to .travis.yml

Design - allow only 1 outbound per-rpc object

Instead of taking a map[string]Outbound, lets have the rpc object take only 1 outbound (which can be used to make custom outbound behavior).

This way we can create ship a ServiceMapOutbound(map[string]Outbound) which has the same behavior.

HTTP content types

Use content-types as specified in https://github.com/yarpc/yarpc/blob/master/http.md

Remove RPC.Handler, if not used atm

s/scheme/encoding/

Let's change the terminology of Scheme to Encoding - @kriskowal used this terminology in the Open Tracing meeting the other day, and everybody knew exactly what he meant. That wouldn't have been the case for Scheme.

Change:

• packages/namespaces
• class names
• variable names

Pending tests

This issue is to track areas of the library that have tests pending.

• Perform necessary refactoring for tests: #62
• Round trip test for TChannel: #63
• HTTP unit tests: #64
• transport.Header tests: #66
• TChannel unit tests: #70
• Add crossdock behaviors to regular test suite (#33): #91

Hash out register story with other langs

Go has a really different register story - work with the other langs to determine if we want this across the board.

crossdock - Rename behaviorSink to crossdock.T

I've struggled with the sink terminology in various Crossdock behaviors - for examlpe, https://github.com/yarpc/yarpc-go/pull/147/files#diff-b84f2b2ac8e475a9c0fb53615befc070R47

@kriskowal mentioned the other day that we should piggyback off of Go's testing.T naming - that's probably a wise idea.

func Run(t crossdock.T) {
    behavior := t.Behavior
    transport := t.Param("transport")
    encoding := t.Param("encoding")

    t.NoError(err, "something went wrong")
    t.AssertEqual("bob", "tod")

    if !t.ChecksTrue(...) {
         ...
    }
}

cc @yarpc/golang

TChannel: ResponseWriter with empty body won't write headers

The TChannel inbound won't write the application headers to arg2 until Write() is called. So if you have an empty body and don't call Write() the headers won't get written either.

https://github.com/yarpc/yarpc-go/blob/master/transport/tchannel/handler.go#L185

RPC outbounds should only take map[string]Outbound

Not a list of outbounds

Register transport-level interceptors per-handler

It's possible that we'll need to be able to apply transport interceptors on a per-handler basis - for example, protecting a subset of handlers with a certain auth interceptor:

rpc.AddInterceptor([myService.ThisProcedure, myService.ThatProcedure], SuperStrongAuthInterceptor{})

Brought up by @shawnburke

@yarpc/golang thoughts? Is this the right path or is there a better way to think about this?

Remove ReqMeta.TTL in favor of Context.Timeout

Having a separate TTL concept is a bit surprising:

 yarpc.ReqMeta(
    Context: context.WithTimeout(time.Second),
    TTL: time.Second,
    ...
)

Instead, lets start with just the context timeout.

cc @yarpc/golang

Move Dockerfile and docker-compose.yml to the xlang folder

I think that would be the least confusing considering this is a lib and not an app - @abhinav, whatchu think?

Enforce timeout set on context

We should blowup if a timeout hasn't been set on a context, like tchannel currently does:

tchannel.SystemError{code:0x6, msg:"timeout required", wrapped:error(nil)}

Instead of leaking the TChannel error, we should detect this in a single place for all transports and emit a specific YARPC error.

Translate panics in Handlers to UnexpectedErrors

Right now, we're letting panics bubble up to the HTTP/TChannel server. We should catch them and translate them to unexpected errors.

We'll want to log the stack trace in case of panics.

Unable to send []byte{} as header for raw tchannel client request

I would expect to, from a pure-tch client, be able to make a raw request with no headers supplied, []byte{}, like so:

arg2, arg3, _, err := raw.Call(
    ctx,
    ch,
    ch.PeerInfo().HostPort,
    ch.ServiceName(),
    "echo",
    []byte{}, // headers here
    []byte("hello"),
)

Instead, I get the following error:

tchannel.SystemError{code:0x6, msg:"BadRequest: failed to decode \"raw\" request headers for procedure \"echo\" of service \"holler\" from caller \"holler\": EOF", wrapped:error(nil)}

Code coverage is tripping PRs for non-existent drops in code coverage

This annoying bugger is denying PRs their big green check marks.

Seen in #139, #142, and probably many others.

Validate outgoing requests

All outgoing requests should be validated by the system.

Use yarpc/crossdock instead of yarpc/xlang

You'll also need to switch XLANG_DIMENSION_* to CROSSDOCK_AXIS_*.

I plan on deleting yarpc/xlang out of Docker Hub tomorrow.

Fix package name does not match import path

Right now we have something like:

import "github.com/yarpc/yarpc-go"

rpc := yarpc.New()

Which feels wrong because yarpc doesnt match the package name, yarpc-go.

I suggest moving all go code into a package yarpc like so:

import "github.com/yarpc/yarpc-go/yarpc"

rpc := yarpc.New()

The import paths aren't the prettiest, but I think it's a good tradeoff.

@yarpc/golang

Remove split request naming ambiguity

Currently the request meta struct is just named Request - this is bound to cause confusion with our users. Instead of:

res, rbody, err := service.Call(req, body)

Let's do something like:

respMeta, respBody, err := service.Call(reqMeta, reqBody)

It's a bit more verbose, but really nails home that the request is "exploded" because of Go's type system.

cc @yarpc/golang

Baggage: Add WithoutBaggage

We should add a function WithoutBaggage that returns a copy of the context with the given baggage removed (if present).

ctx = context.WithoutBaggage(ctx, "foo")

This may be difficult/impossible with opentracing integration, though.

Friendlier interface to raw encoding

The transport level APIs need to be wrapped into a nicer API similar to JSON/Thrift encodings for Raw.

Thrift exceptions should use flags=0x01

Per the Thrift over TChannel Protocol doc -

In case of failure, the Response Code (code:1) must be set to 0x01.

Instead, we're setting 0x00 -

$ sudo tcap -i lo0 -i en0 -p 8080 -p 8082
...

ts=1464736322.907 session=4 127.0.0.1:64892 <-- 127.0.0.1:8082 frame=2 type=0x04 Ok
CALL RESPONSE id=0x0002 (2) flags=0x00
headers
  as: thrift
tracing: spanid=0,0 parentid=0,0 traceid=0,0 flags=0x00
args[0]
    00:                                          empty
args[1]
    00: 0000                                     ..
args[2]
    00: 0c00 0108 0001 0000 03e9 0b00 0200 0000  ................
    10: 0858 6365 7074 696f 6e00 00              .Xception..
arg3 as thrift
{ '1':
   { '1': 1001,
     '2': 'Xception' } }

This is blocking #162's TestException test since the error is being set to nil instead of gauntlet_apache.Xception.

cc @yarpc/golang

Should raw low-level request handlers provide a Writer?

We could have the low-level handler provide a ResponseWriter (similar to http's ResponseWriter) which will let us skip the io.Copy we have to do from the response Reader to the http.ResponseWriter.

Rename RPC.serve to start

Application middleware

Creating this issue to discuss how we want application middleware to look.

Something that came up during the in-person discussion: Thrift middleware could have a generic request body shape with an interface similar to the following:

type ReqBody interface {
    GetParam(name string) (interface{}, ok bool)
    SetParam(name string, value interface{}) error
    Params() []string
}

CC @yarpc/yarpc @shawnburke

Wait until Start is ready to receive requests

Instead of sleeping as we did in #30

Timeouts

• Impose deadlines on contexts when incoming requests are received with a TTL
  • HTTP #161
  • TChannel (inherent)
• Clamp down deadlines on child contexts for outgoing requests if request.TTL is set
  • HTTP
  • TChannel (inherent)
• Rename TTL to Timeout
• Provide per-Channel timeouts
• Use request-local Timeout, defaulting to the Channel timeout, which defaults to the RPC timeout