no more logger on dev/urandom#784
Closed
jessfraz wants to merge 1 commit intodistribution:masterfrom
jessfraz:omfg-dev-urandom-i-hate-u
Closed
no more logger on dev/urandom#784jessfraz wants to merge 1 commit intodistribution:masterfrom jessfraz:omfg-dev-urandom-i-hate-u
jessfraz wants to merge 1 commit intodistribution:masterfrom
jessfraz:omfg-dev-urandom-i-hate-u
Conversation
i never want to see this error again .... or hear the work entropy or touch a computer k thanks xoxo, Jess Signed-off-by: Jessica Frazelle <acidburn@docker.com>
Contributor
|
@jfrazelle @icecrime /cc @dmcgowan @stevvooe I totally understand that we need the CI to be green, and I'm fine merging Jess's PR if we're fine reverting the ability to log this error later. Otherwise I'm -1 on it because this is an issue we have to figure out and burying the symptoms will not help. I would prefer this solution: diff --git a/context/context.go b/context/context.go
index 7a3a70e..d9efed8 100644
--- a/context/context.go
+++ b/context/context.go
@@ -27,13 +27,15 @@ func (ic *instanceContext) Value(key interface{}) interface{} {
var background = &instanceContext{
Context: context.Background(),
- id: uuid.Generate().String(),
}
// Background returns a non-nil, empty Context. The background context
// provides a single key, "instance.id" that is globally unique to the
// process.
func Background() Context {
+ if len(background.id) == 0 {
+ background.id = uuid.Generate().String()
+ }
return background
}
diff --git a/context/logger.go b/context/logger.go
index b0f0c50..78e4212 100644
--- a/context/logger.go
+++ b/context/logger.go
@@ -3,8 +3,6 @@ package context
import (
"fmt"
- "github.com/docker/distribution/uuid"
-
"github.com/Sirupsen/logrus"
)
@@ -101,8 +99,3 @@ func getLogrusLogger(ctx Context, keys ...interface{}) *logrus.Entry {
return logger.WithFields(fields)
}
-
-func init() {
- // inject a logger into the uuid library.
- uuid.Loggerf = GetLogger(Background()).Warnf
-}Basically the uuid package is currently called even before all the init() functions are called so there is no way we can override Loggerf. With the patch above, it should be possible to override And if we really want to play it safe with the initialization orders, we could make it not log by default: diff --git a/uuid/uuid.go b/uuid/uuid.go
index 02de33f..c9556ee 100644
--- a/uuid/uuid.go
+++ b/uuid/uuid.go
@@ -8,7 +8,6 @@ import (
"crypto/rand"
"fmt"
"io"
- "log"
"os"
"syscall"
"time"
@@ -30,7 +29,7 @@ var (
// Loggerf can be used to override the default logging destination. Such
// log messages in this library should be logged at warning or higher.
- Loggerf = log.Printf
+ Loggerf func(string, ...interface{})
)
// UUID represents a UUID value. UUIDs can be compared and set to other values
@@ -66,7 +65,9 @@ func Generate() (u UUID) {
if retryOnError(err) && retries < maxretries {
count += n
retries++
- Loggerf("error generating version 4 uuid, retrying: %v", err)
+ if Loggerf != nil {
+ Loggerf("error generating version 4 uuid, retrying: %v", err)
+ }
continue
} |
Collaborator
|
Closing for #792. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
i never want to see this error again
.... or hear the work entropy
or touch a computer
k thanks
xoxo,
Jess
Signed-off-by: Jessica Frazelle acidburn@docker.com