Skip to content

Commit

Permalink
Add support for Render and ECS (#22)
Browse files Browse the repository at this point in the history
* Add support for Render and ECS

* Include config info in reports
  • Loading branch information
adamlogic authored Dec 15, 2023
1 parent 3ee6ce3 commit cfa3fe2
Show file tree
Hide file tree
Showing 14 changed files with 3,392 additions and 6,800 deletions.
18 changes: 9 additions & 9 deletions express/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion express/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"dependencies": {
"async": "^2.6.3",
"express": "^4.17.3",
"judoscale-node-core": "^1.0.5",
"judoscale-node-core": "*",
"unirest": "^0.6.0",
"winston": "^3.6.0"
}
Expand Down
6 changes: 4 additions & 2 deletions node-core/src/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import Api from './lib/api'
import defaultConfig from './lib/default-config'
import defaultConfigFunction from './lib/default-config'
import mergeConfig from './lib/merge-config'
import logger from './lib/logger'
import Metric from './lib/metric'
import MetricsStore from './lib/metrics-store'
import Report from './lib/report'
import Reporter from './lib/reporter'

export { Api, defaultConfig, mergeConfig, logger, Metric, MetricsStore, Report, Reporter }
const defaultConfig = defaultConfigFunction()

export { Api, mergeConfig, logger, Metric, MetricsStore, Report, Reporter, defaultConfig }
35 changes: 25 additions & 10 deletions node-core/src/lib/default-config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
import getLogger from './logger'

const defaultLogLevel = process.env.JUDOSCALE_LOG_LEVEL || 'info'

export default {
version: '1.1.0',
api_base_url: process.env.JUDOSCALE_URL,
log_level: defaultLogLevel,
dyno: process.env.DYNO,
report_interval_seconds: 10,
now: null,
logger: getLogger(defaultLogLevel),
export default function () {
const defaultLogLevel = process.env.JUDOSCALE_LOG_LEVEL || 'info'

let apiBaseUrl = process.env.JUDOSCALE_URL
let containerID = process.env.DYNO

if (process.env.RENDER_INSTANCE_ID) {
containerID = process.env.RENDER_INSTANCE_ID.replace(process.env.RENDER_SERVICE_ID, '').replace('-', '')
apiBaseUrl = `https://adapter.judoscale.com/api/${process.env.RENDER_SERVICE_ID}`
}

if (process.env.ECS_CONTAINER_METADATA_URI) {
const parts = process.env.ECS_CONTAINER_METADATA_URI.split('/')
containerID = parts[parts.length - 1]
}

return {
version: '1.1.0',
api_base_url: apiBaseUrl,
log_level: defaultLogLevel,
container: containerID,
report_interval_seconds: 10,
now: null,
logger: getLogger(defaultLogLevel),
}
}
2 changes: 1 addition & 1 deletion node-core/src/lib/merge-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import getLogger from './logger'
import defaultConfig from './default-config'

export default (config) => {
const mergedConfig = { ...defaultConfig, ...config }
const mergedConfig = { ...defaultConfig(), ...config }

return {
...mergedConfig,
Expand Down
6 changes: 3 additions & 3 deletions node-core/src/lib/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ class Report {

payload() {
return {
dyno: this.config.dyno,
container: this.config.container,
pid: process.pid,
// TODO: this.config.asJson()
config: {},
// Convert logger (DerivedLogger instance) into something sane
config: { ...this.config, logger: this.config.logger && this.config.logger.constructor.name },
adapters: this.adapter.asJson(),
metrics: this.metrics.map((metric) => [
metric.time.getTime() / 1000,
Expand Down
5 changes: 3 additions & 2 deletions node-core/src/test/api.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* global test, expect, describe, jest */

import Api from '../lib/api'
import defaultConfig from '../lib/default-config'
import defaultConfigFunction from '../lib/default-config'
import unirest from 'unirest'

jest.mock('unirest', () => {
Expand All @@ -11,10 +11,11 @@ jest.mock('unirest', () => {
...originalModule,
post: jest.fn().mockReturnThis(),
headers: jest.fn().mockReturnThis(),
send: jest.fn().mockResolvedValue({})
send: jest.fn().mockResolvedValue({}),
}
})

const defaultConfig = defaultConfigFunction()
const api = new Api(defaultConfig)

describe('constructor', () => {
Expand Down
37 changes: 30 additions & 7 deletions node-core/src/test/default-config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,50 @@ import logger from '../lib/logger'
jest.mock('../lib/logger')

describe('defaultConfig', () => {
beforeEach(() => {
delete process.env.RENDER_INSTANCE_ID
delete process.env.RENDER_SERVICE_ID
delete process.env.DYNO
delete process.env.JUDOSCALE_URL
delete process.env.ECS_CONTAINER_METADATA_URI
})

test('has logger property', () => {
expect(defaultConfig).toHaveProperty('logger')
expect(defaultConfig()).toHaveProperty('logger')
})

test('has now property', () => {
expect(defaultConfig).toHaveProperty('now', null)
expect(defaultConfig()).toHaveProperty('now', null)
})

test('has api_base_url property', () => {
expect(defaultConfig).toHaveProperty('api_base_url', process.env.JUDOSCALE_URL)
process.env.JUDOSCALE_URL = 'HO HO HO'
expect(defaultConfig()).toHaveProperty('api_base_url', 'HO HO HO')
})

test('has api_base_url and container properties for render', () => {
process.env.RENDER_INSTANCE_ID = 'renderServiceId-renderInstanceId'
process.env.RENDER_SERVICE_ID = 'renderServiceId'

expect(defaultConfig()).toHaveProperty('api_base_url', 'https://adapter.judoscale.com/api/renderServiceId')
expect(defaultConfig()).toHaveProperty('container', 'renderInstanceId')
})

test('has container property for ECS', () => {
process.env.ECS_CONTAINER_METADATA_URI = 'ecs-service/container-id'
expect(defaultConfig()).toHaveProperty('container', 'container-id')
})

test('has dyno property', () => {
expect(defaultConfig).toHaveProperty('dyno', process.env.DYNO)
test('has container property for Heroku', () => {
process.env.DYNO = 'web.123'
expect(defaultConfig()).toHaveProperty('container', 'web.123')
})

test('has version property', () => {
expect(defaultConfig).toHaveProperty('version', '1.1.0')
expect(defaultConfig()).toHaveProperty('version', '1.1.0')
})

test('has report_interval_seconds property', () => {
expect(defaultConfig).toHaveProperty('report_interval_seconds', 10)
expect(defaultConfig()).toHaveProperty('report_interval_seconds', 10)
})
})
24 changes: 17 additions & 7 deletions node-core/src/test/report.test.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,38 @@
/* global test, expect, describe */

import Report from '../lib/report'
import defaultConfig from '../lib/default-config'
import Metric from '../lib/metric'

const collectors = [{ a: 'collector' }]
const adapter = { asJson: () => { 'payload' }, collectors }
const adapter = {
asJson: () => {
'payload'
},
collectors,
}
const exampleConfig = { container: 'web.007' }
const metric = new Metric('some-identifier', new Date(), '1234')
const report = new Report(adapter, defaultConfig, [metric])
const report = new Report(adapter, exampleConfig, [metric])

describe('constructor', () => {
test('adapter property', () => {
expect(report.adapter).toEqual(adapter)
})

test('config property', () => {
expect(report.config).toEqual(defaultConfig)
expect(report.config).toEqual(exampleConfig)
})
})

describe('payload', () => {
const payload = report.payload()

test('metrics with property value', () => {
expect(payload).toHaveProperty('metrics', [[(metric.time.getTime() / 1000), metric.value, metric.identifier, null]])
expect(payload).toHaveProperty('metrics', [[metric.time.getTime() / 1000, metric.value, metric.identifier, null]])
})

test('dyno with property value', () => {
expect(payload).toHaveProperty('dyno', report.config.dyno)
test('container with property value', () => {
expect(payload).toHaveProperty('container', 'web.007')
})

test('pid with property value', () => {
Expand All @@ -37,4 +42,9 @@ describe('payload', () => {
test('adapters with property value', () => {
expect(payload).toHaveProperty('adapters', report.adapter.asJson())
})

test('config with property value', () => {
expect(typeof payload.config).toEqual('object')
expect(payload.config.container).toEqual('web.007')
})
})
2 changes: 1 addition & 1 deletion sample_apps/express_web/Procfile
Original file line number Diff line number Diff line change
@@ -1 +1 @@
web: npm run start
web: yarn start
19 changes: 7 additions & 12 deletions sample_apps/express_web/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ A tiny Node.js app for testing Judoscale.

[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy?template=https://github.com/judoscale/judoscale-sample-express)

___
---

### Local Run

Expand All @@ -17,25 +17,20 @@ ___
```shell
touch .env && echo "NODE_ENV=development" >> .env
```
- Install dependencies:
- Install dependencies. Note that we're using `yarn` instead of `npm` so we can use "resolutions". More on this below.
```shell
npm install
yarn install
```
- Then run the app:
```shell
bin/dev
```
This will run not only the app but also Judoscale proxy so you can check the metrics being collected by it.
In case you are working on one of Judoscale's adapters within this project you can replace the **NPM** published one for a local version.
Note that the sample app uses the NPM registry version of judoscale-express, but the local version of judoscale-node-core. If you want to use the local version of judoscale-express, change the `resolutions` in `package.json`.
- Uninstall the original package
```shell
npm uninstall judoscale-node
```
There doesn't seem to be a way to use _both_ local versions of judoscale-express and judoscale-node-core at the same time. When you use the local version of judoscale-express, the transitive dependency judoscale-node-core dependency will always be the register version, even if you include the local override in `resolutions`.

- Replace it with the local copy
```shell
npm install "../../web"
```
You could temporarily fix this by updating the `package.json` in judoscale-express, but make sure you don't commit that change.
Loading

2 comments on commit cfa3fe2

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage report for ./node-core

St.
Category Percentage Covered / Total
🟡 Statements 63.64% 42/66
🟡 Branches 66.67% 16/24
🔴 Functions 56.52% 13/23
🟡 Lines 64.62% 42/65

Test suite run success

38 tests passing in 6 suites.

Report generated by 🧪jest coverage report action from cfa3fe2

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage report for ./express

St.
Category Percentage Covered / Total
🔴 Statements 58.62% 17/29
🔴 Branches 42.86% 3/7
🟡 Functions 75% 6/8
🔴 Lines 58.62% 17/29

Test suite run success

17 tests passing in 4 suites.

Report generated by 🧪jest coverage report action from cfa3fe2

Please sign in to comment.