Unable to push all metrics

[deejay1]

Using the 4.0.1 docker image I'm trying to push all metrics into graphite to replicate the previous (3x) behaviour.

Unfortunately the documented --metrics.filter '*+' option results in an exception

[2016-10-31 10:18:21] 147 requests, 1645.72 kb, firstPaint: 0.18s (±0.01), DOMContentLoaded: 0.36s (±0), Load: 1.22s (±0.21), rumSpeedIndex: 0.55s (±0.01) (3 runs)
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] Send data to Graphite graphite-server:2003
[2016-10-31 10:18:22] TypeError: Cannot read property 'replace' of undefined
    at keyPathFromMessage (/usr/src/app/lib/plugins/graphite/data-generator.js:30:41)
    at GraphiteDataGenerator.dataFromMessage (/usr/src/app/lib/plugins/graphite/data-generator.js:46:19)
    at Object.processMessage (/usr/src/app/lib/plugins/graphite/index.js:46:35)
    at queue.process (/usr/src/app/lib/support/queueHandler.js:157:57)
    at drainItem (/usr/src/app/node_modules/concurrent-queue/index.js:92:21)
    at Immediate.drain (/usr/src/app/node_modules/concurrent-queue/index.js:73:84)
    at runCallback (timers.js:637:20)
    at tryOnImmediate (timers.js:610:5)
    at processImmediate [as _immediateCallback] (timers.js:582:5)
[2016-10-31 10:18:22] Render HTML for 1 page(s)

and results in an empty configuredMetrics.txt file

Tryin --metrics.filter 'browsertime.*' 'pagexray.*' 'coach.*' 'assets.*' 'domains.*' also doesn't seem to send all metrics.

[soulgalore]

Yes seems to broken. Lets fix them one by one.

• First there's bug for the domain with the error you get and that makes everything fail + missing domain values.
• empty configuredMetrics.txt file
• --metrics.filter 'browsertime.' 'pagexray.' 'coach.' 'assets.' 'domains.*' will test that when the others things are fixed.

[soulgalore]

I'll try fix this tonight and then we can release 4.0.2.

[soulgalore]

Ok, so I the first part (so it is possible to send domain data to Graphite on summary level and that it doesn't break).

About the empty configuredMetrics.txt: We need to change the docs and rename the file. The understanding now it that it should hold all the metric names that are configured but the functionality is that it is the configuredMetrics keys. So if you pass *+ it should instead be one start instead of empty, that would make it easier to understand. But wanna sync this with @tobli and @beenanner before we push any changes.

Also the docs for configuring metrics needs to be updated.

There's also some fixes that's needed for making assets work correctly, I'll make a separate issue for that.

[soulgalore]

Now we almost have the functionality as in 3.x. What's left is updating the docs to be clear and more understandable, I'll start on that tomorrow.

[deejay1]

The return of asset metrics is much appreciated!

[deejay1]

Seems like not all URL characters are changed to the proper grephite format, for example we get "?" and "&" pushed to graphite which are rejected either by the relay or carbon itself

[deejay1]

Also there's a problem when testing multiple location on a single site with the slowest assets, where when we test example.com/foo and example.com/bar we end up with {graphite.prefix}.summary.example_com.chrome.native.slowestassets.0.asset1 and {graphite.prefix}.summary.example_com.chrome.native.slowestassets.0.asset2 so the path and query strings are not taken into account which makes it hard to separate different queries. We could try to manipulate the graphite.prefix setting but it would be a less userfriendly..

[soulgalore]

Hey @deejay1 , ok thanks. So you run with --graphite.includeQueryParams then? Just to make sure we don't have another bug. Yep I see have we missed where we convert the value: https://github.com/sitespeedio/sitespeed.io/blob/master/lib/support/flattenMessage.js#L9-L11 Do you have time to send a PR for & and ? ? Else I can fix that tonight.

Yes I see, if you configure includeQueryParams that should also affect the keys for slowestaassets, I've missed that yesterday when I fixed the rest. Can you share your URL public so I can verify when I do the test, else if you can drop me an email: peter [AT] soulgalore [DOT] com.

Thanks Peter

[soulgalore]

hmm about that last thing, not 100% though need think about that. I'll get back tonight.

[deejay1]

@1 Yes we run with includeQueryParams, @2 Sure I can, for example one of the URL-s is: http://allegro.pl/listing/listing.php?order=m&string=bmw

[soulgalore]

thanks!

for {graphite.prefix}.summary.example_com.chrome.native.slowestassets.0.asset1 I don't see a good solution for that right now (except having different second part of the prefix). You mean you wanna have something to show from where you start your run?

[deejay1]

I wonder if we have to put the numbers into the slowest asset times as they can be easily compared anyway (with an slowestassets.{asset_url}.{other_keys} using either a graph or max/min graphite functions. Adding the numbering only seems to break the usual routine. Changing the graphite prefix for each url being run is probably a solution we can live with.

[soulgalore]

I wonder if we have to put the numbers into the slowest asset times as they can be easily compared anyway

I've had a long day, you try to explain again :)

Let aim for doing a release tomorrow night so the fix is pushed. We got some other things we wanna bundle I think.

[deejay1]

OK, today now I have a long day (although it only begun an hour ago :/ ), so I hope I can make this more precise. Right now the slowest asset metrics are pushed in the format:

prefix.summary.my_url.chrome.native.slowestassets.0.http__asset1.cacheTime/Size/etc
prefix.summary.my_url.chrome.native.slowestassets.1.http__asset2.cacheTime/Size/etc

I meant that the 0 and 1 are not needed, because it is already visible by querying graphite for the cacheTime (for example, any value can be taken) itself and the numbers just add additional clutter, because you have to query for prefix.summary.myurl.chrome.native.slowestassets.._.cacheTime instead of just prefix.summary.my_url.chrome.native.slowestassets.*.cacheTime. So the implicit ordering by using 0-9 path elements isn't really helping IMHO...

As a sidenote I preferred the whole asset list that was in 3.x because I think it made tracking speed changes over time easier, now we just forget the assets that were previously on the top list when their loading times have decreased.

[soulgalore]

ah ok I see now what you mean. We could probably change that.

As a sidenote I preferred the whole asset list that was in 3.x because I think it made tracking speed changes over time easier, now we just forget the assets that were previously on the top list when their loading times have decreased.

If you wanna track all assets I don't think you should use the toplist, you are right. I was thinking that you could use the aggregated list for that (but that has a limit of X amount of assets). Right now we have all the data needed in pagexray.pageSummary.assets but is missing the actual URL. Let us create a separate issue for adding that so it's more user friendly (you could see what asset that are slow).

[soulgalore]

ok, I've been rethinking this again :/ the best solution is to keep the toplist as before (without URLS) that will make it usable for all not grow the datasize. Then next step is to add the URL for all assets, then we have the possibility (if you want) to track all assets. lets fix this during the weekend. sorry for the changes.

[soulgalore]

We haven't done anything for four years, I think we will not fix this.