-
Notifications
You must be signed in to change notification settings - Fork 44
feat(dashmate): add prometheus service discovery labels #2818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2.2-dev
Are you sure you want to change the base?
Changes from 3 commits
4e1c115
208608b
62337dd
57a0a67
1a6e248
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -527,7 +527,7 @@ static_resources: | |
| address: gateway_rate_limiter | ||
| port_value: 8081 | ||
| {{?}} | ||
| {{? it.platform.gateway.metrics.enabled && it.platform.gateway.admin.enabled }} | ||
| {{? it.platform.gateway.metrics.enabled }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you remove this condition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics without admin don't work. Assuming the intention of an user who enabled metrics is to get them really working ;), I've changed logic so that when metrics are enabled, always enable admin automatically but don't expose it to public. See line 546-550 below. When admin is not enabled, adminHost will be 127.0.0.1, what means it will listen on loopback of docker container (not docker host). |
||
| - name: admin | ||
| connect_timeout: 0.25s | ||
| type: STATIC | ||
|
|
@@ -542,11 +542,12 @@ static_resources: | |
| port_value: 9901 | ||
| {{?}} | ||
|
|
||
| {{? it.platform.gateway.admin.enabled }} | ||
| {{? it.platform.gateway.admin.enabled || it.platform.gateway.metrics.enabled }} | ||
| {{ adminHost = it.platform.gateway.admin.enabled ? '0.0.0.0' : '127.0.0.1'; }} | ||
| admin: | ||
| address: | ||
| socket_address: | ||
| address: 0.0.0.0 # For docker container only. Must be a local/private interface. | ||
| address: {{= adminHost }} # defaults to loopback when not explicitly enabled | ||
| port_value: 9901 | ||
| {{?}} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import HomeDir from '../../../src/config/HomeDir.js'; | ||
| import getBaseConfigFactory from '../../../configs/defaults/getBaseConfigFactory.js'; | ||
| import renderTemplateFactory from '../../../src/templates/renderTemplateFactory.js'; | ||
| import renderServiceTemplatesFactory from '../../../src/templates/renderServiceTemplatesFactory.js'; | ||
lklimek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| function getRsDapiBlock(dynamicComposeContent) { | ||
| const match = dynamicComposeContent.match(/rs_dapi:\n((?: {2}.*\n)+)/); | ||
| return match ? match[1] : ''; | ||
| } | ||
|
|
||
| describe('dynamic compose template', () => { | ||
| let getBaseConfig; | ||
| let renderServiceTemplates; | ||
|
|
||
| beforeEach(() => { | ||
| getBaseConfig = getBaseConfigFactory(HomeDir.createTemp()); | ||
| const renderTemplate = renderTemplateFactory(); | ||
| renderServiceTemplates = renderServiceTemplatesFactory(renderTemplate); | ||
| }); | ||
|
|
||
| it('should not publish metrics port when rs-dapi metrics are disabled', () => { | ||
| const config = getBaseConfig(); | ||
|
|
||
| const renderedConfigs = renderServiceTemplates(config); | ||
| const rsDapiBlock = getRsDapiBlock(renderedConfigs['dynamic-compose.yml']); | ||
|
|
||
| expect(rsDapiBlock).to.not.include('ports:'); | ||
| expect(rsDapiBlock).to.not.include(':0'); | ||
| }); | ||
|
|
||
| it('should publish metrics port when rs-dapi metrics are enabled', () => { | ||
| const config = getBaseConfig(); | ||
|
|
||
| config.set('platform.dapi.rsDapi.metrics.enabled', true); | ||
| config.set('platform.dapi.rsDapi.metrics.port', 29091); | ||
| config.set('platform.dapi.rsDapi.metrics.host', '127.0.0.1'); | ||
|
|
||
| const renderedConfigs = renderServiceTemplates(config); | ||
| const rsDapiBlock = getRsDapiBlock(renderedConfigs['dynamic-compose.yml']); | ||
|
|
||
| expect(rsDapiBlock).to.include('ports:\n - 127.0.0.1:29091:29091'); | ||
| expect(rsDapiBlock).to.include('- 29091'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import getBaseConfigFactory from '../../../configs/defaults/getBaseConfigFactory.js'; | ||
| import HomeDir from '../../../src/config/HomeDir.js'; | ||
| import renderServiceTemplatesFactory from '../../../src/templates/renderServiceTemplatesFactory.js'; | ||
| import renderTemplateFactory from '../../../src/templates/renderTemplateFactory.js'; | ||
lklimek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| describe('envoy template', () => { | ||
| it('should render admin interface when metrics are enabled even if admin is disabled', () => { | ||
| const getBaseConfig = getBaseConfigFactory(HomeDir.createTemp()); | ||
| const config = getBaseConfig(); | ||
|
|
||
| config.set('platform.gateway.metrics.enabled', true); | ||
| config.set('platform.gateway.admin.enabled', false); | ||
|
|
||
| const renderTemplate = renderTemplateFactory(); | ||
| const renderServiceTemplates = renderServiceTemplatesFactory(renderTemplate); | ||
| const renderedConfigs = renderServiceTemplates(config); | ||
|
|
||
| const envoyConfig = renderedConfigs['platform/gateway/envoy.yaml']; | ||
|
|
||
| expect(envoyConfig).to.include('cluster_name: admin'); | ||
| expect(envoyConfig).to.include('address: 127.0.0.1'); | ||
| expect(envoyConfig).to.include('port_value: 9901'); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you remove this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case will never happen, see https://github.com/dashpay/platform/pull/2818/files#r2472878855