Added drift integration #41
Conversation
| @@ -36,6 +36,12 @@ To send to [Drip][drip]: | |||
|
|
|||
| **Please note** that you need to send an "email" property to be able to get the Drip integration working. | |||
|
|
|||
| To send to Drift: | |||
|
|
|||
| - set `DRIFT_ORG_ID=456` (ATM only possible to find by contacting the drift support dept) | |||
| s.Attributes["forwardlyticsTimestamp"] = identification.Timestamp | ||
| payload, err := json.Marshal(s) | ||
|
|
||
| err = d.api.request("POST", "identify", payload) |
There was a problem hiding this comment.
If there is an error, please log an error using logrus. I just improved it for the other integrations. See 798c7c1
Can you do something similar? It adds more context to the Bugsnag report and is super useful.
There was a problem hiding this comment.
Sure. More logging is a good idea in general. It's a bit hard to know what went wrong per now because of the logging that is a bit minimal. I'll add it everywhere you mentioned as a start.
| if err != nil { | ||
| logrus.WithField("err", err).Fatal("Error marshalling drift event to json") | ||
| } | ||
| err = d.api.request("POST", "track", payload) |
| e.CreatedAt = event.Timestamp | ||
| payload, err := json.Marshal(e) | ||
| if err != nil { | ||
| logrus.WithField("err", err).Fatal("Error marshalling drift event to json") |
There was a problem hiding this comment.
Use the logging I just mentioned here too. It's not your fault, I did not do a good job originally and when I faced a bug with the Drip integration (not your fault, I was not sending an email for a specific event), I had to add more context.
| if resp.StatusCode != http.StatusOK { | ||
| body, err := ioutil.ReadAll(resp.Body) | ||
| if err != nil { | ||
| logrus.WithField("err", err).Error("Error reading body in Drift response") |
| resp, err := client.Do(req) | ||
| defer resp.Body.Close() | ||
| if err != nil { | ||
| logrus.WithField("err", err).Error("Error sending request to Drift api") |
| logrus.WithField("err", err).Error("Error reading body in Drift response") | ||
| return err | ||
| } | ||
| logrus.WithFields( |
|
|
||
| err = d.api.request("POST", "identify", payload) | ||
| if err != nil { | ||
| logrus.WithError(err).WithField("identify", identification).WithField("payload", payload) |
|
I think this is finished, but I force pushed some changes to my branch, so I might have broken some of the review feedback. I'll try to avoid this on the next one. Anything else that needs fixing, or are we ready to merge this? |
Feedback appreciated :)