Skip to content
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

report playback #2301

Merged
merged 1 commit into from
Mar 5, 2017
Merged

report playback #2301

merged 1 commit into from
Mar 5, 2017

Conversation

rade
Copy link
Member

@rade rade commented Mar 4, 2017

Now you can launch the scope app with something like

./prog/scope --mode=app --weave=false --app.collector=file:///tmp/reports

and if the specified dir contains reports with filenames in the form <timestamp>.{msgpack|json}[.gz]
e.g. "1488557088545489008.msgpack.gz", then these reports are replayed in a loop at a sequence and speed determined by the timestamps.

@rade rade requested a review from 2opremio March 4, 2017 19:40
@rade
Copy link
Member Author

rade commented Mar 4, 2017

This is really handy for benchmarking of the standalone app.

@awh grabbed 30s of Weave Cloud production reports for me, which I fed to a scope app running on my laptop and restricted to a single core (i.e. GOMAXPROCS=1). I then ran

for i in $(seq 1 100); do echo $i $(date); curl -o /dev/null -s http://localhost:4040/api/topology; done

to see at what rate I can grab the complete topology - something the UI tries to do every 5s. It's taking an average of 9s, with the CPU pegged. Ouch. And bear in mind that this isn't even reading & parsing reports - that's only done once, at startup.

Now you can launch the scope app with something like

./prog/scope --mode=app --weave=false --app.collector=file:///tmp/reports

and if the specified dir contains reports with filenames in the form
<timestamp>.{msgpack|json}[.gz],
e.g. "1488557088545489008.msgpack.gz", then these reports are replayed
in a loop at a sequence and speed determined by the timestamps.
@rade rade force-pushed the report-playback branch from ee5d392 to 289b4c6 Compare March 4, 2017 22:05
@2opremio
Copy link
Contributor

2opremio commented Mar 5, 2017

@rade Code looks good.

Have you explicitly tested that the demo service doesn't break with these changes? (i.e. that collecting a single file didn't break).

For extra points you could supply a unit test for it (using https://github.com/weaveworks/common/blob/master/fs/fs.go for mocking)

@rade
Copy link
Member Author

rade commented Mar 5, 2017

Have you explicitly tested that the demo service doesn't break with these changes? (i.e. that collecting a single file didn't break).

Yes.

For extra points you could supply a unit test for it

I thought about it, but the pain/gain equation doesn't work out, especially considering that this really isn't a mainstream user-facing feature.

@2opremio
Copy link
Contributor

2opremio commented Mar 5, 2017

I thought about it, but the pain/gain equation doesn't work out, especially considering that this really isn't a mainstream user-facing feature.

It may very well end up used for #2286 but fair enough.

@rade rade merged commit a391ae8 into master Mar 5, 2017
@rade rade deleted the report-playback branch July 5, 2017 13:09
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.

2 participants