search

grpc / proposal

A repository for gRFCs
Apache License 2.0
719 stars 236 forks source link

L117: C-core: replace gpr logging with absl logging #425

Closed tanvi-jagtap closed 5 months ago

eugeneo commented 6 months ago

Questions:

  1. Will gpr_log and friends get removed or will they remain and wrap ABSL APIs?
  2. Are there any ABSL logging features we should not use in gRPC?
  3. Will GRPC_VERBOSITY and GRPC_TRACE env variables keep working?
  4. Is there a way to suppress the absl::InitializeLog warning for our users? It will force them to add dependency to ABSL to their main function and I anticipate a lot of grumbling.
tanvi-jagtap commented 6 months ago

Questions:

  1. Will gpr_log and friends get removed or will they remain and wrap ABSL APIs?

Yes. Removing them is the plan.

  1. Are there any ABSL logging features we should not use in gRPC?

Not that I know of. However @ctiller may be able to validate that.

  1. Will GRPC_VERBOSITY and GRPC_TRACE env variables keep working?

No changes are expected to GRPC_TRACE and related functions. I will get back to you on GRPC_VERBOSITY . There are some things I need to look into.

  1. Is there a way to suppress the absl::InitializeLog warning for our users? It will force them to add dependency to ABSL to their main function and I anticipate a lot of grumbling.

I will get back on this too.

ctiller commented 6 months ago

Re: suppressing the absl::InitializeLog warning: I highly recommend not doing so. That behavior is controlled by absl, and if we introduce mechanisms over top by default then we're fighting their defaults which is going to lead to more confusion - especially since it's unallowed to call InitializeLog twice.

Ack that there'll be some grumbling.

Re: GRPC_VERBOSITY: I'd propose we introduce some code that if it's set overrides the absl defaults -- but if it's not set we exert no opinion on what absl does.

eugeneo commented 6 months ago

Ack that there'll be some grumbling.

This may cause a lot of frustration with our users, i.e. people may perceive it as some real issue and end up spending time trying to figure out how to fix it, then having to add a dependency to ABSL log (and that may be challenging for people with weird build systems).

I feel like we need to be user friendly.