Skip to content

Commit

Permalink
Merge pull request #145 from Financial-Times/bug/NOPS-817-handle-grap…
Browse files Browse the repository at this point in the history 
…hite-null-values

NOPS-817- Handle Sample and Baseline null values.
  • Loading branch information
@bolajiadeyemi
bolajiadeyemi committed May 12, 2021
2 parents ef52afa + f823c05 commit 5cefdc3
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 12 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ Checks if the value of a graphite metric has received data recently.
#### `cloudWatchThreshold`
Checks whether the value of a CloudWatch metric has crossed a threshold

_Note: this assumes that `AWS_ACCESS_KEY` & `AWS_SECRET_ACCESS_KEY` are implictly available as environment variables on process.env_
_Note: this assumes that `AWS_ACCESS_KEY` & `AWS_SECRET_ACCESS_KEY` are implicitly available as environment variables on process.env_

* `cloudWatchRegion` = [default `'eu-west-1'`] AWS region the metrics are stored
* `cloudWatchMetricName` = [required] Name of the CloudWatch metric to count
Expand All @@ -142,7 +142,7 @@ _Note: this assumes that `AWS_ACCESS_KEY` & `AWS_SECRET_ACCESS_KEY` are implictl
#### `cloudWatchAlarm`
Checks whether the state of a CloudWatch alarm is health

_Note: this assumes that `AWS_ACCESS_KEY` & `AWS_SECRET_ACCESS_KEY` are implictly available as environment variables on process.env_
_Note: this assumes that `AWS_ACCESS_KEY` & `AWS_SECRET_ACCESS_KEY` are implicitly available as environment variables on process.env_

* `cloudWatchRegion` = [default `'eu-west-1'`] AWS region the metrics are stored
* `cloudWatchAlarmName` = [required] Name of the CloudWatch alarm to check
8 changes: 4 additions & 4 deletions src/checks/graphiteSpike.check.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ class GraphiteSpikeCheck extends Check {
generateUrl(numerator, divisor, period) {
const urlBase = this.ftGraphiteBaseUrl + `from=-${period}&format=json&target=`;
if(divisor) {
return urlBase + `divideSeries(summarize(${this.seriesFunction}(${numerator}),"${period}","${this.summarizeFunction}",true),summarize(${this.seriesFunction}(${divisor}),"${period}","${this.summarizeFunction}",true))`;
return urlBase + `divideSeries(summarize(${this.seriesFunction}(transformNull(${numerator})),"${period}","${this.summarizeFunction}",true),summarize(${this.seriesFunction}(transformNull(${divisor})),"${period}","${this.summarizeFunction}",true))`;
} else {
return urlBase + `summarize(${this.seriesFunction}(${numerator}),"${period}","${this.summarizeFunction}",true)`;
return urlBase + `summarize(${this.seriesFunction}(transformNull(${numerator})),"${period}","${this.summarizeFunction}",true)`;
}
}

Expand All @@ -95,9 +95,9 @@ class GraphiteSpikeCheck extends Check {
])

const data = this.normalize({
sample: sample[0] ? sample[0].datapoints[0][0] : 0,
sample: sample[0] && !Object.is(sample[0].datapoints[0][0], null) ? sample[0].datapoints[0][0] : 0,
// baseline should not be allowed to be smaller than one as it is use as a divisor
baseline: baseline[0] ? baseline[0].datapoints[0][0] : 1
baseline: baseline[0] && !Object.is(baseline[0].datapoints[0][0], null) ? baseline[0].datapoints[0][0] : 1
});

const ok = this.direction === 'up'
Expand Down
27 changes: 21 additions & 6 deletions test/graphiteSpike.check.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ describe('Graphite Spike Check', function(){
check.start();
setTimeout(() => {

expect(mockFetch.firstCall.args[0]).to.contain('from=-10min&format=json&target=summarize(sumSeries(next.metric.200),"10min","sum",true)');
expect(mockFetch.secondCall.args[0]).to.contain('from=-7d&format=json&target=summarize(sumSeries(next.metric.200),"7d","sum",true)');
expect(mockFetch.firstCall.args[0]).to.contain('from=-10min&format=json&target=summarize(sumSeries(transformNull(next.metric.200)),"10min","sum",true)');
expect(mockFetch.secondCall.args[0]).to.contain('from=-7d&format=json&target=summarize(sumSeries(transformNull(next.metric.200)),"7d","sum",true)');
expect(check.getStatus().ok).to.be.true;
done();
});
Expand All @@ -74,8 +74,8 @@ describe('Graphite Spike Check', function(){
}));
check.start();
setTimeout(() => {
expect(mockFetch.firstCall.args[0]).to.contain('from=-10min&format=json&target=divideSeries(summarize(sumSeries(next.metric.200),"10min","sum",true),summarize(sumSeries(metric.*),"10min","sum",true))');
expect(mockFetch.secondCall.args[0]).to.contain('from=-7d&format=json&target=divideSeries(summarize(sumSeries(next.metric.200),"7d","sum",true),summarize(sumSeries(metric.*),"7d","sum",true))');
expect(mockFetch.firstCall.args[0]).to.contain('from=-10min&format=json&target=divideSeries(summarize(sumSeries(transformNull(next.metric.200)),"10min","sum",true),summarize(sumSeries(transformNull(metric.*)),"10min","sum",true))');
expect(mockFetch.secondCall.args[0]).to.contain('from=-7d&format=json&target=divideSeries(summarize(sumSeries(transformNull(next.metric.200)),"7d","sum",true),summarize(sumSeries(transformNull(metric.*)),"7d","sum",true))');
expect(check.getStatus().ok).to.be.true;
done();
});
Expand Down Expand Up @@ -129,8 +129,8 @@ describe('Graphite Spike Check', function(){
}));
check.start();
setTimeout(() => {
expect(mockFetch.firstCall.args[0]).to.contain('from=-24h&format=json&target=divideSeries(summarize(sumSeries(next.metric.200),"24h","sum",true),summarize(sumSeries(metric.*),"24h","sum",true))');
expect(mockFetch.secondCall.args[0]).to.contain('from=-2d&format=json&target=divideSeries(summarize(sumSeries(next.metric.200),"2d","sum",true),summarize(sumSeries(metric.*),"2d","sum",true))');
expect(mockFetch.firstCall.args[0]).to.contain('from=-24h&format=json&target=divideSeries(summarize(sumSeries(transformNull(next.metric.200)),"24h","sum",true),summarize(sumSeries(transformNull(metric.*)),"24h","sum",true))');
expect(mockFetch.secondCall.args[0]).to.contain('from=-2d&format=json&target=divideSeries(summarize(sumSeries(transformNull(next.metric.200)),"2d","sum",true),summarize(sumSeries(transformNull(metric.*)),"2d","sum",true))');
done();
});
});
Expand Down Expand Up @@ -177,6 +177,21 @@ describe('Graphite Spike Check', function(){
done();
});
});

it('Should be possible to handle sample and baseline null values', function(done){
mockGraphite([null, null]);
check = new Check(getCheckConfig({
direction: 'up',
threshold: 5,
divisor: 'metric.*',
normalize: false,
}));
check.start();
setTimeout(() => {
expect(check.getStatus().ok).to.be.true;
done();
});
});
});


0 comments on commit 5cefdc3

Please sign in to comment.