fix(core): Respect prefix for all Prometheus metrics #10130
Conversation
n8n-assistant
bot
added
core
| } | ||
|
|
||
| disableAllLabels() { | ||
| for (const label of Object.keys(this.includes.labels) as MetricLabel[]) { |
There was a problem hiding this comment.
could we not use a for-in loop here instead of a for-of on Object.keys?
| metrics: { | ||
| default: config.getEnv('endpoints.metrics.includeDefaultMetrics'), | ||
| api: config.getEnv('endpoints.metrics.includeApiEndpoints'), |
There was a problem hiding this comment.
How do you mean? This is an internal implementation detail, the schema path and the env remain the same.
There was a problem hiding this comment.
does this not change the metric name that gets scraped by prometheus?
There was a problem hiding this comment.
Oh that's what you mean. Since we advertise this as experimental I take it this is okay to change, but I can add a mention just in case.
| }); | ||
| } | ||
|
|
||
| private addPrefixToMetrics(metrics: string) { |
There was a problem hiding this comment.
could we make this public and add unit tests just for this method?
There was a problem hiding this comment.
This is covered by integration tests containing the n8n_test_ prefix.
3 flaky tests on run #6064 ↗︎
Details:
|
||||||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| NDV > should not retrieve remote options when required params throw errors |
Screenshots
Video
|
|
| NDV > Stop listening for trigger event from NDV |
Screenshots
Video
|
|
10-undo-redo.cy.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Undo/Redo > should undo/redo adding nodes |
Test Replay
Screenshots
Video
|
|
Review all test suite changes for PR #10130 ↗︎
|
✅ All Cypress E2E specs passed |
|
Got released with |
Prometheus metrics from
express-prom-bundleandprom-clientwere not respectingN8N_METRICS_PREFIX