Skip to content

Commit

Permalink
Handle ports > 32k in Zipkin JSON (#488)
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro authored Oct 23, 2017
1 parent c32f960 commit a7f203b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
16 changes: 12 additions & 4 deletions cmd/collector/app/zipkin/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,23 @@ func TestViaClient(t *testing.T) {

tracer.StartSpan("root").Finish()

waitForSpans(t, handler.zipkinSpansHandler.(*mockZipkinHandler), 1)
}

func waitForSpans(t *testing.T, handler *mockZipkinHandler, expecting int) {
deadline := time.Now().Add(2 * time.Second)
for {
if time.Now().After(deadline) {
t.Error("never received a span")
return
}
if want, have := 1, len(handler.zipkinSpansHandler.(*mockZipkinHandler).getSpans()); want != have {
if have := len(handler.getSpans()); expecting != have {
time.Sleep(time.Millisecond)
continue
}
break
}

assert.Equal(t, 1, len(handler.zipkinSpansHandler.(*mockZipkinHandler).getSpans()))
assert.Len(t, handler.getSpans(), expecting)
}

func TestThriftFormat(t *testing.T) {
Expand All @@ -120,7 +123,7 @@ func TestJsonFormat(t *testing.T) {
server, handler := initializeTestServer(nil)
defer server.Close()

endpJSON := createEndpoint("foo", "127.0.0.1", "2001:db8::c001", 66)
endpJSON := createEndpoint("foo", "127.0.0.1", "2001:db8::c001", 65535)
annoJSON := createAnno("cs", 1515, endpJSON)
binAnnoJSON := createBinAnno("http.status_code", "200", endpJSON)
spanJSON := createSpan("bar", "1234567891234565", "1234567891234567", "1234567891234568", 156, 15145, false,
Expand All @@ -129,6 +132,11 @@ func TestJsonFormat(t *testing.T) {
assert.NoError(t, err)
assert.EqualValues(t, http.StatusAccepted, statusCode)
assert.EqualValues(t, "", resBodyStr)
waitForSpans(t, handler.zipkinSpansHandler.(*mockZipkinHandler), 1)
recdSpan := handler.zipkinSpansHandler.(*mockZipkinHandler).getSpans()[0]
require.Len(t, recdSpan.Annotations, 1)
require.NotNil(t, recdSpan.Annotations[0].Host)
assert.EqualValues(t, -1, recdSpan.Annotations[0].Host.Port, "Port 65535 must be represented as -1 in zipkin.thrift")

endpErrJSON := createEndpoint("", "127.0.0.A", "", 80)

Expand Down
9 changes: 7 additions & 2 deletions cmd/collector/app/zipkin/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type endpoint struct {
ServiceName string `json:"serviceName"`
IPv4 string `json:"ipv4"`
IPv6 string `json:"ipv6"`
Port int16 `json:"port"`
Port int32 `json:"port"`
}
type annotation struct {
Endpoint endpoint `json:"endpoint"`
Expand Down Expand Up @@ -147,10 +147,15 @@ func endpointToThrift(e endpoint) (*zipkincore.Endpoint, error) {
if err != nil {
return nil, err
}
port := e.Port
if port >= (1 << 15) {
// Zipkin.thrift defines port as i16, so values between (2^15 and 2^16-1) must be encoded as negative
port = port - (1 << 16)
}

return &zipkincore.Endpoint{
ServiceName: e.ServiceName,
Port: e.Port,
Port: int16(port),
Ipv4: ipv4,
Ipv6: []byte(e.IPv6),
}, nil
Expand Down

0 comments on commit a7f203b

Please sign in to comment.