Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add logger #68

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func (c *Workwx) WithApp(corpSecret string, agentID int64) *WorkwxApp {
CorpSecret: corpSecret,
AgentID: agentID,

accessToken: &token{mutex: &sync.RWMutex{}},
jsapiTicket: &token{mutex: &sync.RWMutex{}},
jsapiTicketAgentConfig: &token{mutex: &sync.RWMutex{}},
accessToken: &token{mutex: &sync.RWMutex{}, logger: c.opts.Logger},
jsapiTicket: &token{mutex: &sync.RWMutex{}, logger: c.opts.Logger},
jsapiTicketAgentConfig: &token{mutex: &sync.RWMutex{}, logger: c.opts.Logger},
}
app.accessToken.setGetTokenFunc(app.getAccessToken)
app.jsapiTicket.setGetTokenFunc(app.getJSAPITicket)
Expand Down
22 changes: 22 additions & 0 deletions client_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const DefaultQYAPIHost = "https://qyapi.weixin.qq.com"
type options struct {
QYAPIHost string
HTTP *http.Client
Logger Logger
}

// CtorOption 客户端对象构造参数
Expand All @@ -22,6 +23,7 @@ func defaultOptions() options {
return options{
QYAPIHost: DefaultQYAPIHost,
HTTP: &http.Client{},
Logger: newDefaultLogger(),
}
}

Expand Down Expand Up @@ -62,3 +64,23 @@ var _ CtorOption = (*withHTTPClient)(nil)
func (x *withHTTPClient) applyTo(y *options) {
y.HTTP = x.x
}

//
//
//

type withLogger struct {
x Logger
}

// WithLogger 使用给定的 logger 作为日志打印的方式。如果不使用 WithLogger 自定义 logger,
// 则默认 logger 会将 Info 级别的日志打印在 stdout 中,将 Error 级别的日志打印在 stderr 中。
func WithLogger(logger Logger) CtorOption {
return &withLogger{x: logger}
}

var _ CtorOption = (*withLogger)(nil)

func (x *withLogger) applyTo(y *options) {
y.Logger = x.x
}
27 changes: 27 additions & 0 deletions client_options_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package workwx

import (
"fmt"
"net/http"
"testing"

Expand Down Expand Up @@ -52,3 +53,29 @@ func TestWithQYAPIHost(t *testing.T) {
})
})
}

func TestWithLogger(t *testing.T) {
c.Convey("给定一个 options", t, func() {
opts := options{}

c.Convey("用 WithLogger 修饰它", func() {

l := &myLogger{}
o := WithLogger(l)
o.applyTo(&opts)

c.Convey("options.Logger 应该变了", func() {
c.So(opts.Logger, c.ShouldEqual, l)
})
})
})
}

type myLogger struct{}

func (*myLogger) Info(msg string) {
fmt.Println("INFO: " + msg)
}
func (*myLogger) Error(msg string) {
fmt.Println("ERROR: " + msg)
}
30 changes: 30 additions & 0 deletions logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package workwx

import (
"log"
"os"
)

type Logger interface {
Copy link
Author

Choose a reason for hiding this comment

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

这里我只给 Logger 设置了两个方法,目前来看似乎是足够的。但是考虑到 Go 的特点,如果我们未来向这个 interface 中添加更多方法,这就会变成 breaking change。所以我在犹豫是否需要提前添加更多方法(比如 Debug、Warning)。

Copy link
Owner

@xen0n xen0n Mar 29, 2021

Choose a reason for hiding this comment

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

  1. 函数签名建议搞成仿照 fmt.Printf 的形式,这样用起来会简单一些。 既然都是临时解决方案,想了想,只打 string 也没问题
  2. 可以添加也可以不添加,毕竟到时候 bump 主版本号即可,这个 breaking change 不属于那种很难修复的。
  3. 这么搞实质上阻断了使用结构化 logging 的可能性(打出结构化数据而非格式化完的字符串,例如 logrus 等库所倡导的那样),但考虑到 2 的因素,先简单写一写,也不是不可以。

Copy link
Author

@ocavue ocavue Mar 29, 2021

Choose a reason for hiding this comment

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

函数签名建议搞成仿照 fmt.Printf 的形式,这样用起来会简单一些。

我们是不是可以考虑同时实现两种调用 logger 的方法:InfoInfof?比如像 grpclog 这样。从方便 logger 使用者的角度考虑,有时候我们并不想要一个模版字符串作为日志结构的主体。举例来说下面这几种写法都是蛮常见的:

grpclog.Info("message count:", len(msgs))
grpclog.Infof("received %d messages", len(msgs))

毕竟到时候 bump 主版本号即可

考虑到 Go 升级一个库当中的主版本号(不包含 v0 => v1 的升级)还需要全局替换代码中的包名,我觉得我们还是最好一次把事情做好

这么搞实质上阻断了使用结构化 logging 的可能性

有什么更好的建议吗?

Info(msg string)
Error(msg string)
}

type defaultLogger struct {
stderr *log.Logger
stdout *log.Logger
}

func (l *defaultLogger) Info(msg string) {
l.stdout.Println(msg)
}

func (l *defaultLogger) Error(msg string) {
l.stderr.Println(msg)
}

func newDefaultLogger() *defaultLogger {
stderr := log.New(os.Stderr, "[workwx INFO]", 0)
stdout := log.New(os.Stdout, "[workwx ERR]", 0)
return &defaultLogger{stderr: stderr, stdout: stdout}
}
6 changes: 4 additions & 2 deletions token.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package workwx

import (
"context"
"fmt"
"sync"
"time"

Expand All @@ -18,6 +19,7 @@ type token struct {
tokenInfo
lastRefresh time.Time
getTokenFunc func() (tokenInfo, error)
logger Logger
}

// getAccessToken 获取 access token
Expand Down Expand Up @@ -150,8 +152,7 @@ func (t *token) tokenRefresher(ctx context.Context) {
case <-time.After(waitDuration):
retryer := backoff.WithContext(backoff.NewExponentialBackOff(), ctx)
if err := backoff.Retry(t.syncToken, retryer); err != nil {
// TODO: logging
_ = err
t.logger.Error(fmt.Sprintf("failed to refresh token: %v. will retry.", err))
}

waitUntilTime := t.lastRefresh.Add(t.expiresIn).Add(-refreshTimeWindow)
Expand All @@ -160,6 +161,7 @@ func (t *token) tokenRefresher(ctx context.Context) {
waitDuration = minRefreshDuration
}
case <-ctx.Done():
t.logger.Info("tokenRefresher terminated")
return
}
}
Expand Down