Add id field to outputted checks

[nickramsbottom]

The healthcheck standards specify that each individual check requires an id. This adds an id to the check output as an optional argument.

Healthcheck standards

It may be worth making it required here but it would be a more significant breaking change

[andygout]

Seems super weird that it is not one of the props that would cause this error to throw if absent: https://github.com/Financial-Times/n-health/blob/master/src/checks/check.js#L24.

[andygout]

It may be worth making it required here but it would be a more significant breaking change

I think there has to be an approach to this (and is very likely bikeshedding), but to add this change, then ensure all healthchecks are providing an id, and then make it a mandatory field here.

And it might also be worth checking if that Healthcheck Standards doc is still up-to-date so that if we did go to the trouble of adding ids that the healthcheck aggregator would still want to make use of those values.

[nickramsbottom]

@andygout I agree with you on both points. I think it's something we could raise at dev huddle as a follow up to this PR?

[andygout]

Should we do that first or are you looking to merge this sooner?

My other outstanding concern would be that this is likely to produce a lot of ids that are undefined. Could that potentially cause issues further down the trail (e.g. between here and healthcheck aggregators like Heimdall)?

[nickramsbottom]

Looking to merge sooner than that ideally.

I don't know what harm that could cause but fields such as businessImpact are treated in the same way

[andygout]

If businessImpact is same then should be okay.

[nickramsbottom]

ohhh hang about, businessImpact is also a required field

checked the output in use, if the id is undefined it isn't shown on the health endpoint