Skip to content

Conversation

@achichen
Copy link
Contributor

@achichen achichen commented Apr 26, 2023

Just a PoC for scoped logging + attr

  • Logger can be created in scope with attrs
  • Subscope logger inherits parent scope's logger attrs

See example/main.go

)

logging.InfoV2(ctx, "before calling bar()", logging.WithAttr("foo3", "foo3"))
//{"@timestamp":"2023-04-27T00:39:45.574018+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"foo1":"foo1","foo2":456,"foo3":"foo3","msg":"before calling bar()"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from parent scope: "main1", "main2"
  • attrs from current scope: "foo1", "foo2"
  • adhoc attrs : "foo3"

bar(ctx)

logging.InfoV2(ctx, "after called bar()", logging.WithAttr("result", "bar result"))
//{"@timestamp":"2023-04-27T00:39:45.574068+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"foo1":"foo1","foo2":456,"result":"bar result","msg":"after called bar()"}
Copy link
Contributor Author

@achichen achichen Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from parent scope: "main1", "main2"
  • attrs from current scope: "foo1", "foo2"
  • adhoc attrs : "result"

NOTE that this log appears after calling bar(ctx), but the attrs within bar(ctx) does not leak out

logging.WithAttr("bar2", 789),
)
logging.InfoV2(ctx, "before do something", logging.WithAttr("bar3", 9876))
//{"@timestamp":"2023-04-27T00:39:45.574037+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"foo1":"foo1","foo2":456,"bar1":"bar1","bar2":789,"bar3":9876,"msg":"before do something"}
Copy link
Contributor Author

@achichen achichen Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from grantparent scope: "main1", "main2"
  • attrs from parent scope: "foo1", "foo2"
  • attrs from current scope: "bar1", "bar2"
  • adhoc attrs : "bar3"

//{"@timestamp":"2023-04-27T00:39:45.574037+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"foo1":"foo1","foo2":456,"bar1":"bar1","bar2":789,"bar3":9876,"msg":"before do something"}

logging.InfoV2(ctx, "after do something", logging.WithAttr("result", "success"))
//{"@timestamp":"2023-04-27T00:39:45.574055+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"foo1":"foo1","foo2":456,"bar1":"bar1","bar2":789,"result":"success","msg":"after do something"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from grantparent scope: "main1", "main2"
  • attrs from parent scope: "foo1", "foo2"
  • attrs from current scope: "bar1", "bar2"
  • adhoc attrs : "result"


foo(ctx)
logging.InfoV2(ctx, "after called foo()")
//{"@timestamp":"2023-04-27T00:39:45.574079+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"msg":"after called foo()"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from current scope: "main1", "main2"

NOTE that this log appears after calling foo(ctx), but the attrs within foo(ctx) does not leak out

)

logging.InfoV2(ctx, "before calling foo()")
//{"@timestamp":"2023-04-27T00:39:45.573743+08:00","caller":"go-logging/new.go:33","level":"info","main1":"main1","main2":123,"msg":"before calling foo()"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • attrs from current scope: "main1", "main2"


ctx = logging.WithLogger(ctx,
logging.WithAttr("main1", "main1"),
logging.WithAttr("main2", 123),
Copy link
Contributor Author

@achichen achichen Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel this is a bit verbose, but cannot think of a succinct way.

One possible approach is to make shortcut function name might help, like logging.A(key,value). Similar to golang's testing package.

func WithLogger(ctx context.Context, attrs ...Attr) context.Context {
if parentLogger, ok := GetLoggerV2(ctx).(loggerV2); ok {
attrs = append(attrs, parentLogger.attrs...)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inherit parent scope logger attrs

logging.InfoV3(ctx, "msg v3 without attributes", nil)
//{"@timestamp":"2023-04-27T00:01:39.701337+05:30","caller":"go-logging/new.go:35","level":"info","main1":"main1","main2":123,"msg":"msg v3 without attributes"}

logging.InfoV3(ctx, "msg v3", logging.AttrMap{"listing_id": "1000323749", "is_exp_enabled": true})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bhadreswar-ghuku. I was considering taking map[string]interface{}. But this looks even better.

return logger
}

func InfoV2(ctx context.Context, msg string, attrs ...Attr) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using verbose function name InfoV2 just to avoid conflict with existing Info() in old interface.

We will have to deal with function conflict if we are adding new interfaces. But I am open for other options like
a) completely remove old interface from this library (as this library is not yet published, backward compatibility is not the mandatory consideration IMO)
b) create v2 version of this library, and fully switching to new interface
c) adding new interface in a sub-package (but i have no idea a good name atm)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants