Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] [OAP] Combining HTTP and gRPC on a Single Port through Armeria #12372

Closed
3 tasks done
mrproliu opened this issue Jun 26, 2024 · 10 comments
Closed
3 tasks done
Assignees
Labels
backend OAP backend related. feature New feature rejected The issue or PR can't be accepted by upstream.
Milestone

Comments

@mrproliu
Copy link
Contributor

Search before asking

  • I had searched in the issues and found no similar feature requirement.

Description

HTTP and gRPC services cannot share the same port due to protocol differences in the OAP. However, integrating the Armeria framework makes it possible to share a single port among different protocols.

We have already employed Armeria on the HTTP services, but our gRPC services have not yet been integrated with Armeria. By consolidating these services using Armeria, we can bind multiple protocols to the same port, streamlining our server configuration and enhancing our system's flexibility and efficiency.

Use case

After using Armeria, using multiple ports with different protocols (gRPC, HTTP) in OAP can be reduced.

Related issues

No response

Are you willing to submit a pull request to implement this on your own?

  • Yes I am willing to submit a pull request on my own!

Code of Conduct

@mrproliu mrproliu added the feature New feature label Jun 26, 2024
@mrproliu mrproliu added this to the 10.1.0 milestone Jun 26, 2024
@mrproliu mrproliu self-assigned this Jun 26, 2024
@kezhenxu94
Copy link
Member

What's the scope of this issue, does it aim at combining the HTTP and gRPC of the telemetry collecting endpoints (11800 and 12800), or does it also include other HTTP/gRPC protocols such as otel collector or Zipkin endpoints?

@wu-sheng
Copy link
Member

I want to unify the stack and reduce the confusion of users about which port to use, but to keep compatibility, all existing port should be kept.

So 11800 and 12800 should share the HTTP and gRPC. 11800 will become the primary port, and 12800 as secondary port.
Meanwhile, as Zipkin port should keep to handle HTTP traffic only.

WDYT?

@kezhenxu94
Copy link
Member

kezhenxu94 commented Jun 26, 2024

I want to unify the stack and reduce the confusion of users about which port to use, but to keep compatibility, all existing port should be kept.

So 11800 and 12800 should share the HTTP and gRPC. 11800 will become the primary port, and 12800 as secondary port. Meanwhile, as Zipkin port should keep to handle HTTP traffic only.

WDYT?

Sounds reasonable, I've been thinking of reducing the ports we used, one reason stops me to do so is that, different ports has different functionality and for the sake of security, users need to configure different security policies for different ports, for example, 11800 may be internally accessible only inside data center but 12800 may need to be accessible in end user's browser, it may be still possible to configure different security policies after combining the ports but it is foreseeably more complex.

@wu-sheng
Copy link
Member

If consider keeping security controllable, we just make receiver port unified and query separately.

This should be better?

query port and receiver port and core port for internal communications.

@wu-sheng wu-sheng modified the milestones: 10.1.0, 10.2.0 Sep 11, 2024
@wu-sheng
Copy link
Member

Move to 10.2, as we are busy this month.

@mrproliu
Copy link
Contributor Author

I have created a project to street test the performance of the native gRPC framework vs Armeria gPRC framework.

It seems that in unary scenarios, native gRPC is slightly stronger than Armeria gRPC. However. But in streaming scenarios (the main usage of SkyWalking OAP), it appears that Armeria gRPC outperforms native gRPC by more than 20%.
test_result

@lujiajing1126
Copy link
Contributor

lujiajing1126 commented Oct 29, 2024

IIRC, in the scenario of Istio, the port of service are named after their protocol, for example, grpc-80, http-8080. And then the envoy will choose a proper protocol based on the prefix of the port (or based on an extra appProtocol field: https://kubernetes.io/docs/concepts/services-networking/service/#application-protocol). If a port is serving two kinds of application protocol, I suppose the proxy cannot work as expected.

So I am not sure if combining ports is a good choice considering such cases.

@wu-sheng
Copy link
Member

Thanks for pointing out this.

@mrproliu we could consider replace stack only?

@mrproliu
Copy link
Contributor Author

Yes, I think we should only replace the original tech stack, and keep the current configuration.

@mrproliu
Copy link
Contributor Author

After gaining a deeper understanding of the Armeria framework, I found that by default it does not execute business logic in a separate thread pool but directly in the Event Pool. So I tried adding a Time.Sleep operation to Streaming again and found that its execution time increased significantly.
image

Then I attempted to use the BlockingThreadExecutor feature to customize the thread pool and discovered that its performance dropped sharply compared to the native gRPC framework. Therefore, this solution is temporarily not considered.

@wu-sheng wu-sheng added rejected The issue or PR can't be accepted by upstream. backend OAP backend related. labels Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. feature New feature rejected The issue or PR can't be accepted by upstream.
Projects
None yet
Development

No branches or pull requests

4 participants