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

Metrics for page resources #26

Merged
merged 6 commits into from
Aug 1, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Metrics are gathered in the format used by a statsd / graphite / grafana stack -
* PageChange - When the page loads, or whenever a navigation event occurs, we log a counter for the page visit and a gauge for how long the page load took. The page load is a measure of the time between the navigation event firing, the event loop lagging for over 25ms and finally the event loop freeing up for 125ms. Basically we wait for the page to get busy, then we wait for it to finish being busy.
* PageLoad - We use `window.performance` to get details of the initial page load.
* XhrStats - We monkey-patch over `XMLHttpRequest` in order to identify every AJAX request, the destination, the transition between states, final response size and timing.
* PageResources - `connectEnd`, `connectStart`, `domainLookupEnd`, `domainLookupStart`, `duration`, `fetchStart`, `redirectEnd`, `redirectStart`, `requestStart`, `responseEnd`, `responseStart`, `secureConnectionStart`, `startTime`, `workerStart`.

Choose a reason for hiding this comment

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

Seems inconsistent to list these for pageLoad but not the others


### How are the metrics transmitted?

Expand Down
2 changes: 1 addition & 1 deletion dist/barometer.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lib/barometer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var transport = require('./transport.js')
require('./pageLoadStats.js')
require('./xhrStats.js')
require('./pageChange')
require('./pageResources')

tracker.url = null
tracker.gauge = transport.gauge
Expand Down
52 changes: 52 additions & 0 deletions lib/pageResources.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* global window */
'use strict'
var pageResources = module.exports = {}

var pageChange = require('./pageChange.js')
var transport = require('./transport.js')
var lastIndex
var timingOffset = 0
var pageStart = new Date()

pageChange.onPageChange(function () {
timingOffset = (new Date() - pageStart)
var entries = window.performance.getEntries()
for (; lastIndex++; lastIndex < entries.length) {
pageResources.handleResource(entries[lastIndex])
}
})

pageResources.handleResource = function (entry) {
var metrics = pageResources.findRule(entry)
if (!metrics) return
pageResources.track(entry, metrics)
}

pageResources.findRule = function (entry) {
var config = window.barometer.resources
Copy link

@msaspence msaspence Jul 30, 2016

Choose a reason for hiding this comment

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

I'm thinking about this for bandwidthStats:

if (barometer.bandwidth === false) return 
var config = barometer.bandwidth || {}

this will give null or undefined values a default config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going with the idea that you don't define it if you don't want it. If you do define it, it should be an array of properties to match on and the metrics we want to capture. The default value should be [] in the code you've highlighted - I'll get to it on Sunday ;)

Choose a reason for hiding this comment

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

This does seem like one that should be disabled by default, maybe bandwidth as well, not sure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I've just re-read this now I'm fully behind a computer, I think we were talking about different things. I think you're right, bandwidth should be an opt-in, and maybe locked behind an onPageChanged event to limit any impact on end user experience. The other modules in this project check for appropriate browser support before they kick off - a similar approach can be taken for config. If there's no config, the module simply won't initialise.

if (!config) return

var name = entry.name.replace(/https?:\/\//i, '').split('/')
var domain = name.shift()
var path = name.join('/')

for (var i = 0; i < config.length; i++) {
var rule = config[i]
if (rule.domain && !domain.match(rule.domain)) continue
if (rule.resourceRegex && !path.match(rule.resourceRegex)) continue
if (rule.type && entry.entryType !== rule.type) continue
return rule.metrics
}
return null
}

pageResources.track = function (entry, metrics) {
var prefix = entry.name.replace(/https?:\/\//i, '').split('?')[0].replace(/[a-z0-9_]/gi, '_')

for (var i in metrics) {
var value = entry[i]
if (value === 0) continue
if (i !== 'duration') value = timingOffset - value
transport.gauge(prefix + '.' + i, value)
}
}