Add Admin port and group all ports in one file

[yurishkuro]

Which problem is this PR solving?

Health checks, metrics, and other housekeeping data should not be exposed on the ports that also serve public APIs

• Partially supersedes Add pprof and create admin endpoint #1375.
• Resolves Vulnerable data exposed with metrics endpoint #1428
• Resolves Move /metrics from the query port to internal (health-check) #1332

Short description of the changes

• combines health check and metrics handlers on a single admin port. There are breaking changes:
  • agent metrics move from /sampling config port to the new admin port 14271
  • query metrics move from /api port to the former health check port 16687
  • collector metrics move from HTTP API port to the former health check port 14269
  • ingestor metrics move from standalone port 14271 to the health check port 14270
• groups all numeric port numbers in a single file ports/ports.go for easier reference
• consolidates common boilerplate code in all binaries into a Service type
• flag health-check-http-port is deprecated and replaced by admin-http-port
• ingester's DefaultHTTPPort = 14271 is deprecated and repurposed as the agent's admin port

[codecov]

Codecov Report

Merging #1442 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1442   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         165     165           
  Lines        7539    7510   -29     
======================================
- Hits         7539    7510   -29

Impacted Files	Coverage Δ
cmd/ingester/app/flags.go	100% <ø> (ø)	⬆️
cmd/builder/builder_options.go	100% <ø> (ø)	⬆️
cmd/agent/app/builder.go	100% <ø> (ø)	⬆️
cmd/query/app/flags.go	100% <100%> (ø)	⬆️
cmd/agent/app/reporter/tchannel/flags.go	100% <100%> (ø)	⬆️
pkg/healthcheck/handler.go	100% <100%> (ø)	⬆️
plugin/storage/cassandra/options.go	100% <100%> (ø)	⬆️
cmd/collector/app/builder/builder_flags.go	100% <100%> (ø)	⬆️
cmd/agent/app/flags.go	100% <100%> (ø)	⬆️
plugin/storage/memory/factory.go	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc48919...cbccc24. Read the comment docs.

[jpkrohling]

LGTM apart from a couple of questions and one suggestion.

[jpkrohling]

We have other deprecated options as well and I don't think they follow a standard:

--cassandra-archive.enable-dependencies-v2        DEPRECATED: Jaeger will...
--span-storage.type string                        Deprecated; please use SPAN_STORAGE_TYPE...

Deprecated; seems to be most used pattern. One good practice is also to point users to the property that is replacing the deprecated value. In this case, the adminHTTPPort:

flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) The http port for the health check service. See " + healthCheckHTTPPort)

[yurishkuro]

Normalized all flags to (deprecated) see {replacement flag}

[jpkrohling]

Why is this needed? O_O

[yurishkuro]

I added an explanation to CONTRIBUTING.md

[jpkrohling]

This is a breaking change that will probably impact everyone who is using Jaeger in production, as they are probably monitoring Jaeger via Prometheus. We should communicate this change as loud as possible.

[gouthamve]

Hi All, super appreciate the project and all the work that goes into it. I just spent an hour wondering where my metrics went only to find this PR. I could find nothing in the release notes. In the future can we make sure to have an PR label that release leads could look at to find all breaking changes?

Or maybe have a draft changelog that is updated with every PR by the PR author?

[yurishkuro]

@gouthamve apologies, we missed it from release notes. I added a PR #1595 to indicate this was a breaking change.