Skip to content

Conversation

@ajvb
Copy link
Contributor

@ajvb ajvb commented Feb 8, 2019

Adds optional statsd metric collection, with a generic http timer, a get reputation timer, and a timer for all redis commands.

@ajvb ajvb requested a review from ameihm0912 February 8, 2019 23:01
AWS bool
}
VersionResponse string
Statsd struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this gofmt here is off?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but it does look weird. It's just being aligned with the VersionResponse above it.

WriteTimeout: time.Millisecond * time.Duration(cfg.Redis.WriteTimeout),
DialTimeout: time.Millisecond * time.Duration(cfg.Redis.DialTimeout),
})
ret.master.WrapProcess(instrumentRedisCmd)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the performance impact of wrapping every redis command with a statsd call fairly negligible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@ajvb ajvb merged commit 177f71d into master Feb 11, 2019
@ajvb ajvb deleted the ajvb/statsd branch February 11, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants