vitessio / vitess

Vitess is a database clustering system for horizontal scaling of MySQL.
http://vitess.io
Apache License 2.0
18.21k stars 2.07k forks source link

Deprecate syslogging #5844

Open sougou opened 4 years ago

sougou commented 4 years ago

There is old code in vitess that invokes syslog. This has mostly been problematic:

I recommend we delete this:

This most likely changes all those event logging invocations to be no-ops. We can retain those calls and later find a better plugin than syslog.

aquarapid commented 4 years ago

There is actually syslog-referencing code all over the code-base, as you mentioned:

$ grep -r syslog * | cut -d: -f1 | sort -u | grep go$
go/cmd/vtctl/vtctl.go
go/cmd/vttablet/plugin_sysloglogger.go
go/event/syslogger/syslogger.go
go/event/syslogger/syslogger_test.go
go/vt/topo/events/keyspace_change_syslog.go
go/vt/topo/events/keyspace_change_syslog_test.go
go/vt/topo/events/shard_change_syslog.go
go/vt/topo/events/shard_change_syslog_test.go
go/vt/topo/events/tablet_change_syslog.go
go/vt/topo/events/tablet_change_syslog_test.go
go/vt/topotools/events/migrate_syslog.go
go/vt/topotools/events/migrate_syslog_test.go
go/vt/topotools/events/reparent_syslog.go
go/vt/topotools/events/reparent_syslog_test.go
go/vt/vttablet/filelogger/filelogger.go
go/vt/vttablet/sysloglogger/sysloglogger.go
go/vt/vttablet/sysloglogger/sysloglogger_test.go
go/vt/worker/events/split_syslog.go
go/vt/worker/events/split_syslog_test.go

I think we should keep it and document it more prominently. My arguments:

sougou commented 4 years ago

But it could cause problems for someone who doesn't know that it's turned on right? How about turning this off by default, and let those who want it turn it on.

derekperkins commented 4 years ago

I'm totally behind disabling it by default

makmanalp commented 3 years ago

We (HubSpot) also vote to either nuke it or have an option to disable it. Right now it either prints out an error message we don't care about, or we have to do some hackery to shut it up.

tspotts commented 1 year ago

Agree with what @makmanalp stated above, it would be nice to disable syslog OR have it be opt-in. We've had to implement a few hacks to silence the warnings in our CI/CD pipeline.