-
Notifications
You must be signed in to change notification settings - Fork 805
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
show timers distribution on a histogram #3413
show timers distribution on a histogram #3413
Conversation
tools/cli/adminTimers.go
Outdated
h.Add(t.VisibilityTimestamp.Format(hp.timeFormat)) | ||
} | ||
|
||
if err := h.Print(hp.ctx.Int(FlagShardMultiplier)); err != nil { |
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.
nit: maybe just return h.Print(hp.ctx.Int(FlagShardMultiplier))
the if does not add anything here.
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.
good catch!
tools/cli/histogram.go
Outdated
counters []*counter | ||
} | ||
|
||
func NewHistorgram() *Histogram { |
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.
s/NewHistorgram/NewHistogram
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.
done
tools/cli/histogram.go
Outdated
if len(key) > len(h.maxKey) { | ||
h.maxKey = key | ||
} | ||
|
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.
nit: remove empty line
tools/cli/histogram.go
Outdated
func (h *Histogram) Add(key string) { | ||
var found bool | ||
for _, c := range h.counters { | ||
|
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.
nit: remove empty line
tools/cli/adminTimers.go
Outdated
type Printer interface { | ||
Print(timers []*persistence.TimerTaskInfo) error | ||
} | ||
type TimerPrinter struct { |
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 we call it something else? Initial glance suggests that it might implement Printer
interface. While it is an aggregate structure of both loader and printer.
* show timers distribution on a histogram * Add license header * add convenient time bucket option * address comments * rename raw printer to json printer
What changed?
Why?
How did you test it?
Potential risks