NewEndpoint inserts blank ipv4 address even when provided an ipv6 address

[odeke-em]

Thank you again for all your work on this repository and for answering my questions.

I just encountered an issue whereby I am intercepting Zipkin spans from arbitrary sources, parsing them using this library and then processing them and then on the exit users can choose to export them back to Zipkin. For this I need to have a verification test that the intercepted spans' serialization matches almost exactly.

However, when I do

endpoint, err := openzipkin.NewEndpoint("foo", "[7::80:807f]:0")

I get back

&model.Endpoint{ServiceName:"frontend", IPv4:net.IP{0x0, 0x0, 0x0, 0x0}, IPv6:net.IP{0x0, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x80, 0x80, 0x7f}, Port:0x0}

when then reflects in my roundtrip tests as

[{"timestamp":1472470996199000,"duration":207000,"traceId":"4d1e00c0db9010db86154a4ba6e91385","id":"4d1e00c0db9010db","parentId":"86154a4ba6e91385","name":"get","kind":"CLIENT","localEndpoint":{"serviceName":"frontend","ipv4":"0.0.0.0","ipv6":"7::80:807f"},"remoteEndpoint":{"serviceName":"backend","ipv4":"192.168.99.101","port":9000},"annotations":[{"timestamp":1472470996238000,"value":"foo"},{"timestamp":1472470996403000,"value":"bar"}],"tags":{"clnt/finagle.version":"6.45.0","http.path":"/api"}},{"timestamp":1472470996199000,"duration":207000,"traceId":"4d1e00c0db9010db86154a4ba6e91385","id":"4d1e00c0db9010db","parentId":"86154a4ba6e91386","name":"put","kind":"SERVER","localEndpoint":{"serviceName":"frontend","ipv4":"0.0.0.0","ipv6":"7::80:807f"},"remoteEndpoint":{"serviceName":"frontend","ipv4":"192.168.99.101","port":9000},"annotations":[{"timestamp":1472470996238000,"value":"foo"},{"timestamp":1472470996403000,"value":"bar"}],"tags":{"clnt/finagle.version":"6.45.0","http.path":"/api"}}]

I noticed that there is an intentional fluffing of the IPv4 field if blank

zipkin-go/endpoint.go

Lines 65 to 71 in 70244c9

	// default to 0 filled 4 byte array for IPv4 if IPv6 only host was found
	if e.IPv4 == nil {
	e.IPv4 = make([]byte, 4)
	}
	return e, nil
	}

but I don't see a reason documented why this should be.

[codefromthecrypt]

agreed if there's no ipv4 it should be absent not set to 0.0.0.0 there's only one weird case which is thrift encoding where 0.0.0.0 packed into an i32 is literally 0. that happens to be a not optional field (only in thrift) so this would be correct to have zero there. All other encodings absent is better

…

On Tue, Oct 30, 2018 at 8:15 AM Emmanuel T Odeke ***@***.***> wrote: Thank you again for all your work on this repository and for answering my questions. I just encountered an issue whereby I am intercepting Zipkin spans from arbitrary sources, parsing them using this library and then processing them and then on the exit users can choose to export them back to Zipkin. For this I need to have a verification test that the intercepted spans' serialization matches almost exactly. However, when I do endpoint, err := openzipkin.NewEndpoint("foo", "[7::80:807f]:0") I get back &model.Endpoint{ServiceName:"frontend", IPv4:net.IP{0x0, 0x0, 0x0, 0x0}, IPv6:net.IP{0x0, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x80, 0x80, 0x7f}, Port:0x0} when then reflects in my roundtrip tests as [{"timestamp":1472470996199000,"duration":207000,"traceId":"4d1e00c0db9010db86154a4ba6e91385","id":"4d1e00c0db9010db","parentId":"86154a4ba6e91385","name":"get","kind":"CLIENT","localEndpoint":{"serviceName":"frontend","ipv4":"0.0.0.0","ipv6":"7::80:807f"},"remoteEndpoint":{"serviceName":"backend","ipv4":"192.168.99.101","port":9000},"annotations":[{"timestamp":1472470996238000,"value":"foo"},{"timestamp":1472470996403000,"value":"bar"}],"tags":{"clnt/finagle.version":"6.45.0","http.path":"/api"}},{"timestamp":1472470996199000,"duration":207000,"traceId":"4d1e00c0db9010db86154a4ba6e91385","id":"4d1e00c0db9010db","parentId":"86154a4ba6e91386","name":"put","kind":"SERVER","localEndpoint":{"serviceName":"frontend","ipv4":"0.0.0.0","ipv6":"7::80:807f"},"remoteEndpoint":{"serviceName":"frontend","ipv4":"192.168.99.101","port":9000},"annotations":[{"timestamp":1472470996238000,"value":"foo"},{"timestamp":1472470996403000,"value":"bar"}],"tags":{"clnt/finagle.version":"6.45.0","http.path":"/api"}}] I noticed that there is an intentional fluffing of the IPv4 field if blank https://github.com/openzipkin/zipkin-go/blob/70244c9703277c13566aac289c530a1143b14cf5/endpoint.go#L65-L71 but I don't see a reason documented why this should be. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#85>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD614D7N1GollKsz4oKT1leHubHVqaiks5up4wZgaJpZM4YA2HI> .

[odeke-em]

Thank you for the feedback and advisory @adriancole and @basvanbeek for the fix!