removed ServiceEndpoint magic number for port

[hakanson]

require port to be passed in to all GetLocalEndpoint calls, and derive from domain.Port in SpanTracer (similar to how serviceName is derived from domain.Host)

[hakanson]

I implemented a recursive Hello World API with a URL like http://localhost:52399/api/hello?count=3

Looking at the output, I would see port with the 443 value, instead of the expected 52399

Example annotations were:

      {
        "endpoint": {
          "serviceName": "localhost",
          "ipv4": "10.211.55.8",
          "port": 443
        },
        "timestamp": 1469823744758543,
        "value": "cs"
      },
      {
        "endpoint": {
          "serviceName": "localhost",
          "ipv4": "10.211.55.8",
          "port": 443
        },
        "timestamp": 1469823744847448,
        "value": "cr"
      },

Example binaryAnnotations were:

      {
        "key": "http.path",
        "value": "/api/hello",
        "endpoint": {
          "serviceName": "localhost",
          "ipv4": "10.211.55.8",
          "port": 443
        }
      },

      {
        "key": "http.status",
        "value": "200",
        "endpoint": {
          "serviceName": "localhost",
          "ipv4": "10.211.55.8",
          "port": 443
        }
      }

[hakanson]

If you want to provide a default value, zipkinCore.thrift says "IPv4 port or 0, if unknown."

https://github.com/openzipkin/zipkin-api/blob/master/thrift/zipkinCore.thrift#L251

[cabbott]

Oooh, thanks for helping on this Todo item! @jcarres-mdsol and @bvillanueva-mdsol @lschreck-mdsol @kenyamat please take a look. This looks like maybe a breaking change that'll need a major version bump?

[hakanson]

@cabbott I guess it would be a breaking change for any external callers of the GetLocalEndpoint method which could be fixed by adding the parameter default value back. If so, would you still default as 443 or change to 0 per my previous comment?

[bvillanueva-mdsol]

@hakanson Sorry for the late followup check on this PR. I think we can have the default zero value. Thanks for having time to create a PR for this. Sorry again for the late follow up.

@jcarres-mdsol Are you ok with Zero as port number if not specified/unknown? The PR covers in supplying the real ports though. It is just for external callers that will be using GetLocalEndpoint will get affected on this.

[jcarres-mdsol]

Sorry for being super late. This looks good. We should merge

[bvillanueva-mdsol]

Sorry it took me a long time to check and test this PR. Will take sometime within this week to have this tested and merged. Thanks!

[bvillanueva-mdsol]

Note: Needed to recheck and retest as changes were incorporated lately on Zipkin. Thanks

[bvillanueva-mdsol]

Tested locally. Looks good. Merging this PR!