Skip to content

Commit f5abb2d

Browse files
committed
new structure of the Graphite keys (always the same for pages) sitespeedio#651
1 parent 7e7534a commit f5abb2d

File tree

3 files changed

+91
-68
lines changed

3 files changed

+91
-68
lines changed

lib/graphite/graphiteCollector.js

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ GraphiteCollector.prototype.collect = function(aggregates, pages, domains) {
3232

3333
statistics += self._getDomainStats(domains, config.urlObject.hostname);
3434

35-
// FIXME We have a problem with double dots and we need to fix the original
36-
// issue on 4.0.2
37-
statistics = statistics.split('..').join('.');
38-
3935
return statistics;
4036

4137
};
@@ -65,7 +61,7 @@ GraphiteCollector.prototype._getRuleStats = function(page, urlKey) {
6561
if (page.yslow && (this.config.graphiteData.indexOf('rules') > -1 || this.config.graphiteData.indexOf(
6662
'all') > -1)) {
6763
Object.keys(page.rules).forEach(function(rule) {
68-
statistics += self.namespace + '.' + urlKey + 'rules.' + rule + ' ' +
64+
statistics += self.namespace + '.' + urlKey + '.rules.' + rule + ' ' +
6965
page.rules[rule].v + self.timeStamp;
7066
});
7167
}
@@ -94,7 +90,7 @@ GraphiteCollector.prototype._getBrowserTimeStats = function(page, urlKey) {
9490
statsWeWillPush.forEach(function(val) {
9591
// is it a browser?
9692
if (self.config.supportedBrowsers.indexOf(timing) < 0) {
97-
statistics += self.namespace + '.' + urlKey + type + '.' + timing +
93+
statistics += self.namespace + '.' + urlKey + '.' + type + '.' + timing +
9894
'.' + val + ' ' + page[type][timing][val].v + self.timeStamp;
9995
}
10096
});
@@ -105,7 +101,7 @@ GraphiteCollector.prototype._getBrowserTimeStats = function(page, urlKey) {
105101
if (self.config.supportedBrowsers.indexOf(browser) > -1) {
106102
Object.keys(page[type][browser]).forEach(function(timing) {
107103
statsWeWillPush.forEach(function(val) {
108-
statistics += self.namespace + '.' + urlKey + type + '.' +
104+
statistics += self.namespace + '.' + urlKey + '.' + type + '.' +
109105
browser + '.' + timing + '.' + val + ' ' + page[type][
110106
browser
111107
][timing][val].v + self.timeStamp;
@@ -131,23 +127,23 @@ GraphiteCollector.prototype._getPageMetricsStats = function(page, urlKey) {
131127
// and all the assets
132128
if (page.yslow) {
133129
Object.keys(page.yslow.assets).forEach(function(asset) {
134-
statistics += self.namespace + '.' + urlKey + 'assets.' + asset +
130+
statistics += self.namespace + '.' + urlKey + '.assets.' + asset +
135131
' ' + page.yslow.assets[asset].v + self.timeStamp;
136132
});
137133

138134
// and page specific
139-
statistics += self.namespace + '.' + urlKey + 'score' + ' ' + page.score +
135+
statistics += self.namespace + '.' + urlKey + '.score' + ' ' + page.score +
140136
self.timeStamp;
141-
statistics += self.namespace + '.' + urlKey + 'noRequests' + ' ' + page.yslow
137+
statistics += self.namespace + '.' + urlKey + '.noRequests' + ' ' + page.yslow
142138
.requests.v + self.timeStamp;
143-
statistics += self.namespace + '.' + urlKey + 'requestsMissingExpire' +
139+
statistics += self.namespace + '.' + urlKey + '.requestsMissingExpire' +
144140
' ' + page.yslow.requestsMissingExpire.v + self.timeStamp;
145141
statistics += self.namespace + '.' + urlKey +
146-
'timeSinceLastModification' + ' ' + page.yslow.timeSinceLastModification
142+
'.timeSinceLastModification' + ' ' + page.yslow.timeSinceLastModification
147143
.v + self.timeStamp;
148-
statistics += self.namespace + '.' + urlKey + 'cacheTime' + ' ' + page.yslow
144+
statistics += self.namespace + '.' + urlKey + '.cacheTime' + ' ' + page.yslow
149145
.cacheTime.v + self.timeStamp;
150-
statistics += self.namespace + '.' + urlKey + 'pageWeight' + ' ' + page.yslow
146+
statistics += self.namespace + '.' + urlKey + '.pageWeight' + ' ' + page.yslow
151147
.pageWeight.v + self.timeStamp;
152148
}
153149
}
@@ -158,7 +154,7 @@ GraphiteCollector.prototype._getPageMetricsStats = function(page, urlKey) {
158154
GraphiteCollector.prototype._getGPSIStats = function(page, urlKey) {
159155
// add gspi score
160156
if (page.gpsi) {
161-
return this.namespace + '.' + urlKey + 'gpsi' + ' ' + page.gpsi.gscore.v + this.timeStamp;
157+
return this.namespace + '.' + urlKey + '.gpsi' + ' ' + page.gpsi.gscore.v + this.timeStamp;
162158
} else {
163159
return '';
164160
}
@@ -175,7 +171,7 @@ GraphiteCollector.prototype._getWPTStats = function(page, urlKey) {
175171
Object.keys(page.wpt[location][browser]).forEach(function(connectivity) {
176172
Object.keys(page.wpt[location][browser][connectivity]).forEach(function(view) {
177173
Object.keys(page.wpt[location][browser][connectivity][view]).forEach(function(metric) {
178-
statistics += self.namespace + '.' + urlKey + 'wpt.' + location + '.' +
174+
statistics += self.namespace + '.' + urlKey + '.wpt.' + location + '.' +
179175
connectivity + '.' + browser + '.' + view + '.' + metric + '.median ' +
180176
page.wpt[location][browser][connectivity][view][metric].v +
181177
self.timeStamp;
@@ -229,16 +225,16 @@ GraphiteCollector.prototype._getAssetsStats = function(page, urlKey) {
229225
if (entry.timings) {
230226
timings.forEach(function(timing) {
231227
total += entry.timings[timing];
232-
stats += self.namespace + '.' + urlKey + 'requests.' + assetURL + '.timings.' + timing +
228+
stats += self.namespace + '.' + urlKey + '.requests.' + assetURL + '.timings.' + timing +
233229
' ' + entry.timings[timing] + timeStamp;
234230
});
235-
stats += self.namespace + '.' + urlKey + 'requests.' + assetURL + '.timings.total ' + total + ' ' +
231+
stats += self.namespace + '.' + urlKey + '.requests.' + assetURL + '.timings.total ' + total + ' ' +
236232
timeStamp;
237233
}
238234
// lets also add the size & type when we are here
239235
// we use the timestamp for the whole run to make sure
240236
// we only get one entry, this can and should be cleaned up later
241-
stats += self.namespace + '.' + urlKey + 'requests.' + assetURL + '.type.' +
237+
stats += self.namespace + '.' + urlKey + '.requests.' + assetURL + '.type.' +
242238
util.getContentType(entry.response.content.mimeType) + '.size ' + entry.response.content.size + ' ' + self.timeStamp;
243239

244240
});
@@ -258,9 +254,11 @@ GraphiteCollector.prototype._getDomainStats = function(domains, hostname) {
258254
var timings = ['blocked', 'dns', 'connect', 'ssl', 'send', 'wait', 'receive', 'total'];
259255
var values = ['min', 'median', 'max'];
260256

257+
261258
domains.forEach(function(domain) {
262259
timings.forEach(function(timing) {
263260
values.forEach(function(value) {
261+
// TODO we should use the protovol also in the key right
264262
stats += self.namespace + '.summary.' + hostname + '.domains.timings.' + domain.domain.split('.').join('_') +
265263
'.' +
266264
timing + '.' +

lib/util/util.js

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -285,48 +285,29 @@ module.exports = {
285285
return '-' + (locationAndBrowser.replace(':', '-') + '-' + connectivity).toLowerCase();
286286
},
287287

288-
getGraphiteURLKey: function(theUrl, char) {
289-
290-
char = char || '.';
288+
getGraphiteURLKey: function(theUrl) {
291289

292290
var join = '.';
291+
var char = '_';
293292

294293
var myUrl = url.parse(theUrl);
295294
var protocol = myUrl.protocol.replace(':', '');
296-
var hostname = myUrl.hostname;
295+
var hostname = myUrl.hostname.split('.').join(char);
297296
var pathName = myUrl.pathname;
298297

299298
// https://github.com/sitespeedio/sitespeed.io/issues/642
300-
if (pathName === null) {
301-
pathName = '';
302-
}
303-
304-
// special hack if we don't use . as char, then always use the separator
305-
// everywhere
306-
if (char !== '.') {
307-
hostname = myUrl.hostname.split('.').join(char);
308-
}
309-
310-
if (pathName.indexOf('.') > -1) {
311-
pathName = pathName.split('.').join('_');
312-
}
313-
if (pathName.indexOf('~') > -1) {
314-
pathName = pathName.split('~').join('_');
315-
}
316-
if (pathName.indexOf(' ') > -1) {
317-
pathName = pathName.split(' ').join('_');
299+
if (pathName === null || pathName === '' || pathName === '/') {
300+
pathName = 'slash';
318301
}
319302

320-
if (pathName === '' || pathName === '/') {
321-
return protocol + join + hostname + join + 'slash' + join;
322-
}
303+
var replace = ['.', '~', ' ', '/'];
304+
replace.forEach(function(replaceMe) {
305+
if (pathName.indexOf(replaceMe) > -1) {
306+
pathName = pathName.split(replaceMe).join(char);
307+
}
308+
});
323309

324-
var key = protocol + join + hostname + join + pathName.split('/').join(char);
325-
if (key.indexOf('.', key.length - 1) !== -1 || char !== '.') {
326-
return key;
327-
} else {
328-
return key + join;
329-
}
310+
return protocol + join + hostname + join + pathName;
330311
},
331312
fineTuneUrls: function(okUrls, errorUrls, maxPagesToTest, absResultDir, callback) {
332313
var log = winston.loggers.get('sitespeed.io');

test/util/utilTest.js

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,95 +9,139 @@ describe('util', function() {
99

1010
it('text/plain should be categorized as doc', function() {
1111
var result = util.getContentType('text/plain');
12-
assert.deepEqual('doc', result);
12+
assert.deepEqual(result,'doc');
1313
});
1414

1515
it('text/html should be categorized as doc', function() {
1616
var result = util.getContentType('text/html');
17-
assert.deepEqual('doc', result);
17+
assert.deepEqual(result,'doc');
1818
});
1919

2020
it('text/html; charset=utf-8 with charset should be categorized as doc', function() {
2121
var result = util.getContentType('text/html; charset=utf-8');
22-
assert.deepEqual('doc', result);
22+
assert.deepEqual(result,'doc');
2323
});
2424

2525
it('text/javascript should be categorized as js', function() {
2626
var result = util.getContentType('text/javascript');
27-
assert.deepEqual('js', result);
27+
assert.deepEqual(result,'js');
2828
});
2929

3030
it('application/x-javascript; charset=utf-8 should be categorized as js', function() {
3131
var result = util.getContentType('application/x-javascript; charset=utf-8');
32-
assert.deepEqual('js', result);
32+
assert.deepEqual(result,'js');
3333
});
3434

3535
it('text/css should be categorized as css', function() {
3636
var result = util.getContentType('text/css');
37-
assert.deepEqual('css', result);
37+
assert.deepEqual(result,'css');
3838
});
3939

4040
it('image/png should be categorized as image', function() {
4141
var result = util.getContentType('image/png');
42-
assert.deepEqual('image', result);
42+
assert.deepEqual(result,'image');
4343
});
4444

4545
it('image/jpg should be categorized as image', function() {
4646
var result = util.getContentType('image/jpg');
47-
assert.deepEqual('image', result);
47+
assert.deepEqual(result,'image');
4848
});
4949

5050
it('image/gif should be categorized as image', function() {
5151
var result = util.getContentType('image/gif');
52-
assert.deepEqual('image', result);
52+
assert.deepEqual(result,'image');
5353
});
5454

5555
it('image/x-icon should be categorized as image', function() {
5656
var result = util.getContentType('image/x-icon');
57-
assert.deepEqual('image', result);
57+
assert.deepEqual(result,'image');
5858
});
5959

6060
it('image/svg+xml should be categorized as image', function() {
6161
var result = util.getContentType('image/svg+xml');
62-
assert.deepEqual('image', result);
62+
assert.deepEqual(result,'image');
6363
});
6464

6565
it('image/webp should be categorized as image', function() {
6666
var result = util.getContentType('image/webp');
67-
assert.deepEqual('image', result);
67+
assert.deepEqual(result,'image');
6868
});
6969

7070

7171
it('application/font-woff should be categorized as font', function() {
7272
var result = util.getContentType('application/font-woff');
73-
assert.deepEqual('font', result);
73+
assert.deepEqual(result,'font');
7474
});
7575

7676
it('application/font-sfnt should be categorized as font', function() {
7777
var result = util.getContentType('application/font-sfnt');
78-
assert.deepEqual('font', result);
78+
assert.deepEqual(result,'font');
7979
});
8080

8181
it('application/x-font-opentype should be categorized as font', function() {
8282
var result = util.getContentType('application/x-font-opentype');
83-
assert.deepEqual('font', result);
83+
assert.deepEqual(result,'font');
8484
});
8585

8686
it('application/x-font-ttf should be categorized as font', function() {
8787
var result = util.getContentType('application/x-font-ttf');
88-
assert.deepEqual('font', result);
88+
assert.deepEqual(result,'font');
8989
});
9090

9191
it('application/x-shockwave-flash should be categorized as flash', function() {
9292
var result = util.getContentType('application/x-shockwave-flash');
93-
assert.deepEqual('flash', result);
93+
assert.deepEqual(result,'flash');
9494
});
9595

9696
it('application/my-own-type should be categorized as unkown', function() {
9797
var result = util.getContentType('application/my-own-type');
98-
assert.deepEqual('unknown', result);
98+
assert.deepEqual(result,'unknown');
9999
});
100100

101101
});
102102

103+
describe('#getGraphiteURLKey', function() {
104+
105+
it('A domain without slash should return protocol.www_domain_com ', function() {
106+
var result = util.getGraphiteURLKey('http://www.sitespeed.io');
107+
assert.deepEqual(result, 'http.www_sitespeed_io.slash');
108+
});
109+
110+
it('A https domain should start with https in the keys', function() {
111+
var result = util.getGraphiteURLKey('https://www.sitespeed.io');
112+
assert.deepEqual(result,'https.www_sitespeed_io.slash');
113+
});
114+
115+
it('A domain with a slash should return protocol.www_domain_com', function() {
116+
var result = util.getGraphiteURLKey('http://www.sitespeed.io/');
117+
assert.deepEqual(result,'http.www_sitespeed_io.slash');
118+
});
119+
120+
it('Many subdomains should keep all domains in the domain part of the key', function() {
121+
var result = util.getGraphiteURLKey('http://so.many.subdomains.sitespeed.io/hepp');
122+
assert.deepEqual(result,'http.so_many_subdomains_sitespeed_io._hepp');
123+
});
124+
125+
it('The path should be separated from the domain (ending without a slash)', function() {
126+
var result = util.getGraphiteURLKey('http://www.sitespeed.io/too/deep');
127+
assert.deepEqual(result,'http.www_sitespeed_io._too_deep');
128+
});
129+
130+
it('The path should be separated from the domain (ends with a slash)', function() {
131+
var result = util.getGraphiteURLKey('http://www.sitespeed.io/too/deep/');
132+
assert.deepEqual(result,'http.www_sitespeed_io._too_deep_');
133+
});
134+
135+
it('The path and files should be separated from the domain (when a file is in a folder)', function() {
136+
var result = util.getGraphiteURLKey('http://www.sitespeed.io/js/my.js');
137+
assert.deepEqual(result,'http.www_sitespeed_io._js_my_js');
138+
});
139+
140+
it('The path and files should be separated from the domain (when the file is in root)', function() {
141+
var result = util.getGraphiteURLKey('http://www.sitespeed.io/image.gif');
142+
assert.deepEqual(result,'http.www_sitespeed_io._image_gif');
143+
});
144+
145+
146+
});
103147
});

0 commit comments

Comments
 (0)