From 5c1c520f97db5a80aead25922154c90c3bbe6a45 Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Tue, 16 Oct 2018 15:32:19 -0700 Subject: [PATCH] Accept a call scoped context throughout the logging functions The impetus of this change was to provide a simple mechanism for setting the Person for a particular error message without setting it globally. The globally SetPerson mechanism works for certain use cases, but for many others you want to scope the Person to a smaller surface area. This change introduces a context.Context to most (all?) of the logging methods. For now this only is used to possibly pass a Person to the particular call by using `NewPersonContext`. I decided to use a context rather than accepting a person struct so that we could possibly use this context for cancellation and timeouts in the future, or other scoped values. I added a usage to the example. Most of the tests hit these code paths, but some additional tests might be welcome. --- client.go | 90 +++++++++++++++++++++++++++++++++++++++---------- example/main.go | 11 +++++- rollbar.go | 52 +++++++++++++++++++++++++--- rollbar_test.go | 5 +-- transforms.go | 16 +++++---- 5 files changed, 144 insertions(+), 30 deletions(-) diff --git a/client.go b/client.go index 2c2a785..4bd3359 100644 --- a/client.go +++ b/client.go @@ -111,17 +111,17 @@ func (c *Client) SetCustom(custom map[string]interface{}) { // any subsequent errors or messages. Only id is required to be // non-empty. func (c *Client) SetPerson(id, username, email string) { - c.configuration.person = person{ - id: id, - username: username, - email: email, + c.configuration.person = Person{ + Id: id, + Username: username, + Email: email, } } // ClearPerson clears any previously set person information. See `SetPerson` for more // information. func (c *Client) ClearPerson() { - c.configuration.person = person{} + c.configuration.person = Person{} } // SetFingerprint sets whether or not to use a custom client-side fingerprint. The default value is @@ -279,6 +279,12 @@ func (c *Client) ErrorWithExtras(level string, err error, extras map[string]inte c.ErrorWithStackSkipWithExtras(level, err, 1, extras) } +// ErrorWithExtrasAndContext sends an error to Rollbar with the given severity +// level with extra custom data, within the given context. +func (c *Client) ErrorWithExtrasAndContext(ctx context.Context, level string, err error, extras map[string]interface{}) { + c.ErrorWithStackSkipWithExtrasAndContext(ctx, level, err, 1, extras) +} + // RequestError sends an error to Rollbar with the given severity level // and request-specific information. func (c *Client) RequestError(level string, r *http.Request, err error) { @@ -291,6 +297,13 @@ func (c *Client) RequestErrorWithExtras(level string, r *http.Request, err error c.RequestErrorWithStackSkipWithExtras(level, r, err, 1, extras) } +// RequestErrorWithExtrasAndContext sends an error to Rollbar with the given +// severity level and request-specific information with extra custom data, within the given +// context. +func (c *Client) RequestErrorWithExtrasAndContext(ctx context.Context, level string, r *http.Request, err error, extras map[string]interface{}) { + c.RequestErrorWithStackSkipWithExtrasAndContext(ctx, level, r, err, 1, extras) +} + // ErrorWithStackSkip sends an error to Rollbar with the given severity // level and a given number of stack trace frames skipped. func (c *Client) ErrorWithStackSkip(level string, err error, skip int) { @@ -301,10 +314,17 @@ func (c *Client) ErrorWithStackSkip(level string, err error, skip int) { // severity level and a given number of stack trace frames skipped with // extra custom data. func (c *Client) ErrorWithStackSkipWithExtras(level string, err error, skip int, extras map[string]interface{}) { + c.ErrorWithStackSkipWithExtrasAndContext(context.TODO(), level, err, skip, extras) +} + +// ErrorWithStackSkipWithExtrasAndContext sends an error to Rollbar with the given +// severity level and a given number of stack trace frames skipped with +// extra custom data, within the given context. +func (c *Client) ErrorWithStackSkipWithExtrasAndContext(ctx context.Context, level string, err error, skip int, extras map[string]interface{}) { if !c.configuration.enabled { return } - body := c.buildBody(level, err.Error(), extras) + body := c.buildBody(ctx, level, err.Error(), extras) addErrorToBody(c.configuration, body, err, skip) c.push(body) } @@ -321,10 +341,18 @@ func (c *Client) RequestErrorWithStackSkip(level string, r *http.Request, err er // skipped, in addition to extra request-specific information and extra // custom data. func (c *Client) RequestErrorWithStackSkipWithExtras(level string, r *http.Request, err error, skip int, extras map[string]interface{}) { + c.RequestErrorWithStackSkipWithExtrasAndContext(context.TODO(), level, r, err, skip, extras) +} + +// RequestErrorWithStackSkipWithExtrasAndContext sends an error to Rollbar with +// the given severity level and a given number of stack trace frames +// skipped, in addition to extra request-specific information and extra +// custom data, within the given context. +func (c *Client) RequestErrorWithStackSkipWithExtrasAndContext(ctx context.Context, level string, r *http.Request, err error, skip int, extras map[string]interface{}) { if !c.configuration.enabled { return } - body := c.buildBody(level, err.Error(), extras) + body := c.buildBody(ctx, level, err.Error(), extras) data := addErrorToBody(c.configuration, body, err, skip) data["request"] = c.requestDetails(r) c.push(body) @@ -340,10 +368,16 @@ func (c *Client) Message(level string, msg string) { // MessageWithExtras sends a message to Rollbar with the given severity // level with extra custom data. func (c *Client) MessageWithExtras(level string, msg string, extras map[string]interface{}) { + c.MessageWithExtrasAndContext(context.TODO(), level, msg, extras) +} + +// MessageWithExtrasAndContext sends a message to Rollbar with the given severity +// level with extra custom data, within the given context. +func (c *Client) MessageWithExtrasAndContext(ctx context.Context, level string, msg string, extras map[string]interface{}) { if !c.configuration.enabled { return } - body := c.buildBody(level, msg, extras) + body := c.buildBody(ctx, level, msg, extras) data := body["data"].(map[string]interface{}) data["body"] = messageBody(msg) c.push(body) @@ -358,10 +392,17 @@ func (c *Client) RequestMessage(level string, r *http.Request, msg string) { // RequestMessageWithExtras sends a message to Rollbar with the given // severity level and request-specific information with extra custom data. func (c *Client) RequestMessageWithExtras(level string, r *http.Request, msg string, extras map[string]interface{}) { + c.RequestMessageWithExtrasAndContext(context.TODO(), level, r, msg, extras) +} + +// RequestMessageWithExtrasAndContext sends a message to Rollbar with the given +// severity level and request-specific information with extra custom data, within the given +// context. +func (c *Client) RequestMessageWithExtrasAndContext(ctx context.Context, level string, r *http.Request, msg string, extras map[string]interface{}) { if !c.configuration.enabled { return } - body := c.buildBody(level, msg, extras) + body := c.buildBody(ctx, level, msg, extras) data := body["data"].(map[string]interface{}) data["body"] = messageBody(msg) data["request"] = c.requestDetails(r) @@ -441,8 +482,8 @@ func (c *Client) Close() error { return c.Transport.Close() } -func (c *Client) buildBody(level, title string, extras map[string]interface{}) map[string]interface{} { - return buildBody(c.configuration, level, title, extras) +func (c *Client) buildBody(ctx context.Context, level, title string, extras map[string]interface{}) map[string]interface{} { + return buildBody(ctx, c.configuration, level, title, extras) } func (c *Client) requestDetails(r *http.Request) map[string]interface{} { @@ -455,10 +496,25 @@ func (c *Client) push(body map[string]interface{}) error { return c.Transport.Send(body) } -type person struct { - id string - username string - email string +type Person struct { + Id string + Username string + Email string +} + +type pkey int + +var personKey pkey + +// NewPersonContext returns a new Context that carries the person as a value. +func NewPersonContext(ctx context.Context, p *Person) context.Context { + return context.WithValue(ctx, personKey, p) +} + +// PersonFromContext returns the Person value stored in ctx, if any. +func PersonFromContext(ctx context.Context) (*Person, bool) { + p, ok := ctx.Value(personKey).(*Person) + return p, ok } type captureIp int @@ -487,7 +543,7 @@ type configuration struct { scrubFields *regexp.Regexp checkIgnore func(string) bool transform func(map[string]interface{}) - person person + person Person captureIp captureIp } @@ -510,7 +566,7 @@ func createConfiguration(token, environment, codeVersion, serverHost, serverRoot fingerprint: false, checkIgnore: func(_s string) bool { return false }, transform: func(_d map[string]interface{}) {}, - person: person{}, + person: Person{}, captureIp: CaptureIpFull, } } diff --git a/example/main.go b/example/main.go index 157bc3b..9b91a54 100644 --- a/example/main.go +++ b/example/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "fmt" "github.com/rollbar/rollbar-go" @@ -22,7 +23,10 @@ func helloJson(w http.ResponseWriter, r *http.Request) { fmt.Println("key:", k) fmt.Println("val:", v) } - rollbar.Info(r, "Example message json") + // This person context will override the global value set with SetPerson in main for the + // specific call that it is sent with. + ctx := rollbar.NewPersonContext(context.TODO(), &rollbar.Person{Id: "42", Username: "Frank", Email: ""}) + rollbar.Info(r, "Example message json", ctx) fmt.Fprintf(w, "Hello world!") } @@ -34,6 +38,7 @@ func helloForm(w http.ResponseWriter, r *http.Request) { fmt.Println("key:", k) fmt.Println("val:", strings.Join(v, " ")) } + // Without a context this will include the person from the global SetPerson call rollbar.Info(r, "Example message form") fmt.Fprintf(w, "Hello world!") } @@ -42,11 +47,15 @@ func helloForm(w http.ResponseWriter, r *http.Request) { // In another: // curl -X POST -H "Content-Type: application/x-www-form-urlencoded" \ // http://localhost:9090/form -d "password=foobar&fuzz=buzz" +// Or: +// curl -X POST -H "Content-Type: application/json" \ +// http://localhost:9090/json -d '{"password":"foobar","fuzz":"buzz"}' func main() { var token = os.Getenv("TOKEN") rollbar.SetToken(token) rollbar.SetEnvironment("test") rollbar.SetCaptureIp(rollbar.CaptureIpAnonymize) + rollbar.SetPerson("88", "Steve", "") http.HandleFunc("/json", helloJson) http.HandleFunc("/form", helloForm) err := http.ListenAndServe(":9090", nil) diff --git a/rollbar.go b/rollbar.go index f52efc0..7c4c0e6 100644 --- a/rollbar.go +++ b/rollbar.go @@ -1,6 +1,7 @@ package rollbar import ( + "context" "net/http" "os" "regexp" @@ -338,6 +339,7 @@ func Log(level string, interfaces ...interface{}) { skipSet := false var extras map[string]interface{} var msg string + ctx := context.TODO() for _, ival := range interfaces { switch val := ival.(type) { case *http.Request: @@ -351,6 +353,8 @@ func Log(level string, interfaces ...interface{}) { msg = val case map[string]interface{}: extras = val + case context.Context: + ctx = val default: rollbarError(std.Transport.(*AsyncTransport).Logger, "Unknown input type: %T", val) } @@ -360,15 +364,15 @@ func Log(level string, interfaces ...interface{}) { } if err != nil { if r == nil { - std.ErrorWithStackSkipWithExtras(level, err, skip, extras) + std.ErrorWithStackSkipWithExtrasAndContext(ctx, level, err, skip, extras) } else { - std.RequestErrorWithStackSkipWithExtras(level, r, err, skip, extras) + std.RequestErrorWithStackSkipWithExtrasAndContext(ctx, level, r, err, skip, extras) } } else { if r == nil { - std.MessageWithExtras(level, msg, extras) + std.MessageWithExtrasAndContext(ctx, level, msg, extras) } else { - std.RequestMessageWithExtras(level, r, msg, extras) + std.RequestMessageWithExtrasAndContext(ctx, level, r, msg, extras) } } } @@ -391,6 +395,12 @@ func ErrorWithExtras(level string, err error, extras map[string]interface{}) { std.ErrorWithExtras(level, err, extras) } +// ErrorWithExtrasAndContext asynchronously sends an error to Rollbar with the given +// severity level with extra custom data, within the given context. +func ErrorWithExtrasAndContext(ctx context.Context, level string, err error, extras map[string]interface{}) { + std.ErrorWithExtrasAndContext(ctx, level, err, extras) +} + // RequestError asynchronously sends an error to Rollbar with the given // severity level and request-specific information. func RequestError(level string, r *http.Request, err error) { @@ -403,6 +413,12 @@ func RequestErrorWithExtras(level string, r *http.Request, err error, extras map std.RequestErrorWithExtras(level, r, err, extras) } +// RequestErrorWithExtrasAndContext asynchronously sends an error to Rollbar with the given +// severity level and request-specific information with extra custom data. +func RequestErrorWithExtrasAndContext(ctx context.Context, level string, r *http.Request, err error, extras map[string]interface{}) { + std.RequestErrorWithExtrasAndContext(ctx, level, r, err, extras) +} + // ErrorWithStackSkip asynchronously sends an error to Rollbar with the given // severity level and a given number of stack trace frames skipped. func ErrorWithStackSkip(level string, err error, skip int) { @@ -415,6 +431,14 @@ func ErrorWithStackSkipWithExtras(level string, err error, skip int, extras map[ std.ErrorWithStackSkipWithExtras(level, err, skip, extras) } +// ErrorWithStackSkipWithExtrasAndContext asynchronously sends an error to Rollbar with the given +// severity level and a given number of stack trace frames skipped with extra custom data, within +// the given context. +func ErrorWithStackSkipWithExtrasAndContext(ctx context.Context, level string, err error, skip int, extras map[string]interface{}) { + std.ErrorWithStackSkipWithExtrasAndContext(ctx, level, err, skip, extras) +} + +// RequestErrorWithStackSkip asynchronously sends an error to Rollbar with the // RequestErrorWithStackSkip asynchronously sends an error to Rollbar with the // given severity level and a given number of stack trace frames skipped, in // addition to extra request-specific information. @@ -429,6 +453,13 @@ func RequestErrorWithStackSkipWithExtras(level string, r *http.Request, err erro std.RequestErrorWithStackSkipWithExtras(level, r, err, skip, extras) } +// RequestErrorWithStackSkipWithExtrasAndContext asynchronously sends an error to Rollbar +// with the given severity level and a given number of stack trace frames skipped, +// in addition to extra request-specific information and extra custom data, within the given context. +func RequestErrorWithStackSkipWithExtrasAndContext(ctx context.Context, level string, r *http.Request, err error, skip int, extras map[string]interface{}) { + std.RequestErrorWithStackSkipWithExtrasAndContext(ctx, level, r, err, skip, extras) +} + // -- Message reporting // Message asynchronously sends a message to Rollbar with the given severity @@ -443,6 +474,12 @@ func MessageWithExtras(level string, msg string, extras map[string]interface{}) std.MessageWithExtras(level, msg, extras) } +// MessageWithExtrasAndContext asynchronously sends a message to Rollbar with the given severity +// level with extra custom data, within the given context. Rollbar request is asynchronous. +func MessageWithExtrasAndContext(ctx context.Context, level string, msg string, extras map[string]interface{}) { + std.MessageWithExtrasAndContext(ctx, level, msg, extras) +} + // RequestMessage asynchronously sends a message to Rollbar with the given // severity level and request-specific information. func RequestMessage(level string, r *http.Request, msg string) { @@ -456,6 +493,13 @@ func RequestMessageWithExtras(level string, r *http.Request, msg string, extras std.RequestMessageWithExtras(level, r, msg, extras) } +// RequestMessageWithExtrasAndContext asynchronously sends a message to Rollbar with the given severity +// level with extra custom data in addition to extra request-specific information, within the given +// context. Rollbar request is asynchronous. +func RequestMessageWithExtrasAndContext(ctx context.Context, level string, r *http.Request, msg string, extras map[string]interface{}) { + std.RequestMessageWithExtrasAndContext(ctx, level, r, msg, extras) +} + // Wait will block until the queue of errors / messages is empty. func Wait() { std.Wait() diff --git a/rollbar_test.go b/rollbar_test.go index 2ce1460..98e2fbf 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -1,6 +1,7 @@ package rollbar import ( + "context" "errors" "fmt" "net/http" @@ -153,7 +154,7 @@ func TestBuildBody(t *testing.T) { "EXTRA_CUSTOM_KEY": "EXTRA_CUSTOM_VALUE", "OVERRIDDEN_CUSTOM_KEY": "EXTRA", } - body := interface{}(std).(*Client).buildBody(ERR, "test error", extraCustom) + body := interface{}(std).(*Client).buildBody(context.TODO(), ERR, "test error", extraCustom) if body["data"] == nil { t.Error("body should have data") @@ -182,7 +183,7 @@ func TestBuildBodyNoBaseCustom(t *testing.T) { "EXTRA_CUSTOM_KEY": "EXTRA_CUSTOM_VALUE", "OVERRIDDEN_CUSTOM_KEY": "EXTRA", } - body := interface{}(std).(*Client).buildBody(ERR, "test error", extraCustom) + body := interface{}(std).(*Client).buildBody(context.TODO(), ERR, "test error", extraCustom) if body["data"] == nil { t.Error("body should have data") diff --git a/transforms.go b/transforms.go index ef039d9..3bd20dc 100644 --- a/transforms.go +++ b/transforms.go @@ -1,6 +1,7 @@ package rollbar import ( + "context" "fmt" "hash/adler32" "net/http" @@ -13,7 +14,7 @@ import ( // Build the main JSON structure that will be sent to Rollbar with the // appropriate metadata. -func buildBody(configuration configuration, level, title string, extras map[string]interface{}) map[string]interface{} { +func buildBody(ctx context.Context, configuration configuration, level, title string, extras map[string]interface{}) map[string]interface{} { timestamp := time.Now().Unix() data := map[string]interface{}{ @@ -39,12 +40,15 @@ func buildBody(configuration configuration, level, title string, extras map[stri data["custom"] = custom } - person := configuration.person - if person.id != "" { + person, ok := PersonFromContext(ctx) + if !ok { + person = &configuration.person + } + if person.Id != "" { data["person"] = map[string]string{ - "id": person.id, - "username": person.username, - "email": person.email, + "id": person.Id, + "username": person.Username, + "email": person.Email, } }