Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BinaryAnnotation type is broken #27

Closed
codefromthecrypt opened this issue Apr 22, 2017 · 8 comments
Closed

BinaryAnnotation type is broken #27

codefromthecrypt opened this issue Apr 22, 2017 · 8 comments

Comments

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Apr 22, 2017

BinaryAnnotation value can be anyOf number boolean string (in other words "no" array object or list).
Besides string, we only support boolean, for the client and server address annotations. That said, some have accidentally put numeric types in their data before.

It appears this can be solved with "anyOf" or "not" in json schema 3, which is supported in OpenApi v3 (we are on v2). https://github.com/OAI/OpenAPI-Specification/blob/OpenAPI.next/versions/3.0.md

Unfortunately, this is in preview stage. Until this change is in (and tools support openapi 3), I think people looking for an api client will need to make one manually with an http client and a codec from one of the tracers like go or ruby similar to how we do in java

cc @devinsba @kellabyte

@codefromthecrypt
Copy link
Member Author

@whitlockjc favor kind sir. We late discovered a problem as one of our data types needs anyOf I think. Ex one field can be a string or a number. this croaks on go-swagger at the moment, and I think that repo is swagger 2. Seems like swagger 3 is needed for anyOf on a field type.

Do you know any tools that support openapi 3 for codegen? (even if not go, we could test that using anyOf works)

@whitlockjc
Copy link

You are right that if you need anyOf, you will need 3.0 but it's only at the Implementer's Draft and I'm not sure which tooling have started the process of supporting 3.0 yet. There is a list of known implementations here but I don't see anything that generates Go code yet.

@codefromthecrypt
Copy link
Member Author

I've been referred to this, though seems it isn't a complete generator, yet https://github.com/googleapis/gnostic/blob/master/plugins/go/gnostic_go_generator/README.md

@codefromthecrypt
Copy link
Member Author

@whitlockjc thanks for the quick turnaround, bud. That's a great link on implementations. Even if not go, maybe we can at least use one of them to test our api against real data. This is something we had missing before (hence the late figuring out about the missing anyOf thing). Appreciate the knowledge!

@whitlockjc
Copy link

Any time.

@codefromthecrypt
Copy link
Member Author

FYI, for those who are trying to make manual clients in golang in the mean time, here's a thing ported from https://github.com/tracer/tracer/blob/master/transport/zipkinhttp/zipkinhttp.go

Note: to make it work I punted by changing binary annotation value to interface. google seems to say there's several ways to do union in go either explicitly or via method or a switch statement etc

package main

import "encoding/json"
import "fmt"

type zipkinTrace []zipkinSpan
type zipkinEndpoint struct {
	ServiceName string `json:"serviceName"`
	IPv4        string `json:"ipv4"`
	IPv6        string `json:"ipv6"`
	Port        int    `json:"port"`
}
type zipkinAnnotation struct {
	Timestamp uint64         `json:"timestamp"`
	Value     string         `json:"value"`
	Endpoint  zipkinEndpoint `json:"endpoint"`
}
type zipkinBinaryAnnotation struct {
	Key      string         `json:"key"`
	Value    interface{}    `json:"value"`
	Endpoint zipkinEndpoint `json:"endpoint"`
}
type zipkinSpan struct {
	TraceID           string                   `json:"traceId"`
	ParentID          string                   `json:"parentId,omitempty"`
	ID                string                   `json:"id"`
	Name              string                   `json:"name"`
	Timestamp         uint64                   `json:"timestamp"`
	Duration          uint64                   `json:"duration"`
	Annotations       []zipkinAnnotation       `json:"annotations"`
	BinaryAnnotations []zipkinBinaryAnnotation `json:"binaryAnnotations"`
}

func main() {
	byt := []byte(`[[{"traceId":"ffdc9bb9a6453df3","id":"ffdc9bb9a6453df3","name":"http:/","timestamp":1478624611519000,"duration":11940,"annotations":[{"timestamp":1478624611519000,"value":"sr","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624611530000,"value":"ss","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"ffdc9bb9a6453df3","id":"b89b8dfb23214f08","name":"call-backend","parentId":"ffdc9bb9a6453df3","timestamp":1478624611521000,"duration":9012,"binaryAnnotations":[{"key":"lc","value":"unknown","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"mvc.controller.class","value":"Frontend","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"mvc.controller.method","value":"callBackend","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"ffdc9bb9a6453df3","id":"a50cd5a4f50046de","name":"http:/api","parentId":"b89b8dfb23214f08","timestamp":1478624611521000,"duration":6516,"annotations":[{"timestamp":1478624611521000,"value":"cs","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624611523000,"value":"sr","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"timestamp":1478624611527000,"value":"cr","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624611528000,"value":"ss","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}}],"binaryAnnotations":[{"key":"http.host","value":"localhost","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.method","value":"GET","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.path","value":"/api","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.url","value":"http://localhost:9000/api","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"sa","value":true,"endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"ffdc9bb9a6453df3","id":"fb591c54a8ee851b","name":"print-date","parentId":"a50cd5a4f50046de","timestamp":1478624611525000,"duration":1942,"binaryAnnotations":[{"key":"lc","value":"unknown","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"key":"mvc.controller.class","value":"Backend","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"key":"mvc.controller.method","value":"printDate","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}}]}],[{"traceId":"415a94fbd0d1d8a3","id":"415a94fbd0d1d8a3","name":"http:/","timestamp":1478624610288000,"duration":200563,"annotations":[{"timestamp":1478624610289000,"value":"sr","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624610489000,"value":"ss","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"415a94fbd0d1d8a3","id":"a2ed941552f4d7a2","name":"call-backend","parentId":"415a94fbd0d1d8a3","timestamp":1478624610303000,"duration":183703,"binaryAnnotations":[{"key":"lc","value":"unknown","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"mvc.controller.class","value":"Frontend","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"mvc.controller.method","value":"callBackend","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"415a94fbd0d1d8a3","id":"8c75947e5c42b194","name":"http:/api","parentId":"a2ed941552f4d7a2","timestamp":1478624610327000,"duration":145369,"annotations":[{"timestamp":1478624610327000,"value":"cs","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624610423000,"value":"sr","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"timestamp":1478624610459000,"value":"cr","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"timestamp":1478624610473000,"value":"ss","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}}],"binaryAnnotations":[{"key":"http.host","value":"localhost","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.method","value":"GET","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.path","value":"/api","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"http.url","value":"http://localhost:9000/api","endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}},{"key":"sa","value":true,"endpoint":{"serviceName":"frontend","ipv4":"127.0.0.1","port":8081}}]},{"traceId":"415a94fbd0d1d8a3","id":"904febe187850694","name":"print-date","parentId":"8c75947e5c42b194","timestamp":1478624610436000,"duration":18667,"binaryAnnotations":[{"key":"lc","value":"unknown","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"key":"mvc.controller.class","value":"Backend","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}},{"key":"mvc.controller.method","value":"printDate","endpoint":{"serviceName":"backend","ipv4":"127.0.0.1","port":9000}}]}]]`)

	var dat []zipkinTrace

	if err := json.Unmarshal(byt, &dat); err != nil {
		panic(err)
	}
	fmt.Println(dat)
}

@jcchavezs
Copy link
Contributor

jcchavezs commented Nov 12, 2020

Is this still an issue given that we moved out from V1?

@codefromthecrypt
Copy link
Member Author

well we still define the json, which I suppose now could be updated to 3 to seal the deal here. no idea if anyone uses codegen on it though. so ok to close imho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants