From 7d3ddc68a3c3aa2184059ed57648b0d1e18351c0 Mon Sep 17 00:00:00 2001 From: Ernesto Alejo Date: Wed, 6 Sep 2017 19:34:58 +0200 Subject: [PATCH 01/13] 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 4844e5856d75e79287a64073cee56c93e8a3202b Mon Sep 17 00:00:00 2001 From: Marc CARRE Date: Tue, 3 Oct 2017 15:08:46 +0100 Subject: [PATCH 02/13] Add promrus to list of hooks. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5f656c3..5394166 100644 --- a/README.md +++ b/README.md @@ -275,6 +275,7 @@ Note: Syslog hook also support connecting to local syslog (Ex. "/dev/log" or "/v | [Octokit](https://github.com/dorajistyle/logrus-octokit-hook) | Hook for logging to github via octokit | | [Papertrail](https://github.com/polds/logrus-papertrail-hook) | Send errors to the [Papertrail](https://papertrailapp.com) hosted logging service via UDP. | | [PostgreSQL](https://github.com/gemnasium/logrus-postgresql-hook) | Send logs to [PostgreSQL](http://postgresql.org) | +| [Promrus](https://github.com/weaveworks/promrus) | Expose number of log messages as [Prometheus](https://prometheus.io/) metrics | | [Pushover](https://github.com/toorop/logrus_pushover) | Send error via [Pushover](https://pushover.net) | | [Raygun](https://github.com/squirkle/logrus-raygun-hook) | Hook for logging to [Raygun.io](http://raygun.io/) | | [Redis-Hook](https://github.com/rogierlommers/logrus-redis-hook) | Hook for logging to a ELK stack (through Redis) | From 1858a8574d0de3bd58787cc312c5a0b72a4a825f Mon Sep 17 00:00:00 2001 From: Eric Marden Date: Thu, 26 Oct 2017 13:58:09 -0500 Subject: [PATCH 03/13] Adds `logbeat` hook to README Adds link to new hook for sending logrus entries to Opbeat service. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5f656c3..dd9c396 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 | [Journalhook](https://github.com/wercker/journalhook) | Hook for logging to `systemd-journald` | | [KafkaLogrus](https://github.com/tracer0tong/kafkalogrus) | Hook for logging to Kafka | | [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/) | | [Logentrus](https://github.com/puddingfactory/logentrus) | Hook for logging to [Logentries](https://logentries.com/) | | [Logmatic.io](https://github.com/logmatic/logmatic-go) | Hook for logging to [Logmatic.io](http://logmatic.io/) | From 9700beb9b65fd001b5c597e6f5693f07ce37b16a Mon Sep 17 00:00:00 2001 From: Felix Glaser Date: Wed, 1 Nov 2017 14:11:54 -0400 Subject: [PATCH 04/13] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5f656c3..c822248 100644 --- a/README.md +++ b/README.md @@ -374,7 +374,7 @@ The built-in logging formatters are: Third party logging formatters: -* [`FluentdFormatter`](https://github.com/joonix/log). Formats entries that can by parsed by Kubernetes and Google Container Engine. +* [`FluentdFormatter`](https://github.com/joonix/log). Formats entries that can be parsed by Kubernetes and Google Container Engine. * [`logstash`](https://github.com/bshuster-repo/logrus-logstash-hook). Logs fields as [Logstash](http://logstash.net) Events. * [`prefixed`](https://github.com/x-cray/logrus-prefixed-formatter). Displays log entry source along with alternative layout. * [`zalgo`](https://github.com/aybabtme/logzalgo). Invoking the P͉̫o̳̼̊w̖͈̰͎e̬͔̭͂r͚̼̹̲ ̫͓͉̳͈ō̠͕͖̚f̝͍̠ ͕̲̞͖͑Z̖̫̤̫ͪa͉̬͈̗l͖͎g̳̥o̰̥̅!̣͔̲̻͊̄ ̙̘̦̹̦. From 73a13423862b4f01518dd540a89d494720d39534 Mon Sep 17 00:00:00 2001 From: Yen-Cheng Chou Date: Tue, 14 Nov 2017 16:58:00 -0500 Subject: [PATCH 05/13] Fix typo in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5f656c3..c822248 100644 --- a/README.md +++ b/README.md @@ -374,7 +374,7 @@ The built-in logging formatters are: Third party logging formatters: -* [`FluentdFormatter`](https://github.com/joonix/log). Formats entries that can by parsed by Kubernetes and Google Container Engine. +* [`FluentdFormatter`](https://github.com/joonix/log). Formats entries that can be parsed by Kubernetes and Google Container Engine. * [`logstash`](https://github.com/bshuster-repo/logrus-logstash-hook). Logs fields as [Logstash](http://logstash.net) Events. * [`prefixed`](https://github.com/x-cray/logrus-prefixed-formatter). Displays log entry source along with alternative layout. * [`zalgo`](https://github.com/aybabtme/logzalgo). Invoking the P͉̫o̳̼̊w̖͈̰͎e̬͔̭͂r͚̼̹̲ ̫͓͉̳͈ō̠͕͖̚f̝͍̠ ͕̲̞͖͑Z̖̫̤̫ͪa͉̬͈̗l͖͎g̳̥o̰̥̅!̣͔̲̻͊̄ ̙̘̦̹̦. From d682213848ed68c0a260ca37d6dd5ace8423f5ba Mon Sep 17 00:00:00 2001 From: Simon Eskildsen Date: Tue, 5 Dec 2017 15:32:29 -0500 Subject: [PATCH 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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 | 24 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 3 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 c840e59446e71aa3fec56f9317fd2570cc8b3f5f Mon Sep 17 00:00:00 2001 From: Grace Noah Date: Fri, 23 Feb 2018 21:35:25 +0000 Subject: [PATCH 12/13] 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 13/13] 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/) |