easwar review 2

xds/internal/clients/lrsclient/load_store.go

@@ -23,9 +23,12 @@ package lrsclient
 import "time"
 
 // LoadStore keep track of the loads for multiple clusters and services that
-// are intended to be reported via LRS. One store contains loads reported to
-// one LRS server. To track loads for multiple servers, multiple stores can be
-// created.
+// are intended to be reported via LRS.
+//
+// LoadStore stores loads reported to a single LRS server. Use multiple stores
+// for multiple servers.
+//
+// It is safe for concurrent use.
 type LoadStore struct {
 }
 
@@ -37,6 +40,8 @@ type LoadStore struct {
 //
 // If a cluster's Data is empty (no load to report), it's not appended to the
 // returned slice.
+//
+// Calling Stats clears the previous load data from the LoadStore.
 func (s *LoadStore) Stats(clusterNames []string) []*Data {
 	panic("unimplemented")
 }
@@ -95,8 +100,12 @@ type ServerLoadData struct {
 // PerClusterReporter defines the methods that the LoadStore uses to track
 // per-cluster load reporting data.
 type PerClusterReporter interface {
+	// CallStarted records a call started in the LoadStore.
 	CallStarted(locality string)
+	// CallFinished records a call finished in the LoadStore.
 	CallFinished(locality string, err error)
+	// CallServerLoad records the server load in the LoadStore.
 	CallServerLoad(locality, name string, val float64)
+	// CallDropped records a call dropped in the LoadStore.
 	CallDropped(category string)
 }

xds/internal/clients/lrsclient/client.go → xds/internal/clients/lrsclient/lrsclient.go

@@ -31,6 +31,11 @@ import (
 type LRSClient struct {
 }
 
+// New returns a new LRS Client configured with the provided config.
+func New(config Config) (*LRSClient, error) {
+	panic("unimplemented")
+}
+
 // ReportLoad creates a new load reporting stream for the client. It creates a
 // LoadStore and return it for the caller to report loads.
 func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {

xds/internal/clients/lrsclient/config.go → xds/internal/clients/lrsclient/lrsconfig.go

@@ -23,8 +23,8 @@ import (
 )
 
 // Config is used to configure an LRS client. After one has been passed to an
-// LRS function, it must not be modified. A Config may be used; the LRS package
-// will also not modify it.
+// LRS function, it must not be modified. A Config may be reused; the LRS
+// package will also not modify it.
 type Config struct {
 	// Node is the identity of the client application reporting load to the
 	// LRS server.

xds/internal/clients/xdsclient/resource_type.go

@@ -24,18 +24,16 @@ import (
 )
 
 // ResourceType wraps all resource-type specific functionality. Each supported
-// resource type needs to provide an implementation of this interface.
+// resource type needs to provide an implementation of the Decoder.
 type ResourceType struct {
 	// TypeURL is the xDS type URL of this resource type for the v3 xDS
 	// protocol. This URL is used as the key to look up the corresponding
 	// ResourceType implementation in the ResourceTypes map provided in the
 	// Config.
 	TypeURL string
 
-	// TypeName identifies resources in a transport protocol agnostic way. This
-	// can be used for logging/debugging purposes, as well as in cases where
-	// the resource type name is to be uniquely identified but the actual
-	// functionality provided by the resource type is not required.
+	// TypeName is the shorter representation of the TypeURL to identify the
+	// resource type. It can be used for logging/debugging purposes.
 	TypeName string
 
 	// AllResourcesRequiredInSotW indicates whether this resource type requires

xds/internal/clients/xdsclient/resource_watcher.go

@@ -37,7 +37,7 @@ type ResourceWatcher interface {
 
 	// AmbientError indicates an error occurred after a resource has been
 	// received that should not modify the use of that resource but may be
-	// provide useful information about the ambient state of the XdsClient for
+	// provide useful information about the ambient state of the XDSClient for
 	// debugging purposes. The previous version of the resource should still be
 	// considered valid.
 	AmbientError(err error, onCallbackProcessed func())

xds/internal/clients/xdsclient/client.go → xds/internal/clients/xdsclient/xdsclient.go

@@ -18,7 +18,7 @@
  *
  */
 
-// Package xdsclient provides an xDS (Extensible Discovery Service) client.
+// Package xdsclient provides an xDS (Discovery Service) client.
 //
 // It allows applications to:
 //   - Create xDS client instances with in-memory configurations.
@@ -47,13 +47,12 @@ func New(config Config) (*XDSClient, error) {
 
 // WatchResource starts watching the specified resource.
 //
-// typeURL must be present in the XDSClient's configured ResourceTypes. If
-// typeURL is not present, watch will not be started. The ResourceType
-// obtained from the ResourceTypes against the typeURL will be used to decode
-// the matching resource when it is received, and the watcher will be called
-// with the result.
+// typeURL specifies the resource type implementation to use. The watch fails
+// if there is no resource type implementation for the given typeURL. See the
+// ResourceTypes field in the Config struct used to create the XDSClient.
 //
-// Cancel cancels the watch and prevents future calls to the watcher.
+// The returned function cancels the watch and prevents future calls to the
+// watcher.
 func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) {
 	panic("unimplemented")
 }

xds/internal/clients/xdsclient/config.go → xds/internal/clients/xdsclient/xdsconfig.go

@@ -23,8 +23,8 @@ import (
 )
 
 // Config is used to configure an xDS client. After one has been passed to an
-// xDS function, it must not be modified. A Config may be used; the xDS package
-// will also not modify it.
+// xDS function, it must not be modified. A Config may be reused; the xDS
+// package will also not modify it.
 type Config struct {
 	// Servers specifies a list of xDS management servers to connect to. The
 	// order of the servers in this list reflects the order of preference of
@@ -71,13 +71,15 @@ type Authority struct {
 	// gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
 	XDSServers []clients.ServerConfig
 
-	// Extensions can be populated with arbitrary data to be passed to the xDS
-	// Client's user specific implementations. This field can be used to
-	// provide additional configuration or context specific to the user's
-	// needs.
+	// Extensions can be populated with arbitrary authority-specific data to be
+	// passed from the xDS client configuration down to the user defined
+	// resource decoder implementations. This allows the user to provide
+	// authority-specific context or configuration to their resource
+	// processing logic.
+
 	//
-	// The xDS and LRS clients do not interpret the contents of this field. It
-	// is the responsibility of the user's implementations to handle and
-	// interpret these extensions.
+	// The xDS client do not interpret the contents of this field. It is the
+	// responsibility of the user's implementations to handle and interpret
+	// these extensions.
 	Extensions any
 }