cluster: add new option to set VM as a remote writer #2520

nolouch · 2025-03-25T07:21:03Z

What problem does this PR solve?

allow metrics data write to ng-monitroing (VM)

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

Need to cherry-pick to the release branch
Need to update the documentation

Release notes:

NONE

Signed-off-by: nolouch <nolouch@gmail.com>

codecov-commenter · 2025-03-25T07:41:50Z

Codecov Report

Attention: Patch coverage is 55.35714% with 25 lines in your changes missing coverage. Please review.

Project coverage is 50.12%. Comparing base (e8c63c6) to head (30bf980).
Report is 4 commits behind head on master.

❗ Current head 30bf980 differs from pull request most recent head 5f507af

Please upload reports for the commit 5f507af to get more accurate results.

Files with missing lines	Patch %	Lines
pkg/cluster/spec/monitoring.go	0.00%	16 Missing ⚠️
pkg/cluster/spec/grafana.go	75.00%	5 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2520      +/-   ##
==========================================
+ Coverage   45.65%   50.12%   +4.47%     
==========================================
  Files         337      337              
  Lines       36436    36465      +29     
==========================================
+ Hits        16633    18275    +1642     
+ Misses      17774    15950    -1824     
- Partials     2029     2240     +211

Flag	Coverage Δ
tiup	`33.81% <ø> (ø)`
unittest	`22.03% <55.36%> (+0.40%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

xhebox

Would you like to add related docs? LGTM

Signed-off-by: nolouch <nolouch@gmail.com>

ti-chi-bot · 2025-04-02T10:49:35Z

[LGTM Timeline notifier]

Timeline:

2025-04-01 02:36:55.196759879 +0000 UTC m=+1532708.880995967: ☑️ agreed by xhebox.
2025-04-02 10:49:33.960148519 +0000 UTC m=+1648667.644384614: ✖️🔁 reset by nolouch.

ti-chi-bot · 2025-04-02T10:49:35Z

New changes are detected. LGTM label has been removed.

xhebox · 2025-04-03T01:40:49Z

components/cluster/cluster

Remove this plz

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch · 2025-04-07T11:34:42Z

yes, I will update the document after all PR finished.

nolouch · 2025-04-09T07:05:22Z

PTAL @xhebox @XuHuaiyu

lonng · 2025-04-12T03:46:30Z

pkg/cluster/spec/grafana.go

+	if monitors[0].EnableVMRemoteWrite {
+		vmDatasource := config.NewDatasourceConfig(
+			fmt.Sprintf("%s-vm", clusterName),
+			fmt.Sprintf("http://%s", utils.JoinHostPort(monitors[0].Host, monitors[0].NgPort)),


What If TLS enabled?

I tested enabling TLS, but the monitoring service isn't using it. It seems TLS isn’t enabled for monitoring(only for scraping data).

ptal @nexustar

Signed-off-by: nolouch <nolouch@gmail.com>

XuHuaiyu · 2025-04-15T09:53:30Z

pkg/cluster/template/config/datasource.go

+func NewDatasourceConfig(clusterName, url string) *DatasourceConfig {
+	return &DatasourceConfig{
+		Name:      clusterName,
+		Type:      "prometheus",


Why the Type is also "prometheus" when NewConfig for vm?

VM compatibility with Prometheus API. I still choose to treat the VM as Prometheus to ensure that PromQL can be used.

ti-chi-bot · 2025-04-16T06:42:47Z

@XuHuaiyu: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions 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.

kaaaaaaang · 2025-04-16T08:56:34Z

/approve

ti-chi-bot · 2025-04-16T08:56:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kaaaaaaang
Once this PR has been reviewed and has the lgtm label, please assign bb7133 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cluster: add new option to set VM as a remote writer

3c3852c

Signed-off-by: nolouch <nolouch@gmail.com>

ti-chi-bot bot requested review from kaaaaaaang and nexustar March 25, 2025 07:21

ti-chi-bot bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 25, 2025

xhebox approved these changes Apr 1, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Apr 1, 2025

add datasource

52929c0

Signed-off-by: nolouch <nolouch@gmail.com>

ti-chi-bot bot removed the lgtm label Apr 2, 2025

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 2, 2025

xhebox reviewed Apr 3, 2025

View reviewed changes

components/cluster/cluster Outdated

Copy link

Collaborator

xhebox Apr 3, 2025

Choose a reason for hiding this comment

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

Remove this plz

nolouch added 2 commits April 3, 2025 10:44

address

185ba44

Signed-off-by: nolouch <nolouch@gmail.com>

update tpl

5f507af

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the forward-vm branch from 1d6148a to 5f507af Compare April 3, 2025 06:55

lonng reviewed Apr 12, 2025

View reviewed changes

nolouch force-pushed the forward-vm branch from d631b53 to e7b1d34 Compare April 15, 2025 03:21

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 15, 2025

address

826758e

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the forward-vm branch from e7b1d34 to 826758e Compare April 15, 2025 06:58

XuHuaiyu reviewed Apr 15, 2025

View reviewed changes

XuHuaiyu approved these changes Apr 16, 2025

View reviewed changes

kaaaaaaang merged commit e6ac0e9 into pingcap:master Apr 16, 2025
19 of 20 checks passed

nolouch deleted the forward-vm branch April 16, 2025 09:01

nolouch mentioned this pull request Apr 18, 2025

Support Victoria Metrics to replace the Prometheus #2531

Closed

3 tasks

cluster: add new option to set VM as a remote writer #2520

cluster: add new option to set VM as a remote writer #2520

Uh oh!

Conversation

nolouch commented Mar 25, 2025

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

codecov-commenter commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

xhebox left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Apr 2, 2025

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Apr 2, 2025

Uh oh!

xhebox Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

nolouch commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nolouch commented Apr 9, 2025

Uh oh!

lonng Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

nolouch Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolouch Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

XuHuaiyu Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

nolouch Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Apr 16, 2025

Uh oh!

kaaaaaaang commented Apr 16, 2025

Uh oh!

ti-chi-bot bot commented Apr 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Mar 25, 2025 •

edited

Loading

nolouch commented Apr 7, 2025 •

edited

Loading

nolouch Apr 14, 2025 •

edited

Loading

nolouch Apr 15, 2025 •

edited

Loading