-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: set telemetry events #1421
Conversation
internal/server/errors.go
Outdated
@@ -66,6 +66,8 @@ func sendAPIError(c *gin.Context, err error) { | |||
logging.WrappedSugarLogger(c).Errorw(err.Error(), "statusCode", resp.Code) | |||
} | |||
|
|||
a.t.Event(c, "error", Properties{"code": resp.Code}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this have context of what errored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, maybe just the endpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need this for telemetry, since it will error for every single (expected) error as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing this to only record for status code >= 500
internal/server/errors.go
Outdated
@@ -66,6 +66,8 @@ func sendAPIError(c *gin.Context, err error) { | |||
logging.WrappedSugarLogger(c).Errorw(err.Error(), "statusCode", resp.Code) | |||
} | |||
|
|||
a.t.Event(c, "error", Properties{"code": resp.Code}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, maybe just the endpoint
if err != nil { | ||
return err | ||
switch track := track.(type) { | ||
case analytics.Track: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do an if statement here and get rid of the duplication
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't. it's a reflection thing where track
assumes the type of the case
Summary
Checklist
Related Issues
Resolves #1285