grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

xds: switch generic xds client to use slog instead of grpclog

Open purnesh42H opened this issue 6 months ago • 4 comments
trafficstars

Currently generic xDS and LRS client https://github.com/grpc/grpc-go/tree/master/xds/internal/clients are using grpclog which is not a long term solution as eventually the generic clients need to be moved to a separate repo.

This proposal recommends to use slog.Logger. The logger should be an optional attribute of the lrsclient.Config and xdsclient.Config. If user doesn't provide the logger, a default slog.Logger should be assigned to it

  1. Update xdsclient.Config and lrsclient.Config to add a new Logger field as slog.Logger type

  2. Modify xdsclient.XDSClient and lrsclient.LRSClient internal components: to use the provided Logger implementation from their respective config structs

type XDSClient struct {
    // ... other fields
    logger      slog.Logger
}

type LRSClient struct {
    // ... other fields
    logger      slog.Logger
}
  1. In the init method of xdsclient.go and lrsclient.go, set the default instance of the slog.Logger
slog.SetDefault(slog.New(slog.NewJSONHandler(os.Stdout, slog.LevelError)))
  1. When the xDS and LRS clients are instantiated, they may or may not receive the logger through their config structs.

xdsclient.go

func newClient(config *Config, watchExpiryTimeout time.Duration, streamBackoff func(int) time.Duration, target string) (*XDSClient, error) {
    .....
    .....

   if config.logger != nil {
       c.logger = config.logger 
   } else {
      c.logger  = slog.Default()
   }
}

lrsclient.go

func New(config Config) (*LRSClient, error) {
	....
        ....
   if config.logger != nil {
       c.logger = config.logger 
   } else {
      c.logger  = slog.Default()
   }
}
  1. Throughout the client's codebase, existing logging statements (which might currently use grpclog, fmt.Println, or another logging library) would be replaced with calls to the methods of the new logger instance.
if c.logger.Enabled(2) {
     c.logger.Info(mgs, )
}
  1. Internal grpc xds client code should be able to provide an slog.Logger to xdsclient.XDSClient that uses grpc.PrefixLogger underneath. This will require to implement a custom slog.Handler that wraps the grpc.PrefixLogger and use that handler in slog.New() when creating the xdsclient instance.

purnesh42H avatar May 13 '25 11:05 purnesh42H

I'm not sure about slog yet - it may be more complex than we need, and it might require that we change grpc over to that same interface, since the xdsclient is used by grpc. For now, maybe we should define an interface that is compatible with slog's logger instead (i.e. has Info/Warn/Error/Enabled methods)?

dfawley avatar May 14 '25 15:05 dfawley

Modified the issue to just add the new interface that mirrors methods from grpclog.PrefixLogger and user must provide their own implementation of it through the configs.

purnesh42H avatar May 15 '25 10:05 purnesh42H

Looks like we should be accepting a *slog.Logger and not the Handler and not our own interface.

dfawley avatar May 28 '25 17:05 dfawley

Looks like we should be accepting a *slog.Logger and not the Handler and not our own interface.

Edited

purnesh42H avatar Jun 05 '25 05:06 purnesh42H