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

WIP: Change gRPC to streaming #1183

Closed
wants to merge 2 commits into from

Conversation

pavolloffay
Copy link
Member

Opening this to trigger discussion. We should clarify why we need streaming API. I am not fixing tests until it's decided we want to go this way. The binaries should be executable in this PR.

census trace proto https://github.com/census-instrumentation/opencensus-proto/blob/master/src/opencensus/proto/agent/trace/v1/trace_service.proto
census metrics proto https://github.com/census-instrumentation/opencensus-proto/blob/master/src/opencensus/proto/agent/metrics/v1/metrics_service.proto

basic gRPC docs https://grpc.io/docs/tutorials/basic/go.html

Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
@pavolloffay
Copy link
Member Author

pavolloffay commented Nov 14, 2018

One streaming consideration can be performance but I didn't find any good resources. Other use case might be to push configurations to clients. Not sure if we need it for posting batches, maybe if we wanted to get rid of batches and send per span from clients.

@yurishkuro
Copy link
Member

Performance is certainly the main consideration, but we need to demonstrate it with perf tests, rather than assuming.

Using streaming for the config endpoints allows switching the agents to the push model. It did come up in our experiments with adaptive sampling where sometimes the 1min polling interval is not fast enough to reduce traffic spikes from certain services (e.g. periodic batch jobs). However, it's not straightforward because we currently share the collector proxy between collector and config APIs, and for collector API we want the agent to load balance across several collectors, where as for push notifications to work they have to be done over a single stable connection, without load balancing (which indirectly implies that we need a stable partitioning scheme in the collectors by the agent id).

@pavolloffay
Copy link
Member Author

Just copying from the bi-weekly meeting.

Before moving to streaming it would be good to clarify:

  • performance vs rpc
  • load balancing between collectors

for now we have decided to go with rpc and if necessary open another stream port in the future.

@ghost ghost removed the review label Nov 16, 2018
@pavolloffay
Copy link
Member Author

Streams seems to be stateful the requests go to the same backed after the connection is established

In our case we could use between agent and clients where we do not expect load balancing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants