Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Commit

Permalink
Merge pull request #69 from Jayflux/stats_to_method
Browse files Browse the repository at this point in the history 
Bring getStatsName into the Client object fixes #68
  • Loading branch information
Nick Spragg authored Jul 7, 2017
2 parents 1d4355e + 62c8abf commit bbfe4cd
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 22 deletions.
25 changes: 16 additions & 9 deletions lib/cachingClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,18 @@ CachingClient.prototype.get = function (url, opts, cb) {
}

if (maxAge) {
cache.set(createKeyObject(url, opts, doNotVary), cachedObject, maxAge, client._handleCacheError.bind(client));
// Anonymous function needed here to pass opts through, as cache only calls func(err)
cache.set(createKeyObject(url, opts, doNotVary), cachedObject, maxAge, function(err) {
client._handleCacheError.call(client, err, opts);
});
}

if (!err && client.staleIfError && staleIfError) {
var staleExpiry = staleIfError + maxAge;
cache.set(createStaleKeyObject(url, opts, doNotVary), cachedObject, staleExpiry, client._handleCacheError.bind(client));
// Anonymous function needed here to pass opts through, as cache only calls func(err)
cache.set(createStaleKeyObject(url, opts, doNotVary), cachedObject, staleExpiry, function(err) {
client._handleCacheError.call(client, err, opts);
});
}
}

Expand All @@ -143,13 +149,14 @@ CachingClient.prototype._getCachedOrFetch = function (url, opts, cb) {
var delegate = this.delegate;
var doNotVary = this.doNotVary;


cache.get(createKeyObject(url, opts, doNotVary), function (err, cached) {
if (err) client._handleCacheError(err);
if (err) client._handleCacheError(err, opts);

var cacheHit = cached && cached.item && (cached.item.body || cached.item.error);

if (cacheHit) {
if (delegate.stats) delegate.stats.increment(delegate.name + '.cache.hits');
if (delegate.stats) delegate.stats.increment(delegate._getStatsName(opts) + '.cache.hits');

if (cached.item.error) {
err = createErrorFromCache(cached.item.error);
Expand All @@ -158,16 +165,16 @@ CachingClient.prototype._getCachedOrFetch = function (url, opts, cb) {
return cb(err, cached.item.body, cached.item.response, true);
}

if (delegate.stats) delegate.stats.increment(delegate.name + '.cache.misses');
if (delegate.stats) delegate.stats.increment(delegate._getStatsName(opts) + '.cache.misses');

delegate.getWithResponse(url, opts, function (err, body, res) {
if (err && client.staleIfError) {
var originalErr = err;

return cache.get(createStaleKeyObject(url, opts, doNotVary), function (err, cached) {
if (err) client._handleCacheError(err);
if (err) client._handleCacheError(err, opts);
if (!cached || cached.item.error) return cb(originalErr, body, res);
if (delegate.stats) delegate.stats.increment(delegate.name + '.cache.stale');
if (delegate.stats) delegate.stats.increment(delegate._getStatsName(opts) + '.cache.stale');

cb(err, cached.item.body, cached.item.response, true);
});
Expand All @@ -178,10 +185,10 @@ CachingClient.prototype._getCachedOrFetch = function (url, opts, cb) {
});
};

CachingClient.prototype._handleCacheError = function (err) {
CachingClient.prototype._handleCacheError = function (err, opts) {
if (err) {
if (this.delegate.logger) this.delegate.logger.warn('Cache error:', err.message);
if (this.delegate.stats) this.delegate.stats.increment(this.delegate.name + '.cache.errors');
if (this.delegate.stats) this.delegate.stats.increment(this.delegate._getStatsName(opts) + '.cache.errors');
}
};

Expand Down
26 changes: 13 additions & 13 deletions lib/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,6 @@ function isCriticalError(err) {
return true;
}

function getStatsName(clientName, feedName) {
if (feedName) {
return clientName + '.' + feedName;
}

return clientName;
}

function buildResponse(requestRes) {
return {
statusCode: requestRes.statusCode,
Expand All @@ -115,11 +107,19 @@ Client.prototype._log = function (res) {
}
};

Client.prototype._recordStats = function (res, feedName) {
Client.prototype._getStatsName = function (opts) {
if (opts && 'name' in opts) {
return this.name + '.' + opts.name;
}

return this.name;
};

Client.prototype._recordStats = function (res, opts) {
var statsName;

if (this.stats) {
statsName = getStatsName(this.name, feedName);
statsName = this._getStatsName(opts);
this.stats.increment(statsName + '.requests');
this.stats.increment(statsName + '.responses.' + res.statusCode);
this.stats.timing(statsName + '.response_time', res.elapsedTime);
Expand All @@ -140,7 +140,7 @@ Client.prototype._request = function (method, url, opts, cb) {

if (err) {
if (client.stats) {
statsName = getStatsName(client.name, opts.name);
statsName = client._getStatsName(opts);
client.stats.increment(statsName + '.request_errors');
}
err = new Error(util.format('Request failed for %s %s', buildUrl(url, opts), err.message));
Expand All @@ -151,7 +151,7 @@ Client.prototype._request = function (method, url, opts, cb) {
var res = buildResponse(requestRes);

client._log(requestRes);
client._recordStats(requestRes, opts.name);
client._recordStats(requestRes, opts);

if (requestRes.statusCode >= 400) {
err = new Error(util.format('Received HTTP code %d for %s %s', requestRes.statusCode, requestRes.request.method, requestRes.request.href));
Expand Down Expand Up @@ -194,7 +194,7 @@ Client.prototype._requestWithRetries = function (method, url, opts, cb) {
}

if (client.stats) {
statsName = getStatsName(client.name, opts.name);
statsName = client._getStatsName(opts);
client.stats.timing(statsName + '.attempts', currentAttempts);
}

Expand Down
18 changes: 18 additions & 0 deletions test/caching.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,24 @@ describe('Caching', function () {
});
});

it('Uses the request name when incrementing the counter', function (done) {
client.get(url, {name: 'sheeba'}, function (err) {
assert.ifError(err);
sinon.assert.calledWith(stats.increment, 'http.sheeba.requests');
sinon.assert.calledWith(stats.increment, 'http.sheeba.responses.200');
done();
});
});

it('Uses the request name when incrementing the counter (when the cache fails)', function (done) {
catbox.set.yields(new Error('Good use of Sheeba!'));
client.get(url, {name: 'sheeba'}, function (err) {
assert.ifError(err);
sinon.assert.calledWith(stats.increment, 'http.sheeba.cache.errors');
done();
});
});

it('returns the response when writing to the cache fails', function (done) {
catbox.set.withArgs(expectedKey).returns(new Error('Good use of Sheeba!'));
client.get(url, function (err, body) {
Expand Down
10 changes: 10 additions & 0 deletions test/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ describe('Rest Client', function () {
});
});

it('calls _getStatsName when incrementing', function (done) {
var spy = sinon.spy(client, '_getStatsName');
nock.cleanAll();
client.get(url, function (err) {
assert(err);
sinon.assert.called(spy);
done();
});
});

it('increments a counter for errors with feed name in it', function (done) {
var client = Client.createClient({
name: 'my_client',
Expand Down

0 comments on commit bbfe4cd

Please sign in to comment.