removed usage of setlogoption chain element #1186

Mixaster995 · 2021-12-02T04:15:55Z

Signed-off-by: Mikhail Avramenko [email protected]

Description

Removed excessive chain element setlopoption

Issue link

closes #1147

How Has This Been Tested?

Added unit testing to cover
Tested manually
Tested by integration testing
Have not tested

Types of changes

Signed-off-by: Mikhail Avramenko <[email protected]>

denis-tingaikin

Looks good. Added a few comments.

denis-tingaikin · 2021-12-02T06:52:42Z

pkg/registry/core/trace/context.go

@@ -27,6 +27,10 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/tools/log/spanlogger"
 )

+const (
+	loggedType = "registry"


Let's use this inline

Suggested change

loggedType = "registry"

denis-tingaikin · 2021-12-02T06:54:05Z

pkg/tools/log/logruslogger/logruslogger.go

@@ -185,7 +185,7 @@ func (s *logrusLogger) WithField(key, value interface{}) log.Logger {

 // New - creates a logruslogger and returns it
 func New(ctx context.Context) log.Logger {
-	entry := logrus.WithFields(log.Fields(ctx))
+	entry := logrus.NewEntry(logrus.StandardLogger())


Why is this changed?

Also, who is using logruslogger.New?

i removed log.Fields(ctx) method, so it had to be changed
logruslogger.New used in cmd-...s

Please note, cmd- previously could put fields into ctx. So please add logrus.Field variadic.

denis-tingaikin · 2021-12-02T22:01:01Z

pkg/tools/log/logruslogger/logruslogger.go

@@ -185,7 +185,7 @@ func (s *logrusLogger) WithField(key, value interface{}) log.Logger {

 // New - creates a logruslogger and returns it
 func New(ctx context.Context) log.Logger {


Suggested change

func New(ctx context.Context) log.Logger {

func New(ctx context.Context, fields ... logrus.Field) log.Logger {

Signed-off-by: Mikhail Avramenko <[email protected]>

denis-tingaikin · 2021-12-05T14:53:30Z

pkg/tools/log/logruslogger/logruslogger.go

+// Field is simple key value pair
+type Field struct {
+	key string
+	val interface{}
+}
+
+func toMap(fields ...Field) map[string]interface{} {
+	rv := make(map[string]interface{})
+
+	for _, f := range fields {
+		rv[f.key] = f.val
+	}
+
+	return rv
+}


Suggested change

// Field is simple key value pair

type Field struct {

key string

val interface{}

}

func toMap(fields ...Field) map[string]interface{} {

rv := make(map[string]interface{})

for _, f := range fields {

rv[f.key] = f.val

}

return rv

}

denis-tingaikin · 2021-12-05T14:53:53Z

pkg/tools/log/logruslogger/logruslogger.go

 // New - creates a logruslogger and returns it
-func New(ctx context.Context) log.Logger {
-	entry := logrus.NewEntry(logrus.StandardLogger())
+func New(ctx context.Context, fields ...Field) log.Logger {


Suggested change

func New(ctx context.Context, fields ...Field) log.Logger {

func New(ctx context.Context, fields ...logrus.Fields) log.Logger {

denis-tingaikin · 2021-12-05T14:57:23Z

pkg/tools/log/logruslogger/logruslogger.go

-func New(ctx context.Context) log.Logger {
-	entry := logrus.NewEntry(logrus.StandardLogger())
+func New(ctx context.Context, fields ...Field) log.Logger {
+	entry := logrus.WithFields(toMap(fields...))


Suggested change

entry := logrus.WithFields(toMap(fields...))

var entry = logrus.NewEntry(logrus.New())

for _, fieldMap := range fields {

entry = entry.WithFields(fieldMap)

}

Signed-off-by: Mikhail Avramenko <[email protected]>

…k@main PR link: networkservicemesh/sdk#1186 Commit: c6c78c3 Author: Авраменко Михаил Date: 2021-12-06 14:26:30 +0700 Message: - removed usage of setlogoption chain element (#1186) * removed usage of setlogoption chain element Signed-off-by: Mikhail Avramenko <[email protected]> * added variadic fields to log creation Signed-off-by: Mikhail Avramenko <[email protected]> * used logrus fields instead of custom type Signed-off-by: NSMBot <[email protected]>

removed usage of setlogoption chain element

206497a

Signed-off-by: Mikhail Avramenko <[email protected]>

Mixaster995 force-pushed the fix/remove-setlogoption branch from 1c47243 to 206497a Compare December 2, 2021 04:31

denis-tingaikin reviewed Dec 2, 2021

View reviewed changes

Mixaster995 assigned glazychev-art and NikitaSkrynnik and unassigned NikitaSkrynnik Dec 2, 2021

Mixaster995 mentioned this pull request Dec 2, 2021

Removed adapters #1149

Closed

5 tasks

denis-tingaikin reviewed Dec 2, 2021

View reviewed changes

added variadic fields to log creation

f4b93f6

Signed-off-by: Mikhail Avramenko <[email protected]>

Mixaster995 force-pushed the fix/remove-setlogoption branch from b3329fb to f4b93f6 Compare December 3, 2021 06:16

denis-tingaikin requested changes Dec 5, 2021

View reviewed changes

used logrus fields instead of custom type

d95815a

Signed-off-by: Mikhail Avramenko <[email protected]>

denis-tingaikin approved these changes Dec 6, 2021

View reviewed changes

denis-tingaikin merged commit c6c78c3 into networkservicemesh:main Dec 6, 2021

This was referenced Dec 6, 2021

Update from update/networkservicemesh/sdk networkservicemesh/cmd-nse-vfio#282

Closed

Update from update/networkservicemesh/sdk networkservicemesh/cmd-nsc-init#309

Closed

Mixaster995 deleted the fix/remove-setlogoption branch December 6, 2021 08:16

Mixaster995 mentioned this pull request Dec 7, 2021

TestHealClient_FindTest fails #1157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed usage of setlogoption chain element #1186

removed usage of setlogoption chain element #1186

Mixaster995 commented Dec 2, 2021

denis-tingaikin left a comment

denis-tingaikin Dec 2, 2021

denis-tingaikin Dec 2, 2021

Mixaster995 Dec 2, 2021 •

edited

Loading

denis-tingaikin Dec 2, 2021

denis-tingaikin Dec 2, 2021

Mixaster995 Dec 3, 2021

denis-tingaikin Dec 5, 2021

denis-tingaikin Dec 5, 2021

Mixaster995 Dec 6, 2021

denis-tingaikin Dec 5, 2021

Mixaster995 Dec 6, 2021

		@@ -185,7 +185,7 @@ func (s *logrusLogger) WithField(key, value interface{}) log.Logger {

		// New - creates a logruslogger and returns it
		func New(ctx context.Context) log.Logger {

	func New(ctx context.Context) log.Logger {
	func New(ctx context.Context, fields ... logrus.Field) log.Logger {

-	entry := logrus.WithFields(toMap(fields...))
+	var entry = logrus.NewEntry(logrus.New())
+	for _, fieldMap := range fields {
+		entry = entry.WithFields(fieldMap)
+	}

removed usage of setlogoption chain element #1186

removed usage of setlogoption chain element #1186

Conversation

Mixaster995 commented Dec 2, 2021

Description

Issue link

How Has This Been Tested?

Types of changes

denis-tingaikin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mixaster995 Dec 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mixaster995 Dec 2, 2021 •

edited

Loading