feat: Added support for getting container ids from ECS metadata API #2292
Conversation
| if (process.env.ECS_CONTAINER_METADATA_URI_V4 != null) { | ||
| return true | ||
| } | ||
| return process.env.ECS_CONTAINER_METADATA_URI != null |
There was a problem hiding this comment.
| if (process.env.ECS_CONTAINER_METADATA_URI_V4 != null) { | |
| return true | |
| } | |
| return process.env.ECS_CONTAINER_METADATA_URI != null | |
| return process.env.ECS_CONTAINER_METADATA_URI_V4 != null || process.env.ECS_CONTAINER_METADATA_URI != null |
There was a problem hiding this comment.
For me, it's clearer as it is written that we are preferring the V4 version and then falling back to the other.
|
|
||
| module.exports.getVendorInfo = fetchDockerVendorInfo | ||
| module.exports.clearVendorCache = function clearDockerVendorCache() { | ||
| vendorInfo = null | ||
| } | ||
|
|
||
| module.exports.getBootId = function getBootId(agent, callback) { | ||
| module.exports.getBootId = function getBootId(agent, callback, logger = log) { |
There was a problem hiding this comment.
it's fine but seems like a lot of signature updates to inject a logger. if you look through our unit tests we use proxyquire to inject mock loggers
There was a problem hiding this comment.
I think being able to inject the parameter is a lot cleaner than using proxyquire. We don't need to couple the implementation of the whole module to the tests (the tests needing to know how the logger is imported and used in the code).
| fs_access: fs.access, | ||
| os_platform: os.platform | ||
| } | ||
| fs.access = (file, mode, cb) => { |
There was a problem hiding this comment.
this is purely informational but a lot of our tests use sinon for mocking.
sinon.stub(fs, 'access').yields(Error('no proc file'))
This PR resolves #2276.