refactor: Simplify setlogoption chain element usage + fix few issues (n…

…etworkservicemesh#580) * simplify setlogoption chain element usage Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com> * fix review comments Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com> * apply self code review Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com> Signed-off-by: Sergey Ershov <sergey.ershov@xored.com>
illbegood · Dec 20, 2020 · dcde680 · dcde680
1 parent 515d528
commit dcde680
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 125 deletions.
diff --git a/pkg/networkservice/chains/endpoint/server.go b/pkg/networkservice/chains/endpoint/server.go
@@ -28,8 +28,6 @@ import (
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatetoken"
-	"github.com/networkservicemesh/sdk/pkg/networkservice/core/setlogoption"
-
 	"github.com/networkservicemesh/sdk/pkg/tools/grpcutils"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/monitor"
@@ -62,18 +60,18 @@ type endpoint struct {
 //             - additionalFunctionality - any additional NetworkServiceServer chain elements to be included in the chain
 func NewServer(ctx context.Context, name string, authzServer networkservice.NetworkServiceServer, tokenGenerator token.GeneratorFunc, additionalFunctionality ...networkservice.NetworkServiceServer) Endpoint {
 	rv := &endpoint{}
-	rv.NetworkServiceServer = setlogoption.NewServer(map[string]string{"chain": name},
-		chain.NewNetworkServiceServer(
-			append([]networkservice.NetworkServiceServer{
-				authzServer,
-				updatepath.NewServer(name),
-				// `timeout` uses ctx as a context for the timeout Close and it closes only the subsequent chain, so
-				// chain elements before the `timeout` in chain shouldn't make any updates to the Close context and
-				// shouldn't be closed on Connection Close.
-				timeout.NewServer(ctx),
-				monitor.NewServer(ctx, &rv.MonitorConnectionServer),
-				updatetoken.NewServer(tokenGenerator),
-			}, additionalFunctionality...)...))
+	rv.NetworkServiceServer = chain.NewNamedNetworkServiceServer(
+		name,
+		append([]networkservice.NetworkServiceServer{
+			authzServer,
+			updatepath.NewServer(name),
+			// `timeout` uses ctx as a context for the timeout Close and it closes only the subsequent chain, so
+			// chain elements before the `timeout` in chain shouldn't make any updates to the Close context and
+			// shouldn't be closed on Connection Close.
+			timeout.NewServer(ctx),
+			monitor.NewServer(ctx, &rv.MonitorConnectionServer),
+			updatetoken.NewServer(tokenGenerator),
+		}, additionalFunctionality...)...)
 	return rv
 }
 

diff --git a/pkg/networkservice/chains/nsmgr/server.go b/pkg/networkservice/chains/nsmgr/server.go
@@ -21,11 +21,9 @@ package nsmgr
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/networkservicemesh/sdk/pkg/registry/common/querycache"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
-	setlogoption_reg "github.com/networkservicemesh/sdk/pkg/registry/core/setlogoption"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/filtermechanisms"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/interpose"
@@ -127,20 +125,17 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 			clientDialOptions...),
 	)
 
-	nsChain := setlogoption_reg.NewNetworkServiceRegistryServer(
-		map[string]string{"chain": fmt.Sprintf("%s:Registry", nsmRegistration.Name)},
-		chain_registry.NewNetworkServiceRegistryServer(nsRegistry),
+	nsChain := chain_registry.NewNamedNetworkServiceRegistryServer(nsmRegistration.Name+".NetworkServiceRegistry", nsRegistry)
+
+	nseChain := chain_registry.NewNamedNetworkServiceEndpointRegistryServer(
+		nsmRegistration.Name+".NetworkServiceEndpointRegistry",
+		newRecvFDEndpointRegistry(), // Allow to receive a passed files
+		urlsRegistryServer,
+		interposeRegistry,         // Store cross connect NSEs
+		localbypassRegistryServer, // Store endpoint Id to EndpointURL for local access.
+		seturl.NewNetworkServiceEndpointRegistryServer(nsmRegistration.Url), // Remember endpoint URL
+		nseRegistry, // Register NSE inside Remote registry with ID assigned
 	)
-	nseChain := setlogoption_reg.NewNetworkServiceEndpointRegistryServer(
-		map[string]string{"chain": fmt.Sprintf("%s:Registry", nsmRegistration.Name)},
-		chain_registry.NewNetworkServiceEndpointRegistryServer(
-			newRecvFDEndpointRegistry(), // Allow to receive a passed files
-			urlsRegistryServer,
-			interposeRegistry,         // Store cross connect NSEs
-			localbypassRegistryServer, // Store endpoint Id to EndpointURL for local access.
-			seturl.NewNetworkServiceEndpointRegistryServer(nsmRegistration.Url), // Remember endpoint URL
-			nseRegistry, // Register NSE inside Remote registry with ID assigned
-		))
 	rv.Registry = registry.NewServer(nsChain, nseChain)
 
 	return rv

diff --git a/pkg/networkservice/core/chain/server.go b/pkg/networkservice/core/chain/server.go
@@ -22,6 +22,7 @@ import (
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"github.com/sirupsen/logrus"
 
+	"github.com/networkservicemesh/sdk/pkg/networkservice/common/setlogoption"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/trace"
 )
@@ -33,3 +34,14 @@ func NewNetworkServiceServer(servers ...networkservice.NetworkServiceServer) net
 	}
 	return next.NewNetworkServiceServer(servers...)
 }
+
+// NewNamedNetworkServiceServer - chains together a list of networkservice.Servers with tracing and name log option
+func NewNamedNetworkServiceServer(name string, servers ...networkservice.NetworkServiceServer) networkservice.NetworkServiceServer {
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewNetworkServiceServer(
+			setlogoption.NewServer(map[string]string{"name": name}),
+			next.NewWrappedNetworkServiceServer(trace.NewNetworkServiceServer, servers...),
+		)
+	}
+	return next.NewNetworkServiceServer(servers...)
+}
diff --git a/pkg/networkservice/core/setlogoption/server.go b/pkg/networkservice/core/setlogoption/server.go
diff --git a/pkg/registry/core/setlogoption/ns_server.go → ...registry/common/setlogoption/ns_server.go b/pkg/registry/core/setlogoption/ns_server.go → ...registry/common/setlogoption/ns_server.go
@@ -14,14 +14,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package setlogoption implements a chain element to set log options before full chain
 package setlogoption
 
 import (
 	"context"
 
 	"github.com/sirupsen/logrus"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
 
 	"github.com/golang/protobuf/ptypes/empty"
@@ -30,42 +30,40 @@ import (
 
 type setNSLogOption struct {
 	options map[string]string
-	server  registry.NetworkServiceRegistryServer
 }
 
-type setNSLogOptionFindServer struct {
+type setLogOptionFindServer struct {
 	registry.NetworkServiceRegistry_FindServer
 	ctx context.Context
 }
 
-func (s *setNSLogOptionFindServer) Send(service *registry.NetworkService) error {
-	return s.NetworkServiceRegistry_FindServer.Send(service)
+func (s *setLogOptionFindServer) Send(ns *registry.NetworkService) error {
+	return s.NetworkServiceRegistry_FindServer.Send(ns)
 }
 
-func (s *setNSLogOptionFindServer) Context() context.Context {
+func (s *setLogOptionFindServer) Context() context.Context {
 	return s.ctx
 }
 
-func (s *setNSLogOption) Register(ctx context.Context, service *registry.NetworkService) (*registry.NetworkService, error) {
+func (s *setNSLogOption) Register(ctx context.Context, ns *registry.NetworkService) (*registry.NetworkService, error) {
 	ctx = s.withFields(ctx)
-	return s.server.Register(ctx, service)
+	return next.NetworkServiceRegistryServer(ctx).Register(ctx, ns)
 }
 
 func (s *setNSLogOption) Find(query *registry.NetworkServiceQuery, server registry.NetworkServiceRegistry_FindServer) error {
 	ctx := s.withFields(server.Context())
-	return s.server.Find(query, &setNSLogOptionFindServer{ctx: ctx, NetworkServiceRegistry_FindServer: server})
+	return next.NetworkServiceRegistryServer(ctx).Find(query, &setLogOptionFindServer{ctx: ctx, NetworkServiceRegistry_FindServer: server})
 }
 
-func (s *setNSLogOption) Unregister(ctx context.Context, service *registry.NetworkService) (*empty.Empty, error) {
+func (s *setNSLogOption) Unregister(ctx context.Context, ns *registry.NetworkService) (*empty.Empty, error) {
 	ctx = s.withFields(ctx)
-	return s.server.Unregister(ctx, service)
+	return next.NetworkServiceRegistryServer(ctx).Unregister(ctx, ns)
 }
 
-// NewNetworkServiceRegistryServer creates new instance of NewNetworkServiceRegistryServer which sets the passed options
-func NewNetworkServiceRegistryServer(options map[string]string, server registry.NetworkServiceRegistryServer) registry.NetworkServiceRegistryServer {
+// NewNetworkServiceRegistryServer creates new instance of NetworkServiceRegistryServer which sets the passed options
+func NewNetworkServiceRegistryServer(options map[string]string) registry.NetworkServiceRegistryServer {
 	return &setNSLogOption{
 		options: options,
-		server:  server,
 	}
 }
 

diff --git a/pkg/registry/core/setlogoption/nse_server.go → ...egistry/common/setlogoption/nse_server.go b/pkg/registry/core/setlogoption/nse_server.go → ...egistry/common/setlogoption/nse_server.go
@@ -22,54 +22,53 @@ import (
 
 	"github.com/sirupsen/logrus"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
 
 	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/networkservicemesh/api/pkg/api/registry"
 )
 
-type setLogOption struct {
+type setNSELogOption struct {
 	options map[string]string
-	server  registry.NetworkServiceEndpointRegistryServer
 }
 
-type setLogOptionFindServer struct {
+type setNSELogOptionFindServer struct {
 	registry.NetworkServiceEndpointRegistry_FindServer
 	ctx context.Context
 }
 
-func (s *setLogOptionFindServer) Send(endpoint *registry.NetworkServiceEndpoint) error {
+func (s *setNSELogOptionFindServer) Send(endpoint *registry.NetworkServiceEndpoint) error {
 	return s.NetworkServiceEndpointRegistry_FindServer.Send(endpoint)
 }
 
-func (s *setLogOptionFindServer) Context() context.Context {
+func (s *setNSELogOptionFindServer) Context() context.Context {
 	return s.ctx
 }
 
-func (s *setLogOption) Register(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
+func (s *setNSELogOption) Register(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
 	ctx = s.withFields(ctx)
-	return s.server.Register(ctx, endpoint)
+	return next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, endpoint)
 }
 
-func (s *setLogOption) Find(query *registry.NetworkServiceEndpointQuery, server registry.NetworkServiceEndpointRegistry_FindServer) error {
+func (s *setNSELogOption) Find(query *registry.NetworkServiceEndpointQuery, server registry.NetworkServiceEndpointRegistry_FindServer) error {
 	ctx := s.withFields(server.Context())
-	return s.server.Find(query, &setLogOptionFindServer{ctx: ctx, NetworkServiceEndpointRegistry_FindServer: server})
+	return next.NetworkServiceEndpointRegistryServer(ctx).Find(query, &setNSELogOptionFindServer{ctx: ctx, NetworkServiceEndpointRegistry_FindServer: server})
 }
 
-func (s *setLogOption) Unregister(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
+func (s *setNSELogOption) Unregister(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
 	ctx = s.withFields(ctx)
-	return s.server.Unregister(ctx, endpoint)
+	return next.NetworkServiceEndpointRegistryServer(ctx).Unregister(ctx, endpoint)
 }
 
 // NewNetworkServiceEndpointRegistryServer creates new instance of NetworkServiceEndpointRegistryServer which sets the passed options
-func NewNetworkServiceEndpointRegistryServer(options map[string]string, server registry.NetworkServiceEndpointRegistryServer) registry.NetworkServiceEndpointRegistryServer {
-	return &setLogOption{
+func NewNetworkServiceEndpointRegistryServer(options map[string]string) registry.NetworkServiceEndpointRegistryServer {
+	return &setNSELogOption{
 		options: options,
-		server:  server,
 	}
 }
 
-func (s *setLogOption) withFields(ctx context.Context) context.Context {
+func (s *setNSELogOption) withFields(ctx context.Context) context.Context {
 	fields := make(logrus.Fields)
 	for k, v := range s.options {
 		fields[k] = v

diff --git a/pkg/registry/core/chain/ns_registry.go b/pkg/registry/core/chain/ns_registry.go
@@ -20,17 +20,35 @@ package chain
 
 import (
 	"github.com/networkservicemesh/api/pkg/api/registry"
+	"github.com/sirupsen/logrus"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/common/setlogoption"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/trace"
 )
 
 // NewNetworkServiceRegistryServer - creates a chain of servers
 func NewNetworkServiceRegistryServer(servers ...registry.NetworkServiceRegistryServer) registry.NetworkServiceRegistryServer {
-	return next.NewWrappedNetworkServiceRegistryServer(trace.NewNetworkServiceRegistryServer, servers...)
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewWrappedNetworkServiceRegistryServer(trace.NewNetworkServiceRegistryServer, servers...)
+	}
+	return next.NewNetworkServiceRegistryServer(servers...)
+}
+
+// NewNamedNetworkServiceRegistryServer - creates a chain of servers with name log option if tracing enabled
+func NewNamedNetworkServiceRegistryServer(name string, servers ...registry.NetworkServiceRegistryServer) registry.NetworkServiceRegistryServer {
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewNetworkServiceRegistryServer(
+			setlogoption.NewNetworkServiceRegistryServer(map[string]string{"name": name}),
+			next.NewWrappedNetworkServiceRegistryServer(trace.NewNetworkServiceRegistryServer, servers...))
+	}
+	return next.NewNetworkServiceRegistryServer(servers...)
 }
 
 // NewNetworkServiceRegistryClient - creates a chain of clients
 func NewNetworkServiceRegistryClient(clients ...registry.NetworkServiceRegistryClient) registry.NetworkServiceRegistryClient {
-	return next.NewWrappedNetworkServiceRegistryClient(trace.NewNetworkServiceRegistryClient, clients...)
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewWrappedNetworkServiceRegistryClient(trace.NewNetworkServiceRegistryClient, clients...)
+	}
+	return next.NewNetworkServiceRegistryClient(clients...)
 }
diff --git a/pkg/registry/core/chain/nse_registry.go b/pkg/registry/core/chain/nse_registry.go
@@ -1,3 +1,5 @@
+// Copyright (c) 2020 Cisco Systems, Inc.
+//
 // Copyright (c) 2020 Doc.ai and/or its affiliates.
 //
 // SPDX-License-Identifier: Apache-2.0
@@ -19,17 +21,35 @@ package chain
 
 import (
 	"github.com/networkservicemesh/api/pkg/api/registry"
+	"github.com/sirupsen/logrus"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/common/setlogoption"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/next"
 	"github.com/networkservicemesh/sdk/pkg/registry/core/trace"
 )
 
 // NewNetworkServiceEndpointRegistryServer - creates a chain of servers
 func NewNetworkServiceEndpointRegistryServer(servers ...registry.NetworkServiceEndpointRegistryServer) registry.NetworkServiceEndpointRegistryServer {
-	return next.NewWrappedNetworkServiceEndpointRegistryServer(trace.NewNetworkServiceEndpointRegistryServer, servers...)
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewWrappedNetworkServiceEndpointRegistryServer(trace.NewNetworkServiceEndpointRegistryServer, servers...)
+	}
+	return next.NewNetworkServiceEndpointRegistryServer(servers...)
+}
+
+// NewNamedNetworkServiceEndpointRegistryServer - creates a chain of servers with name log option if tracing enabled
+func NewNamedNetworkServiceEndpointRegistryServer(name string, servers ...registry.NetworkServiceEndpointRegistryServer) registry.NetworkServiceEndpointRegistryServer {
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewNetworkServiceEndpointRegistryServer(
+			setlogoption.NewNetworkServiceEndpointRegistryServer(map[string]string{"name": name}),
+			next.NewWrappedNetworkServiceEndpointRegistryServer(trace.NewNetworkServiceEndpointRegistryServer, servers...))
+	}
+	return next.NewNetworkServiceEndpointRegistryServer(servers...)
 }
 
 // NewNetworkServiceEndpointRegistryClient - creates a chain of clients
 func NewNetworkServiceEndpointRegistryClient(clients ...registry.NetworkServiceEndpointRegistryClient) registry.NetworkServiceEndpointRegistryClient {
-	return next.NewWrappedNetworkServiceEndpointRegistryClient(trace.NewNetworkServiceEndpointRegistryClient, clients...)
+	if logrus.GetLevel() == logrus.TraceLevel {
+		return next.NewWrappedNetworkServiceEndpointRegistryClient(trace.NewNetworkServiceEndpointRegistryClient, clients...)
+	}
+	return next.NewNetworkServiceEndpointRegistryClient(clients...)
 }