expressMiddleware.js and default port = 0
The constructor for expressMiddleware takes a default value for port of 0
module.exports = function expressMiddleware({tracer, serviceName = 'unknown', port = 0}) {
Using ConsoleRecorder I see log records like this, where the port is known to be 3000 as part of the HOST header but port=0 when constructing the InetAddress.
Record at (spanId=b1d6a95787bb7f9d, parentId=b1d6a95787bb7f9d, traceId=b1d6a95787bb7f9d): BinaryAnnotation(http.url="http://localhost:3000/")
Record at (spanId=b1d6a95787bb7f9d, parentId=b1d6a95787bb7f9d, traceId=b1d6a95787bb7f9d): ServerRecv()
Record at (spanId=b1d6a95787bb7f9d, parentId=b1d6a95787bb7f9d, traceId=b1d6a95787bb7f9d): LocalAddr(host="InetAddress(10.211.55.2)", port=0)
It seems like the port should be derived from the HOST header when left as its default value.
tracer.recordBinary('http.url', url.format({
protocol: req.protocol,
host: req.get('host'),
pathname: req.originalUrl
}));
tracer.recordAnnotation(new Annotation.ServerRecv());
tracer.recordAnnotation(new Annotation.LocalAddr({port}));
It makes sense that it could pick this stuff up. PR welcome! (any input @eirslett ?)
Go ahead, that's a good change, I think! Even better would be if the middleware had access to the server object, with knows which port it is listening on. In some cases, there could be a call like service a -> haproxy -> service b, and the Host header would then have haproxy's port, while the app would listen on some other port. We prefer that the app's port is shown.
I not a haproxy expert, but doesn't it change the HOST header to the target hostname and port values and require additional config if you want the original values using the standard headers??
option forwardfor
http-request set-header X-Forwarded-Port %[dst_port]
http-request set-header X-Forwarded-Proto https if { ssl_fc }
http-request set-header X-Forwarded-Host %[dst]
I don't think localEndpoint should be retrieved from the URL instead it is the tracer providing the information for the local endpoint (as in https://github.com/openzipkin/zipkin-go/blob/master/tracer.go#L36) cc @adriancole