Let's get this running on CI once tests are merged, so we don't break anything later on!

add function to bin so that we can run it like -

elb-metrics --starttime -endtime 

Let's keep it simple in terms of what this tool provides:

• Input: the bare minimum params you need to retrieve ELB metrics.
• Output: an object that provides a high-level overview of results.

Current bench provides a high level overview of the whole time period, like

### Bench (600s)

- requests:     225.2/s
- connect mean: 142.0ms
- finish mean:  142.1ms
- payload mean: 3315.2
- non 200/304:  1602 (1.2%)
- cacheHit:     0 (0.0%)
- cacheRefresh: 0 (0.0%)
- cacheMiss:    0 (0.0%)
- cacheUnknown: 135119 (100.0%)

I would like to see us do the same. How about we return

• Time period (number of seconds)
• Average requests/second over the period
• Average latency over the period
• Percentage of total requests that were 2xx
• Percentage of total requests that were 3xx
• Percentage of total requests that were 4xx
• Percentage of total requests that were 5xx

Here's an example of how I want to interact with this utility:

test('give me metrics', function(assert) {
  var params = {
    start: 1471356157000,
    end: 1471356167000,
    elbName: 'my-elb',
    region: 'us-east-1'
  };

  elbMetrics(params, function(err, res) {
    assert.ifError(err);
    assert.deepEqual(res, {
      'period': '10s',
      'requests': '500/s',
      'mean latency': '100ms',
      '2xx': '67%',
      '3xx': '20%',
      '4xx': '10%',
      '5xx': '3%'
    });
    assert.end();
  });
});

The elbMetrics function will be doing more work behind the scenes, and should be comprised of other functions that we test individually:

1. take user parameters and combine them with other assumed parameters that we need to make a request to CloudWatch
2. make the request to CloudWatch
3. parse the results and do some math to find the averages/values we are interested in
4. prepare the final object based on those values ^ and return it

Even though there's a lot of different work to do, the idea is to expose a single function, elbMetrics, to the user.

@aarthykc do you feel 👍 about tackling each of those functions (and tests) separately and then putting them all together in a function like the test case above describes ?

Required Elements

If any elements in the below list are not checked, this repo will fail standards compliance.

• Not running node 4 or below
• Has at least some test coverage?
• Has a README?

Rubric

• 1 pt Is in Version Control/Github ✅ (free points)
• 1-2 pt node version:
  • 2 pt Best: running node 8+ 🏅
  • 1 pt Questionable: node 6
• 1 pt CI enabled for repo?
• 1 pt Not running Circle CI version 1? (Point awarded if using Travis)
• 1 pt nyc integrated to show test coverage summary?
• 1-3 pt test coverage percentage from nyc?
  • 3 pt High coverage: > 90%
  • 2 pt Moderate coverage: between 75 and 90% total coverage
  • 1 pt 0 - 74% test coverage
• 1-2 pt evidence of bug fixes/edge cases being tested?
  • 2 pt Strong evidence/several instances noted
  • 1 pt Some evidence - I had a hard time deciding this one
• 1 pt no flags to enable different functionality in non-test environments?
• 1 pt Has README?
• 1 pt Has CHANGELOG?
• 1-2 pt README explains purpose of a project and how it works to some detail?
  • 2 pt High (but appropriate) amount of detail about the project
  • 1 pt Some detail about the project documented, could be more extensive
• 1 pt README contains deploy/publish/install instructions?
• 1 pt README contains CI badges, as appropriate?
• 1-2 pt Code seems self-documenting: file/module names, function names, variables? No redundant comments to explain naming conventions?
  • 2 pt Strongly self-documented code, little to no improvements needed
  • 1 pt Some evidence of self-documenting code
• 1 pt No potential security vulnerabilities are reported in dependencies?
• 1 pt Package is scoped to @mapbox on NPM?
• 1-2 pt master branch protected?
  • 1 pt PRs can only be merged if tests are passing?
  • 1 pt PRs must be approved before merging?
• 2 pt BONUS: was this repo covered in a deep dive at some point? (part of bench, so, going to award points for that!)

Total possible: 24 points (+2 bonus)
Grading scale:

Repo grade: 15/24. Grade 3 (2018-08-08)

cc/ @mapbox/sreious-business

From #5, our goal for this project is some human-readable output like:

{
  'period': '10s',
  'requests': '500/s',
  'mean latency': '100ms',
  '2xx': '67%',
  '3xx': '20%',
  '4xx': '10%',
  '5xx': '3%'
}

Doing the math

• period
  • Calculate this with the difference between startTime and endTime - should be in seconds, for now
• requests
  • Add up all datapoints from RequestCount metric, and divide by period (ie number of seconds), to get number of requests per second
• mean latency
  • Average of all datapoints from Latency metric
• 2xx
  • Calculate total requests by adding all datapoints from RequestCount metric. Calculate total 2xx requests by adding all datapoints from HTTPCode_Backend_2XX metric. 2xx percentage will be total 2xx requests / total requests
• 3xx
  • Rinse and repeat process from 2xx
• 4xx
  • Rinse and repeat process from 2xx
• 5xx
  • Rinse and repeat process from 2xx

Scope of work

✅ New (synchronous) function prepareResults

• takes, as input, raw metrics (ie what comes back from your outputMetrics function). also needs, as input, startTimeand endTime
• output should be an object that looks something like what is below, making sure that units are also included

{
  'period': '10s',
  'requests': '500/s',
  'mean latency': '100ms',
  '2xx': '67%',
  '3xx': '20%',
  '4xx': '10%',
  '5xx': '3%'
}

✅ Tests for prepareResults

• test that all the math makes sense on a small scale (using small fixtures so you can do the math in your head to confirm it makes sense)
• test that it handles edge cases gracefully -- e.g. what if there are no 5xx requests?
• test any error cases we need to handle here

✅ Integrate into elbMetrics function

• should be able to feed this output into your new prepareResults function, and return that response to the end user

I think if this was scoped to the @mapbox namespace and published with public access, by default everyone at Mapbox would have access to publish to the package per https://docs.npmjs.com/cli/access. 🙂

Ref: #24

cc/@aarthykc

Return the first data point with the correct statistic in the form of a table

Bench errors out, when the duration of a task is > 60 mins and doesn't return any valid inputs:

{
 "messageId": "xxx",
 "error": "start and end time should not be more than 60 minutes apart"
}

https://github.com/mapbox/elb-metrics/blob/master/index.js#L38

Would it be okay to increase this limit or remove it?

cc/ @aarthykc

ELB 2 / Application Load Balancing is a different namespace, and some of the metrics are named differently (ie "Latency" becomes "TargetResponseTime"). It would be great to be compatible with both versions of ELB. Can this be detected automatically, or passed as a flag, and the right metrics returned whether you're using classic ELB or ELB 2?

cc @aarthykc @yhahn

Allow it to take arguments like - --starttime --endtime

cc @emilymcafee, @yhahn

We're close with getMetrics, but we need to make sure it gathers everything we're looking for. I'm seeing a need for a synchronous function that prepares all of your CloudWatch queries. It could be called something like prepareQueries or prepareParams.

• Input: startTime, endTime, elbname
• Output: an array of objects that could each be passed as params to cloudwatch.getMetricStatistics. (eg each will be an object like this)
• The metrics to collect, per #5 and http://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/elb-metricscollected.html#loadbalancing-metrics-clb:

@aarthykc let's add this to your branch in #1 and close this issue when you have the following:

• new function in index.js that takes startTime, endTime, elbname and returns all of the params objects you will need (in an array)
• new tests in index.test.js that:
  • tests successful cases
  • tests error cases (e.g. if some input is missing, or if the input is the incorrect type) -- remember this is a synchronous function, so we should throw ^_^

Once we have ^ we can close here and worry about integration back in #1.

The final act of getMetrics will be to return all datapoints in a sensible way, that way they can be consumed for analysis later on.

@aarthykc I'm thinking the final response from getMetrics should be formatted like this:

{
    'request count': [ /* all of your datapoints here */ ],
    'latency': [ /* datapoints */ ],
    '2xx': [ /* datapoints */ ],
    '3xx': [ /* datapoints */ ],
    '4xx': [ /* datapoints */ ],
    '5xx': [ /* datapoints */ ]
}

Just opening this for visibility, you can implement in #1 and close here when done?

The README.md should have some basic documentation on how to use this tool.

• Description of what it does
• How to install
• Usage