fix(vector): fix internal_metrics typo and set sane 30s scrape interval - #3440
fix(vector): fix internal_metrics typo and set sane 30s scrape interval#3440saule1508 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: saule1508 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe Vector internal metrics API now uses ChangesInternal metrics scrape interval
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR corrects the internal metrics configuration naming and sets an explicit 30-second scrape interval; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the memory issue, identifies the missing configuration and 1-second default, and summarizes the implementation. It includes reviewer and approver assignments and the required Links section. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @saule1508. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
@jcantrill fixing name of config parameter is totally correct scrape_interval_secs, but not sure about value in |
|
/retest-required |
|
@saule1508: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Hi @vparfonov, thank you for the comment ! Currently the scrape_interval_secs is omitted, and it default to one second (https://vector.dev/docs/reference/configuration/sources/internal_metrics/) but due to the volume of metrics computed in our openshift cluster collector nodes (which is not a specially big cluster) this generates massive memory consumption. |
|
@saule1508 do you have a graphic or other image to identifies the memory savings after this change? |
|
@jcantrill Spent the day measuring this properly. Test cluster, Logging 6.6, 7 collectors, external Loki, worker nodes have 128 CPU cores. On this PR specifically: vector_internal_metrics_cardinality = 359 per collector, so at the 1s default each collector emits ∼359 events/s.
For scale, that 2.7 MB/s was more than all real log sources on the same collectors combined — application, audit and infrastructure inputs are each in the 100–500 KB/s range.
So the collector was producing ∼26× more metric snapshots than anything consumed, each additionally cloned through the generated add_nodename_to_metric remap. Our ServiceMonitors (instance, minimal-instance, telemetry-instance) all inherit the platform-default 30s scrape interval. Correction to my original report: I opened this attributing our collector memory usage to internal metrics. I believe that was mostly wrong. Our actual memory problem is that collector memory scales with node core count, not log volume — ∼810 MB on 8-core nodes vs 5–6.5 GB on 128-core nodes in the same cluster, with memory.peak hitting 9.7 GiB against a 10 GiB limit. Setting VECTOR_THREADS=8 dropped those 128-core collectors from ∼6 GB to under 1 GB. I'll report that separately once I've confirmed the mechanism — I suspect it's simply that the collector container has no CPU limit (cpu.max = max), so Vector sizes its runtime to all 128 host cores, in which case it may be fixable via spec.collector.resources.limits.cpu with no code change. Testing that tomorrow. I do see a memory reduction after adding the scrape_interval_secs = 30, not as compelling as the one from the vector_threads. I will do a proper test and post a chart tomorrow. I am not sure the value of 30s is the best, since the default scape interval is 30s it might make sense, or could be a bit lower, maybe 15s ? I can test other values if it helps. Will work on it tomorrow |
If the CLF defines no collector requests/limits, the operator defaults to the following values: This means it should at most consume 6 cores at 2G each. If you provide any values, it takes what is spec'd. Vector is agressive and will consume as much is available. If the default's are not being respected then that is a bug and should be logged accordingly |
|
Following up on your comment about default limits, I have found strong evidence of a bug where the default CPU limit is not being applied. On a test cluster, I examined a collector pod running on a 128-cores worker node.
oc describe command shows:
but when I look into the container at the number of tasks I have 177, so it will be 128 threads for sure This confirms that the operator is not enforcing the expected default CPU limit of 6000m, causing Vector to consume excessive resources on high-core nodes.
I applied the following limit (via terraform) and now the threads are only 36 and the memory dropped to 500MB, in the collector pod: |
|
@jcantrill do you want me to create a new issue regarding the default cpu limit not being respected ? I can also look at the code to see if I can find the error ? |

Description
The Vector collector DaemonSet consistently consumes 4–6 GiB of RAM per node (and up to 10 GiB) on our cluster. By looking at the prometheus metrics, the source of the memory consumption is the component internal_metrics which generates upward of 3Mib/s (compared to 0.2 MiB/s forthe application logs).
This is because the generated
vector.tomlcompletely omitsscrape_interval_secs, forcing Vector to poll its entire registry on its internal 1-second defaultThis PR corrects the typo in the key name (ScrapeIntervalSeconds instead of ScrapIntervalSeconds) and in the serialization tags (secs instead of seconds) and sets a sane, explicit default of 30 seconds inside the generator.
/cc jcantrill
/assign alanconway
Links
Summary by CodeRabbit