Skip to content

Real Time Charts #93

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.

Sign up for GitHub

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

Closed
simonwistow wants to merge 3 commits into from
Closed

Real Time Charts #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e660663
Refactor the data fetching part of real-time stats out into its own m…
simonwistow May 22, 2020
cf04c00
This builds on the work done on the realtime stats display by generat…
simonwistow May 23, 2020
a313135
Various linter cleanups
simonwistow May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ require (
github.com/ajg/form v1.5.1 // indirect
github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d // indirect
github.com/blang/semver v3.5.1+incompatible
github.com/dustin/go-humanize v1.0.0
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
github.com/fastly/go-fastly v1.11.0
github.com/fatih/color v1.7.0
github.com/frankban/quicktest v1.5.0 // indirect
github.com/gizak/termui/v3 v3.1.0
github.com/google/go-cmp v0.3.1
github.com/google/go-github/v28 v28.1.1
github.com/google/jsonapi v0.0.0-20200226002910-c8283f632fb7 // indirect
Expand Down
10 changes: 10 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyG
github.com/dsnet/compress v0.0.1 h1:PlZu0n3Tuv04TzpfPbrnI0HW/YwodEXDS+oPKahKF0Q=
github.com/dsnet/compress v0.0.1/go.mod h1:Aw8dCMJ7RioblQeTqt88akK31OvO8Dhf5JflhBbQEHo=
github.com/dsnet/golib v0.0.0-20171103203638-1ea166775780/go.mod h1:Lj+Z9rebOhdfkVLjJ8T6VcRQv3SXugXy999NBtR9aFY=
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 h1:90Ly+6UfUypEF6vvvW5rQIv9opIL8CbmW9FT20LDQoY=
github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0/go.mod h1:V+Qd57rJe8gd4eiGzZyg4h54VLHmYVVw54iMnlAMrF8=
github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
Expand All @@ -36,6 +38,9 @@ github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjr
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
github.com/frankban/quicktest v1.5.0 h1:Tb4jWdSpdjKzTUicPnY61PZxKbDoGa7ABbrReT3gQVY=
github.com/frankban/quicktest v1.5.0/go.mod h1:jaStnuzAqU1AJdCO0l53JDCJrVDKcS03DbaAcR7Ks/o=
github.com/gizak/termui v3.1.0+incompatible h1:N3CFm+j087lanTxPpHOmQs0uS3s5I9TxoAFy6DqPqv8=
github.com/gizak/termui/v3 v3.1.0 h1:ZZmVDgwHl7gR7elfKf1xc4IudXZ5qqfDh4wExk4Iajc=
github.com/gizak/termui/v3 v3.1.0/go.mod h1:bXQEBkJpzxUAKf0+xq9MSWAvWZlE7c+aidmyFlkYTrY=
github.com/gliderlabs/ssh v0.2.2 h1:6zsha5zo/TWhRhwqCD3+EarCAgZ2yN28ipRnGPnwkI0=
github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0=
github.com/golang/gddo v0.0.0-20190419222130-af0f2af80721 h1:KRMr9A3qfbVM7iV/WcLY/rL5LICqwMHLhwRXKu99fXw=
Expand Down Expand Up @@ -81,19 +86,24 @@ github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc
github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
github.com/mattn/go-isatty v0.0.10 h1:qxFzApOv4WsAL965uUPIsXzAKCZxN2p9UqdhFS4ZW10=
github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcMEpPG5Rm84=
github.com/mattn/go-runewidth v0.0.2 h1:UnlwIPBGaTZfPQ6T1IGzPI0EkYAQmT9fAEJ/poFC63o=
github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mholt/archiver v3.1.1+incompatible h1:1dCVxuqs0dJseYEhi5pl7MYPH9zDa1wBi7mF09cbNkU=
github.com/mholt/archiver v3.1.1+incompatible/go.mod h1:Dh2dOXnSdiLxRiPoVfIr/fI1TwETms9B8CTWfeh7ROU=
github.com/mholt/archiver/v3 v3.3.0 h1:vWjhY8SQp5yzM9P6OJ/eZEkmi3UAbRrxCq48MxjAzig=
github.com/mholt/archiver/v3 v3.3.0/go.mod h1:YnQtqsp+94Rwd0D/rk5cnLrxusUBUXg+08Ebtr1Mqao=
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4=
github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
github.com/mitchellh/mapstructure v0.0.0-20170523030023-d0303fe80992/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/nicksnyder/go-i18n v1.10.1 h1:isfg77E/aCD7+0lD/D00ebR2MV5vgeQ276WYyDaCRQc=
github.com/nicksnyder/go-i18n v1.10.1/go.mod h1:e4Di5xjP9oTVrC6y3C7C0HoSYXjSbhh/dU0eUV32nB4=
github.com/nsf/termbox-go v0.0.0-20190121233118-02980233997d h1:x3S6kxmy49zXVVyhcnrFqxvNVCBPb2KZ9hV2RBdS840=
github.com/nsf/termbox-go v0.0.0-20190121233118-02980233997d/go.mod h1:IuKpRQcYE1Tfu+oAQqaLisqDeXgjyyltCfsaoYN18NQ=
github.com/nwaples/rardecode v1.0.0 h1:r7vGuS5akxOnR4JQSkko62RJ1ReCMXxQRPtxsiFMBOs=
github.com/nwaples/rardecode v1.0.0/go.mod h1:5DzqNKiOdpKKBH87u8VlvAnPZMXcGRhxWkRpHbbfGS0=
github.com/pelletier/go-buffruneio v0.2.0/go.mod h1:JkE26KsDizTr40EUHkXVtNPvgGtbSNq5BcowyYOWdKo=
Expand Down
2 changes: 1 addition & 1 deletion pkg/app/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,7 @@ COMMANDS
View realtime stats for a Fastly service

-s, --service-id=SERVICE-ID Service ID
--format=FORMAT Output format (json)
--format=FORMAT Output format (json, chart)

@pteichman pteichman Jul 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, something about the termui takeover rubs me the wrong way here–I thought of both the historical & realtime stats commands as stages in a shell pipeline, so format might expand to json/csv/text but this full takeover might be its own command. I guess that's more of a UI question than anything else.


For help on a specific command, try e.g.

Expand Down
5 changes: 3 additions & 2 deletions pkg/stats/obj.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type realtimeResponse struct {
}

type realtimeResponseData struct {
Recorded float64 `json:"recorded"`
Aggregated statsResponseData `json:"aggregated"`
Recorded float64 `json:"recorded"`
Aggregated statsResponseData `json:"aggregated"`
Datacenter map[string]statsResponseData `json:"datacenter"`
}
93 changes: 76 additions & 17 deletions pkg/stats/realtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewRealtimeCommand(parent common.Registerer, globals *config.Data) *Realtim
c.CmdClause = parent.Command("realtime", "View realtime stats for a Fastly service")
c.CmdClause.Flag("service-id", "Service ID").Short('s').Required().StringVar(&c.manifest.Flag.ServiceID)

c.CmdClause.Flag("format", "Output format (json)").EnumVar(&c.formatFlag, "json")
c.CmdClause.Flag("format", "Output format (json, chart)").EnumVar(&c.formatFlag, "json", "chart")

return &c
}
Expand All @@ -46,6 +46,10 @@ func (c *RealtimeCommand) Exec(in io.Reader, out io.Writer) error {
if err := loopJSON(c.Globals.RTSClient, service, out); err != nil {
return err
}
case "chart":
if err := loopChart(c.Globals.RTSClient, service, out); err != nil {
return err
}

default:
if err := loopText(c.Globals.RTSClient, service, out); err != nil {
Expand All @@ -64,6 +68,7 @@ func loopJSON(client api.RealtimeStatsInterface, service string, out io.Writer)
Data []json.RawMessage `json:"data"`
}

// this is annoyingly similar to the same loop in loopText
err := client.GetRealtimeStatsJSON(&fastly.GetRealtimeStatsInput{
Service: service,
Timestamp: timestamp,
Expand All @@ -82,6 +87,74 @@ func loopJSON(client api.RealtimeStatsInterface, service string, out io.Writer)
}

func loopText(client api.RealtimeStatsInterface, service string, out io.Writer) error {

@pteichman pteichman Jul 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like this already slipped in, but loopText/loopJSON never return an error so there shouldn't be one in the function signature.

responses := make(chan realtimeResponse)
errors := make(chan error)
go fetchData(client, service, responses, errors)

@pteichman pteichman Jul 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Splitting responses and errors into their own channels implies that they're totally decoupled from each other, and that will make it harder in the future to add features to this loop (backoff? error handling?). So if doing this with a channel, you might want a channel of struct { realtimeResponse, error } to get one item per attempt.

fetchData also doesn't ever return, aside from being implicitly killed at process exit. This is risky because it means loopText etc can't add an innocent looking return without leaking the goroutine.

Personally I wouldn't solve this with channels at all–make fetchData return one batch of data and move it into the for loop a few lines down.


for {
select {
case err := <-errors:
text.Error(out, "fetching stats: %w", err)
continue
case envelope := <-responses:
for _, block := range envelope.Data {
agg := block.Aggregated

// FIXME: These are heavy-handed compatibility
// fixes for stats vs realtime, so we can use
// fmtBlock for both.
agg["start_time"] = block.Recorded
delete(agg, "miss_histogram")

if err := fmtBlock(out, service, agg); err != nil {
text.Error(out, "formatting stats: %w", err)
continue
}
}
}
}
}

func loopChart(client api.RealtimeStatsInterface, service string, out io.Writer) error {
responses := make(chan realtimeResponse)
errors := make(chan error)

view, err := NewView(service)
if err != nil {
return err
}
defer view.Close()
view.Resize()
view.SetLayout()

go fetchData(client, service, responses, errors)

for {
select {
case err := <-errors:

@pteichman pteichman Jul 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, here it makes more sense to use a channel because you do want to select across your remote calls and view events (because the termui API is all channel based).

So I might do a combination of what was above:

  • make one channel of struct { realtimeResponse, error } (to make future loop changes easier)
  • have the various loopFoos create a context.Context using WithCancel so you get a CancelFunc
  • make fetchData cancel-able by adding a ctx context.Context as its first argument (and check for cancelation in its loop)
  • call the CancelFunc at all of the loopFoo exit points (defer is fine here too) so that fetchData won't survive any longer than loopFoo

text.Error(out, "fetching stats: %w", err)
continue
case envelope := <-responses:
for _, block := range envelope.Data {
if err := view.UpdateStats(block); err != nil {
text.Error(out, "charting stats: %w", err)
continue
}
view.Render()
}
case e := <-view.Events:
switch e.ID { // event string/identifier
case "q", "<C-c>": // press 'q' or 'C-c' to quit
return nil

@pteichman pteichman Jul 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This return is one place the infinite looping fetchData will leak.

case "<Resize>":
view.Resize()
view.Render()
}
}
}
}

func fetchData(client api.RealtimeStatsInterface, service string, responses chan realtimeResponse, errors chan error) {
var timestamp uint64
for {
var envelope realtimeResponse
Expand All @@ -91,24 +164,10 @@ func loopText(client api.RealtimeStatsInterface, service string, out io.Writer)
Timestamp: timestamp,
}, &envelope)
if err != nil {
text.Error(out, "fetching stats: %w", err)
errors <- err
continue
}
timestamp = envelope.Timestamp

for _, block := range envelope.Data {
agg := block.Aggregated

// FIXME: These are heavy-handed compatibility
// fixes for stats vs realtime, so we can use
// fmtBlock for both.
agg["start_time"] = block.Recorded
delete(agg, "miss_histogram")

if err := fmtBlock(out, service, agg); err != nil {
text.Error(out, "formatting stats: %w", err)
continue
}
}
responses <- envelope
}
}
Loading