Unsplit the OTLP receiver protocols, use a shared port

[jmacd]

Is your feature request related to a problem? Please describe.
This is a request to revert #1223

Describe the solution you'd like
I would like both protocols to share a port, as this reduces configuration cost for users and system operators.

Describe alternatives you've considered
The alternative is to use a single port. The PR #1223 did not provide a good justification for this change.

Additional context
At LightStep we once had a multi-port satellite, and it was a huge problem for our users. We eventually were able to support a single-port configuration with both gRPC and HTTP, secure and insecure settings.

[austinlparker]

Mostly just echoing Josh's comment here. Multi-port satellites caused us no end of headaches for users.

[bogdandrutu]

@jmacd happy to revert if you show me how to make TLS work with port multiplexing.

[jmacd]

I'm asking around Lightstep for someone who can contribute here. Thanks.

[jpkrohling]

For context, #1227 might have been the reason for the change.

[jmacd]

Just want to repeat that "no end of headaches for users" is not an exaggeration. It was an awful time. I'm looking for someone to help on this.

[MatthewDolan]

I think there recommendation from grpc-go for supporting both http and grpc on the same port is to utilize the ServerHTTP(...) method on the grpc server.

https://godoc.org/google.golang.org/grpc#Server.ServeHTTP

Something similar to the example code in the docs:

if r.ProtoMajor == 2 && strings.HasPrefix(r.Header.Get("Content-Type"), "application/grpc") {
	grpcServer.ServeHTTP(w, r)
} else {
	httpMux.ServeHTTP(w, r)
}

The TLS support needs to be handled at the http layer because the request has to be decrypted before it can be routed to either the grpc or http mux.

@bogdandrutu, is that the information you were looking for? Apologies if I am missing context from another thread.

[bogdandrutu]

@jmacd can you please tell me if all protocols are enabled by default or you still want user to configure which protocols to enable?

[tcolgate]

I would say that the operational overhead of two ports is worth it for the simpler code. I previously attempted to add full mutual tls to OTel collector but failed due to issues with cmux. They may have been resolvable, but the port sharing made debugging quite painful. (it looks like you reverted the previous cmux usage due to #1227)

[tcolgate]

FWIW, the grpc server.ServeHTTP() http.Handler approach has worked better for me, but that ends up mandating a http2/https client, which may not be acceptable. It is also less performant that the cmux approach, acceptable for a lot of use cases, but possible not if you are getting a lot of small requests (probably OK for streaming though).

[jpkrohling]

but the port sharing made debugging quite painful

I think this alone is worth having two ports. At least, we know what each port is supposed to be receiving.

acceptable for a lot of use cases, but possible not if you are getting a lot of small requests

I'd like to echo this point. Having a performance compromise on the receiver end sounds like a bad idea, especially if there's no workaround or option to disable this compromise.

[jmacd]

I don't intend to contest any decisions here. LightStep's experience with configuring SDKs, firewalls, and ingress rules for multi-port collectors is still a painful memory.

[jpkrohling]

@jmacd I really appreciate your experience, and I think it would be easier for us all to understand the problems you had if you gave some concrete examples. We could then compare both problems and see which poison we want to take.

[austinlparker]

The biggest things off the top of my head...

• It's a nightmare for users who are instrumenting things, because now they need to know more than one port number
• You need to do something different depending on where your service is running (am I inside the dc/vpc? outside?), which adds another thing for people to keep in mind.
• Multiple ports make sane defaults harder, because your default and my default might need to be different rather than having a single well-known port.
• Operators need to remap ports and configure load balancing (with multiple transports!) rather than using a single TCP load balancer or if they want single port operation they'll need to do more fancy LB configuration... again, it's just extra setup steps.

I'm compiling some more horror stories, I'll post 'em when I got 'em.

[tcolgate]

@austinlparker I've had experience with the network ops side of these things, I've felt the paiin. What I would say though, for me, being unable to support mutual TLS would be a show stopper, especially for the client -> agent -> collector communication, agent -> collector in particular. (sorry if the terminology is incorrect for OTel).
It's also worth saying that, if you are doing inter-service mTLS, then it becomes more likely that the http are potentially serving different functions, so might need different creds for those purposes, and in that case, having the separate ports, and config for the separate, makes the configuration much easier to understand.
I would also suggest that debugging issues with a shared port would probably lead to harder
debugging problems in the longer term. You're going to have to know exactly what the incoming client is doing (and what any intermediate proxy might do too), vis http/https/(in|)secure-grpc. I've wasted hours debugging cmux projects due to curl typos just on my laptop, let alone have to do that across a network and with a different company.

[jpkrohling]

If we decide to go with a single port for both protocols for v1.0, changing it later to individual ports would probably be a breaking change. Going with individual ports for v1.0 and changing it later to a single port would not be a breaking change, unless I'm missing something.

Here's my suggestion: let's have them separated in the configuration, each with its own host-port/TLS options. If post-GA we decide that the operational headaches are indeed affecting a good number of users (which I think it really might), we can add an option for people to have both protocols in the same port, given the right conditions:

1. at most one of TLS options are set
2. at most one of the host-ports are set
3. (optional) a new combine-ports option is set

[tcolgate]

The alternative is to use a single port. The PR #1223 did not provide a good justification for this change.

#1223 did explicitly state you can't do mTLS with cmux, which has also been my experience (I added the mTLS support to thanos and jaeger, but failed on otel, and have previous experience with cmux projects, I also spent a good few days on the otel attempt).

Now, I didn't look at the internal cmux details, and I do suspect that it should /possible/ to do this (you need to probe forward enough but still be able to use a full featured tls.Config)

It's also worth mentioning that cockroachdb reverted sharing their UI and grpc on the same port. They had problems with hanging client (due to grpc clients having a different behaviour in sending headers), and also ended up with problems with an authentication bypass problem

• https://www.cockroachlabs.com/blog/a-tale-of-two-ports/
• security: Authentication bypass for internal RPCs cockroachdb/cockroach#30821
• server: Three ports to rule them all cockroachdb/cockroach#30828

[austinlparker]

I'll also present the opposite side of my own argument, for context - while we did have these problems, many of them were due to the fact that Lightstep supported (and continues to support) a lot of transports to our satellite (I believe we accept 8 discrete transport formats at this point?), and a lot of the configuration confusion was specifically around people that were implementing polyglot tracing deployments using client libraries that may have only supported one or two out of several different wire formats, so you'd have a situation where the port used for ruby was different than the port used for java and those were both different than iOS ports, etc. etc.

I don't think it's unreasonable to suggest that future collector receivers could lead us to a place where similar situations occur. In a couple of years, I can easily imagine a world where a collector exists that accepts New Relic, Datadog, Jaeger, Zipkin, SFX, Stackdriver, AppInsights, Honeycomb, Lightstep, OTLP, etc... and each of those potentially having its own set of secure/insecure ports. That's not a problem we can solve right now, but it's a problem that we should at least be mindful of, because as much as I'd like to pretend we're going to solve all the worlds problems with OTLP, I think there's enough 'legacy' protocols out there that receiver proliferation will cause really intense config pain for operators down the line.

That said, I think @jpkrohling suggestion of an optional combined port solution for users who wish to have it isn't a bad one - I think if we can at least hold the line on a "default configuration of least surprise" then we'll ease the adoption curve.

[jpkrohling]

default configuration of least surprise

I like that :-)

[tigrannajaryan]

@bogdandrutu assigning to you to either implement the shared port or close it as "won't do". We need the decision for 1.0.