Skip to content
New issue

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

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

Already on GitHub? # to your account

Support BraveRpcService #6084

Open
jrhee17 opened this issue Feb 3, 2025 · 1 comment
Open

Support BraveRpcService #6084

jrhee17 opened this issue Feb 3, 2025 · 1 comment

Comments

@jrhee17
Copy link
Contributor

jrhee17 commented Feb 3, 2025

Motivation

For high-traffic services, it may be unfeasible to add tracing for all requests due to the large volume.
It is a common practice to sample traces for specific requests (e.g. by user id) instead of random sampling.

Currently, BraveService already provides a way to do this via either 1) Tracing.newBuilder().sampler(...) or 2) HttpTracing.newBuilder(...).serverSampler().
However, sometimes users would like to sample based on a parameter parsed by rpc requests.

Because BraveService determines whether a trace should be sampled before the request is parsed into a rpc request, it is difficult for users to sample based on RPC requests.

Proposal

I propose that a BraveRpcService is introduced. By doing so, users may sample traces based on RPC parameters more easily.
By doing so, users will be able to sample traces based on ServiceRequestContext#rpcRequest

i.e.

RpcTracing.newBuilder(tracing)
          .serverSampler(arg -> {
              final ServiceRequestContext ctx = (ServiceRequestContext) arg.unwrap();
              final RpcRequest rpcRequest = ctx.rpcRequest();
              if (rpcRequest != null && "...".equals(rpcRequest.serviceName())) {
                  return true;
              }
              return null;
          });

One caveat is that BraveRpcService sampler won't have an effect if a BraveService is already in the decorator chain and was processed.
The additional span containing RPC specific information would still probably be useful though.

@jrhee17 jrhee17 added this to the 1.32.0 milestone Feb 3, 2025
@minwoox
Copy link
Contributor

minwoox commented Feb 4, 2025

Sounds good. 👍

@minwoox minwoox removed this from the 1.32.0 milestone Feb 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants