Implement baseline cloudwatch metrics

[MatthewJWalls]

What does this change?

Adds a node based implementation of our standard base cloudwatch metrics to the production server (we don't collect these in CODE)

These incliude:

• maxHeapMemory
• freeDiskSpace
• usedHeapMemory
• freePhysicalMemory
• totalPhysicalMemory

Why?

All frontend services should log out these core health metrics so that we can use them to diagnose production issues.

[MatthewJWalls]

Since I don't work with js much I imagine there will be a lot of feedback for non-idiomatic code in here, if it's easier I'm willing to pair fixing this up with somebody!

[SiAdcock]

What is this used for? Note that environment variables set on process.env are not available outside of the Node runtime

[MatthewJWalls]

This makes the node aws client library use the appropriate local aws credentials if they're available, so that it works locally on your machine if you're testing the production server.

[SiAdcock]

Okay. Do we always want to set this, or just when we're running the app locally?

[MatthewJWalls]

It should be ok to always set this, as aws credentials work essentially as a series of fallbacks - it should try your local aws profile first, then look for credentials in other standard places. That's how it works in scala too (if it doesn't work in production it should be a quick tweak to force it to look explicitly).

[SiAdcock]

Cool, thanks for explaining, sounds good 👍

[SiAdcock]

For functions that are not constructor functions:

• prefer using arrow functions (EDIT: only when defined inline as callbacks)
• prefer (lower) camel case names
• for commands, prefer verb phrases

const collectAndSendAWSMetrics = (...metrics: Metric) => {
 // ... 
};

[MatthewJWalls]

Gotcha. I was following https://github.com/airbnb/javascript#functions but I might have missed something elsewhere.

[SiAdcock]

Interesting! Okay, let's stick with AirBnB's recos, ignore the arrow function point 😄

[MatthewJWalls]

Is this better?

[MatthewJWalls]

Wasn't sure whether I should do:

const collectAndSendAWSMetrics = function collectAndSendAWSMetrics(...)
vs
const collectAndSendAWSMetrics = function(...)

The other functions use the former but it's a bit of duplication

[SiAdcock]

As per AirBnB, the function name ought to be descriptive and unique, whereas the const name should be short. For example:

const sendMetrics = function collectAndSendAWSMetrics(...) { }

I'm not 100% sure I like it. It's hard enough to think of one function name let alone two! They have their reasons. I think we can do something a little more palatable. For now, let's leave it as is and take the discussion outside this PR.

[SiAdcock]

I did not know you could truncate an array like this! JS is full of surprises.

[MatthewJWalls]

Forgot to mention but I also added a 'make logs' command that shows the pm2 logs which was super useful for testing.

[SiAdcock]

I think since this and TimingMetric are not constructor functions, these should be lower camelcase also.

That said, they are serving as factory functions. Maybe we need a convention around these? What's better:

• createBytesMetric()
• bytesMetric()

?

[SiAdcock]

Happy for naming convention discussion to be taken offline, don't let me stop you merging this 👍

[MatthewJWalls]

I'm going to rebase this branch and then merge it, and take the convention issues offline.