From 7d3ddc68a3c3aa2184059ed57648b0d1e18351c0 Mon Sep 17 00:00:00 2001 From: Ernesto Alejo Date: Wed, 6 Sep 2017 19:34:58 +0200 Subject: [PATCH 01/11] Split terminal check to add build tags to support App Engine. --- terminal_check_appengine.go | 11 +++++++++++ terminal_check_notappengine.go | 19 +++++++++++++++++++ text_formatter.go | 15 +-------------- 3 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 terminal_check_appengine.go create mode 100644 terminal_check_notappengine.go diff --git a/terminal_check_appengine.go b/terminal_check_appengine.go new file mode 100644 index 0000000..2403de9 --- /dev/null +++ b/terminal_check_appengine.go @@ -0,0 +1,11 @@ +// +build appengine + +package logrus + +import ( + "io" +) + +func checkIfTerminal(w io.Writer) bool { + return true +} diff --git a/terminal_check_notappengine.go b/terminal_check_notappengine.go new file mode 100644 index 0000000..116bcb4 --- /dev/null +++ b/terminal_check_notappengine.go @@ -0,0 +1,19 @@ +// +build !appengine + +package logrus + +import ( + "io" + "os" + + "golang.org/x/crypto/ssh/terminal" +) + +func checkIfTerminal(w io.Writer) bool { + switch v := w.(type) { + case *os.File: + return terminal.IsTerminal(int(v.Fd())) + default: + return false + } +} diff --git a/text_formatter.go b/text_formatter.go index be412aa..61b21ca 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -3,14 +3,10 @@ package logrus import ( "bytes" "fmt" - "io" - "os" "sort" "strings" "sync" "time" - - "golang.org/x/crypto/ssh/terminal" ) const ( @@ -65,16 +61,7 @@ type TextFormatter struct { func (f *TextFormatter) init(entry *Entry) { if entry.Logger != nil { - f.isTerminal = f.checkIfTerminal(entry.Logger.Out) - } -} - -func (f *TextFormatter) checkIfTerminal(w io.Writer) bool { - switch v := w.(type) { - case *os.File: - return terminal.IsTerminal(int(v.Fd())) - default: - return false + f.isTerminal = checkIfTerminal(entry.Logger.Out) } } From d682213848ed68c0a260ca37d6dd5ace8423f5ba Mon Sep 17 00:00:00 2001 From: Simon Eskildsen Date: Tue, 5 Dec 2017 15:32:29 -0500 Subject: [PATCH 02/11] changelog: 1.0.4 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8236d8b..cc58f64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 1.0.4 + +* Fix race when adding hooks (#612) +* Fix terminal check in AppEngine (#635) + # 1.0.3 * Replace example files with testable examples From 977e03308a73cbc954765de9e2efce93f1416294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Linhares?= Date: Mon, 22 Jan 2018 10:35:26 -0500 Subject: [PATCH 03/11] Fix deadlock on panics at Entry.log When calling Entry.log a panic inside some of the locking blocks could cause the whole logger to deadlock. One of the ways this could happen is for a hook to cause a panic, when this happens the lock is never unlocked and the library deadlocks, causing the code that is calling it to deadlock as well. This changes how locking happens with unlocks at defer blocks so even if a panic happens somewhere along the log call the library will still unlock and continue to function. --- entry.go | 49 ++++++++++++++++++++++++++++--------------------- entry_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/entry.go b/entry.go index 1fad45e..df6f92d 100644 --- a/entry.go +++ b/entry.go @@ -94,32 +94,16 @@ func (entry Entry) log(level Level, msg string) { entry.Level = level entry.Message = msg - entry.Logger.mu.Lock() - err := entry.Logger.Hooks.Fire(level, &entry) - entry.Logger.mu.Unlock() - if err != nil { - entry.Logger.mu.Lock() - fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err) - entry.Logger.mu.Unlock() - } + entry.fireHooks() + buffer = bufferPool.Get().(*bytes.Buffer) buffer.Reset() defer bufferPool.Put(buffer) entry.Buffer = buffer - serialized, err := entry.Logger.Formatter.Format(&entry) + + entry.write() + entry.Buffer = nil - if err != nil { - entry.Logger.mu.Lock() - fmt.Fprintf(os.Stderr, "Failed to obtain reader, %v\n", err) - entry.Logger.mu.Unlock() - } else { - entry.Logger.mu.Lock() - _, err = entry.Logger.Out.Write(serialized) - if err != nil { - fmt.Fprintf(os.Stderr, "Failed to write to log, %v\n", err) - } - entry.Logger.mu.Unlock() - } // To avoid Entry#log() returning a value that only would make sense for // panic() to use in Entry#Panic(), we avoid the allocation by checking @@ -129,6 +113,29 @@ func (entry Entry) log(level Level, msg string) { } } +func (entry *Entry) fireHooks() { + entry.Logger.mu.Lock() + defer entry.Logger.mu.Unlock() + err := entry.Logger.Hooks.Fire(entry.Level, entry) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err) + } +} + +func (entry *Entry) write() { + serialized, err := entry.Logger.Formatter.Format(entry) + entry.Logger.mu.Lock() + defer entry.Logger.mu.Unlock() + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to obtain reader, %v\n", err) + } else { + _, err = entry.Logger.Out.Write(serialized) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to write to log, %v\n", err) + } + } +} + func (entry *Entry) Debug(args ...interface{}) { if entry.Logger.level() >= DebugLevel { entry.log(DebugLevel, fmt.Sprint(args...)) diff --git a/entry_test.go b/entry_test.go index 99c3b41..a81e2b3 100644 --- a/entry_test.go +++ b/entry_test.go @@ -75,3 +75,41 @@ func TestEntryPanicf(t *testing.T) { entry := NewEntry(logger) entry.WithField("err", errBoom).Panicf("kaboom %v", true) } + +const ( + badMessage = "this is going to panic" + panicMessage = "this is broken" +) + +type panickyHook struct{} + +func (p *panickyHook) Levels() []Level { + return []Level{InfoLevel} +} + +func (p *panickyHook) Fire(entry *Entry) error { + if entry.Message == badMessage { + panic(panicMessage) + } + + return nil +} + +func TestEntryHooksPanic(t *testing.T) { + logger := New() + logger.Out = &bytes.Buffer{} + logger.Level = InfoLevel + logger.Hooks.Add(&panickyHook{}) + + defer func() { + p := recover() + assert.NotNil(t, p) + assert.Equal(t, panicMessage, p) + + entry := NewEntry(logger) + entry.Info("another message") + }() + + entry := NewEntry(logger) + entry.Info(badMessage) +} From 516f6c178d8d405bf866692998d3d5885cb855ee Mon Sep 17 00:00:00 2001 From: Joni Collinge Date: Tue, 30 Jan 2018 15:58:49 +0000 Subject: [PATCH 04/11] Add Application Insights hook to README Adds README link to Application Insights hook --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 08584b5..213e651 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,7 @@ Note: Syslog hook also support connecting to local syslog (Ex. "/dev/log" or "/v | [Airbrake](https://github.com/gemnasium/logrus-airbrake-hook) | Send errors to the Airbrake API V3. Uses the official [`gobrake`](https://github.com/airbrake/gobrake) behind the scenes. | | [Amazon Kinesis](https://github.com/evalphobia/logrus_kinesis) | Hook for logging to [Amazon Kinesis](https://aws.amazon.com/kinesis/) | | [Amqp-Hook](https://github.com/vladoatanasov/logrus_amqp) | Hook for logging to Amqp broker (Like RabbitMQ) | +| [Application Insights](https://github.com/jjcollinge/logrus-appinsights) | Hook for [Application Insights](https://azure.microsoft.com/en-us/services/application-insights/) | [AzureTableHook](https://github.com/kpfaulkner/azuretablehook/) | Hook for logging to Azure Table Storage| | [Bugsnag](https://github.com/Shopify/logrus-bugsnag/blob/master/bugsnag.go) | Send errors to the Bugsnag exception tracking service. | | [DeferPanic](https://github.com/deferpanic/dp-logrus) | Hook for logging to DeferPanic | From 0cf9f0bff936165cf3297ddc2d3d53d14a250c8f Mon Sep 17 00:00:00 2001 From: Joni Collinge Date: Tue, 30 Jan 2018 16:00:33 +0000 Subject: [PATCH 05/11] Made text consistent with other hooks --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 213e651..f986a5e 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ Note: Syslog hook also support connecting to local syslog (Ex. "/dev/log" or "/v | [Airbrake](https://github.com/gemnasium/logrus-airbrake-hook) | Send errors to the Airbrake API V3. Uses the official [`gobrake`](https://github.com/airbrake/gobrake) behind the scenes. | | [Amazon Kinesis](https://github.com/evalphobia/logrus_kinesis) | Hook for logging to [Amazon Kinesis](https://aws.amazon.com/kinesis/) | | [Amqp-Hook](https://github.com/vladoatanasov/logrus_amqp) | Hook for logging to Amqp broker (Like RabbitMQ) | -| [Application Insights](https://github.com/jjcollinge/logrus-appinsights) | Hook for [Application Insights](https://azure.microsoft.com/en-us/services/application-insights/) +| [Application Insights](https://github.com/jjcollinge/logrus-appinsights) | Hook for logging to [Application Insights](https://azure.microsoft.com/en-us/services/application-insights/) | [AzureTableHook](https://github.com/kpfaulkner/azuretablehook/) | Hook for logging to Azure Table Storage| | [Bugsnag](https://github.com/Shopify/logrus-bugsnag/blob/master/bugsnag.go) | Send errors to the Bugsnag exception tracking service. | | [DeferPanic](https://github.com/deferpanic/dp-logrus) | Hook for logging to DeferPanic | From 178041e53c9337c7c7781963f2dbb46080484237 Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Mon, 5 Feb 2018 21:59:23 +0100 Subject: [PATCH 06/11] Fix typo in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 08584b5..e5da2c2 100644 --- a/README.md +++ b/README.md @@ -220,7 +220,7 @@ Logrus comes with [built-in hooks](hooks/). Add those, or your custom hook, in ```go import ( log "github.com/sirupsen/logrus" - "gopkg.in/gemnasium/logrus-airbrake-hook.v2" // the package is named "aibrake" + "gopkg.in/gemnasium/logrus-airbrake-hook.v2" // the package is named "airbrake" logrus_syslog "github.com/sirupsen/logrus/hooks/syslog" "log/syslog" ) From be569094e9c87b6de4fa8909e1e7db1603702f9f Mon Sep 17 00:00:00 2001 From: Jay Ching Lim Date: Mon, 12 Feb 2018 17:26:48 -0500 Subject: [PATCH 07/11] Make fireHooks() method receive a copy of Entry structure to avoid race conditions Signed-off-by: Jay Ching Lim --- entry.go | 6 ++++-- hooks/test/test_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/entry.go b/entry.go index df6f92d..778f4c9 100644 --- a/entry.go +++ b/entry.go @@ -113,10 +113,12 @@ func (entry Entry) log(level Level, msg string) { } } -func (entry *Entry) fireHooks() { +// This function is not declared with a pointer value because otherwise +// race conditions will occur when using multiple goroutines +func (entry Entry) fireHooks() { entry.Logger.mu.Lock() defer entry.Logger.mu.Unlock() - err := entry.Logger.Hooks.Fire(entry.Level, entry) + err := entry.Logger.Hooks.Fire(entry.Level, &entry) if err != nil { fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err) } diff --git a/hooks/test/test_test.go b/hooks/test/test_test.go index 3f55cfe..dea768e 100644 --- a/hooks/test/test_test.go +++ b/hooks/test/test_test.go @@ -1,6 +1,7 @@ package test import ( + "sync" "testing" "github.com/sirupsen/logrus" @@ -8,7 +9,6 @@ import ( ) func TestAllHooks(t *testing.T) { - assert := assert.New(t) logger, hook := NewNullLogger() @@ -35,5 +35,27 @@ func TestAllHooks(t *testing.T) { assert.Equal(logrus.ErrorLevel, hook.LastEntry().Level) assert.Equal("Hello error", hook.LastEntry().Message) assert.Equal(1, len(hook.Entries)) - +} + +func TestLoggingWithHooksRace(t *testing.T) { + assert := assert.New(t) + logger, hook := NewNullLogger() + + var wg sync.WaitGroup + wg.Add(100) + + for i := 0; i < 100; i++ { + go func() { + logger.Info("info") + wg.Done() + }() + } + + assert.Equal(logrus.InfoLevel, hook.LastEntry().Level) + assert.Equal("info", hook.LastEntry().Message) + + wg.Wait() + + entries := hook.AllEntries() + assert.Equal(100, len(entries)) } From f4118d2ead4e96d931fd6751a831165147c99889 Mon Sep 17 00:00:00 2001 From: Andrew Rezcov Date: Fri, 16 Feb 2018 16:50:54 +0300 Subject: [PATCH 08/11] reamde: add logrus-clickhouse-hook --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index bc3f9bc..be18d2f 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,7 @@ Note: Syslog hook also support connecting to local syslog (Ex. "/dev/log" or "/v | [Application Insights](https://github.com/jjcollinge/logrus-appinsights) | Hook for logging to [Application Insights](https://azure.microsoft.com/en-us/services/application-insights/) | [AzureTableHook](https://github.com/kpfaulkner/azuretablehook/) | Hook for logging to Azure Table Storage| | [Bugsnag](https://github.com/Shopify/logrus-bugsnag/blob/master/bugsnag.go) | Send errors to the Bugsnag exception tracking service. | +| [ClickHouse](https://github.com/oxgrouby/logrus-clickhouse-hook) | Send logs to [ClickHouse](https://clickhouse.yandex/) | | [DeferPanic](https://github.com/deferpanic/dp-logrus) | Hook for logging to DeferPanic | | [Discordrus](https://github.com/kz/discordrus) | Hook for logging to [Discord](https://discordapp.com/) | | [ElasticSearch](https://github.com/sohlich/elogrus) | Hook for logging to ElasticSearch| From c840e59446e71aa3fec56f9317fd2570cc8b3f5f Mon Sep 17 00:00:00 2001 From: Grace Noah Date: Fri, 23 Feb 2018 21:35:25 +0000 Subject: [PATCH 09/11] add gopherjs build tag it behaves the same way as the appengine tag fix #716 --- terminal_bsd.go | 2 +- terminal_check_appengine.go | 2 +- terminal_check_notappengine.go | 2 +- terminal_linux.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/terminal_bsd.go b/terminal_bsd.go index d7b3893..4880d13 100644 --- a/terminal_bsd.go +++ b/terminal_bsd.go @@ -1,5 +1,5 @@ // +build darwin freebsd openbsd netbsd dragonfly -// +build !appengine +// +build !appengine,!gopherjs package logrus diff --git a/terminal_check_appengine.go b/terminal_check_appengine.go index 2403de9..3de08e8 100644 --- a/terminal_check_appengine.go +++ b/terminal_check_appengine.go @@ -1,4 +1,4 @@ -// +build appengine +// +build appengine gopherjs package logrus diff --git a/terminal_check_notappengine.go b/terminal_check_notappengine.go index 116bcb4..067047a 100644 --- a/terminal_check_notappengine.go +++ b/terminal_check_notappengine.go @@ -1,4 +1,4 @@ -// +build !appengine +// +build !appengine,!gopherjs package logrus diff --git a/terminal_linux.go b/terminal_linux.go index 88d7298..f29a009 100644 --- a/terminal_linux.go +++ b/terminal_linux.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build !appengine +// +build !appengine,!gopherjs package logrus From 91b159d34d862d10fa1e5d3ffb251656af545580 Mon Sep 17 00:00:00 2001 From: Dylan Meissner Date: Sun, 11 Feb 2018 07:37:38 -0800 Subject: [PATCH 10/11] Add Kafka REST Proxy hook to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index bc3f9bc..f77819b 100644 --- a/README.md +++ b/README.md @@ -263,6 +263,7 @@ Note: Syslog hook also support connecting to local syslog (Ex. "/dev/log" or "/v | [Influxus](http://github.com/vlad-doru/influxus) | Hook for concurrently logging to [InfluxDB](http://influxdata.com/) | | [Journalhook](https://github.com/wercker/journalhook) | Hook for logging to `systemd-journald` | | [KafkaLogrus](https://github.com/tracer0tong/kafkalogrus) | Hook for logging to Kafka | +| [Kafka REST Proxy](https://github.com/Nordstrom/logrus-kafka-rest-proxy) | Hook for logging to [Kafka REST Proxy](https://docs.confluent.io/current/kafka-rest/docs) | | [LFShook](https://github.com/rifflock/lfshook) | Hook for logging to the local filesystem | | [Logbeat](https://github.com/macandmia/logbeat) | Hook for logging to [Opbeat](https://opbeat.com/) | | [Logentries](https://github.com/jcftang/logentriesrus) | Hook for logging to [Logentries](https://logentries.com/) | From c155da19408a8799da419ed3eeb0cb5db0ad5dbc Mon Sep 17 00:00:00 2001 From: Simon Eskildsen Date: Sun, 11 Mar 2018 18:51:37 -0400 Subject: [PATCH 11/11] changelog: add 1.0.5 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc58f64..1bd1deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 1.0.5 + +* Fix hooks race (#707) +* Fix panic deadlock (#695) + # 1.0.4 * Fix race when adding hooks (#612)