switch from juju/errors to pingcap/errors

[gregwebs]

I tried to do this switch, but what held me back is that tidb is vendored and still using juju/errors, including some of its esoteric APIs.
The latest version of tidb has switched to using pingcap/errors. Similarly, pd has switched to pkg/errors.
So I am hoping that the vendored tidb can be updated.

One of the main motivations for me to prompt tidb-binlog to do this change is that juju/errors is LGPL code, which makes proper distribution of tidb-binlog more difficult.

[iamxy]

Please update the related vendored codes. @GregoryIan

[gregwebs]

These changes are in 2.1: you may need to wait for the stable 2.1 release

[kennytm]

tidb-binlog has two LGPL dependencies: juju/errors and ngaut/log. Replacing juju/errors is straightforward with TiDB ≥ v2.1.0-rc.3, but ngaut/log is more difficult.

In TiDB the replacement is sirupsen/logrus for logging, and gopkg.in/natefinch/lumberjack.v2 for log rotation. However, binlog supports rotating by hours, whereas the minimum granularity for lumberjack is days. This makes lumberjack not a suitable replacement of ngaut/log.

[gregwebs]

Thanks for looking into this. I am trying out the zap logger now also :).

I much prefer to leave log rotation to an external tool. In general I prefer to log to stdout and leave it up to the deployment details what happens next. Assuming logging to the machine where it is running is not cloud friendly. We would want the option to ship the log elsewhere. Still its just as possible for the deployment to redirect stdout to a file and use a log rotation tool.

[gregwebs]

Would we need to coordinate with the ansible based deployment to see how we can have that deployment ensure log rotation? @LinuxGit

[gregwebs]

Although in the long-run I do think it is best to avoid coupling log rotation to the binlog process, I did see that zap mentions integration with lumberjack.

[kennytm]

Applied lumberjack's recommendation on how to do time-based rotation, so ngaut/log is no longer the blocking issue.

tidb-binlog/pkg/util/util.go

Lines 90 to 103 in 8f996ce

	// see https://github.com/natefinch/lumberjack/issues/17#issuecomment-185846531 for how to do time-based rotation
	if rotateLog, ok := log.StandardLogger().Out.(*lumberjack.Logger); ok {
	var interval time.Duration
	if rotateBy == "hour" {
	interval = time.Hour
	} else {
	interval = 24 * time.Hour
	}
	go func() {
	for range time.Tick(interval) {
	rotateLog.Rotate()
	}
	}()
	}

The next issue is that drainer depends on zanmato1984/clickhouse (MIT), which itself depends on juju/errors

https://github.com/zanmato1984/clickhouse/blob/6c32f7b4cd79451e23e7369c500d082815380b3e/lib/column/decimal.go#L9

The only feature used is errors.Trace so it can be entirely replaced by pingcap/errors in go.mod, but I'm not sure if this is "legal".

replace github.com/juju/errors => github.com/pingcap/errors v0.10.1

[gregwebs]

Thanks for figuring out the log rotation!

Replacing a dependency is perfectly legal. We use Gopkg to replace pkg/errors, but you could do the same to replace juju/errors: https://github.com/pingcap/tidb/blob/master/Gopkg.toml#L101.
I don't know how to do that technique with go modules yet.

We could also send a pull request to the clickhouse driver. pingcap/errors offers stack traces, which could be very helpful.

[july2993]

Thanks gregwebs
have switch juju/errors to pingcap/error in #464
drop juju/errors in zanmato1984/clickhouse by zanmato1984/clickhouse#1
for ngaut/log, will switch to pingcap/log later.
so close this first.