Skip to content

support https for metric dumper #489

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.

Sign up for GitHub

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

Merged
kaaaaaaang merged 2 commits into from
Nov 26, 2025
Merged

support https for metric dumper #489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b21972
support https for metric dumper
kaaaaaaang Nov 25, 2025
7873228
fix
kaaaaaaang Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions collector/prometheus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,25 +116,26 @@ func (c *AlertCollectOptions) Collect(m *Manager, topo *models.TiDBCluster) erro
monitors = append(monitors, eps.([]string)...)
} else {
for _, prom := range topo.Monitors {
monitors = append(monitors, fmt.Sprintf("%s:%d", prom.Host(), prom.MainPort()))
// todo: check is TLS enabled in tiup deployed prometheus
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

[nitpick] The TODO comment mentions checking if TLS is enabled for tiup deployed prometheus, but there's no mechanism to actually detect or handle this. The code currently hardcodes http:// as the protocol. Consider implementing the TLS detection or document the expected behavior when TLS is enabled (e.g., users should manually configure endpoints with https:// via AttrKeyPromEndpoint).

Suggested change
// todo: check is TLS enabled in tiup deployed prometheus
// TODO: If Prometheus is deployed with TLS enabled, users must manually configure endpoints with `https://` via `AttrKeyPromEndpoint`.

Copilot uses AI. Check for mistakes.
monitors = append(monitors, fmt.Sprintf("http://%s:%d", prom.Host(), prom.MainPort()))
}
}

var queryOK bool
var queryErr error
for _, promAddr := range monitors {
if err := ensureMonitorDir(c.resultDir, subdirAlerts, strings.ReplaceAll(promAddr, ":", "-")); err != nil {
if err := ensureMonitorDir(c.resultDir, subdirAlerts, utils.URL2Name(promAddr)); err != nil {
return err
}

client := &http.Client{Timeout: time.Second * time.Duration(c.opt.APITimeout)}
resp, err := client.PostForm(fmt.Sprintf("http://%s/api/v1/query", promAddr), url.Values{"query": {"ALERTS"}})
resp, err := client.PostForm(fmt.Sprintf("%s/api/v1/query", promAddr), url.Values{"query": {"ALERTS"}})
if err != nil {
return err
}
defer resp.Body.Close()

f, err := os.Create(filepath.Join(c.resultDir, subdirMonitor, subdirAlerts, strings.ReplaceAll(promAddr, ":", "-"), "alerts.json"))
f, err := os.Create(filepath.Join(c.resultDir, subdirMonitor, subdirAlerts, utils.URL2Name(promAddr), "alerts.json"))
if err == nil {
queryOK = true
} else {
Expand Down Expand Up @@ -231,9 +232,10 @@ func (c *MetricCollectOptions) Prepare(m *Manager, topo *models.TiDBCluster) (ma
return nil, err
}
c.stopChans = append(c.stopChans, stopChan)
c.endpoint = fmt.Sprintf("127.0.0.1:%d", port)
c.endpoint = fmt.Sprintf("http://127.0.0.1:%d", port)
} else {
c.endpoint = fmt.Sprintf("%s:%d", prom.Host(), prom.MainPort())
// todo: detect TLS enabled from tiup
c.endpoint = fmt.Sprintf("http://%s:%d", prom.Host(), prom.MainPort())
}
} else {
m.logger.Warnf("No Prometheus node found in topology, skip.")
Expand Down Expand Up @@ -304,7 +306,7 @@ func (c *MetricCollectOptions) Collect(m *Manager, topo *models.TiDBCluster) err
tl := utils.NewTokenLimiter(uint(qLimit))

done := 1
if err := ensureMonitorDir(c.resultDir, subdirMetrics, strings.ReplaceAll(c.endpoint, ":", "-")); err != nil {
if err := ensureMonitorDir(c.resultDir, subdirMetrics, utils.URL2Name(c.endpoint)); err != nil {
bars[key].UpdateDisplay(&progress.DisplayProps{
Prefix: fmt.Sprintf(" - Query server %s: %s", key, err),
Mode: progress.ModeError,
Expand Down Expand Up @@ -387,7 +389,10 @@ func getAPIData[T any](c *http.Client, url string, customHeader []string) (T, er
}

func makeURL(addr string, path string, queries map[string]string) string {
link := "http://" + addr + path
link := addr + path
if !strings.HasPrefix(addr, "http://") && !strings.HasPrefix(addr, "https://") {
link = "http://" + link
}
if len(queries) == 0 {
return link
}
Expand Down Expand Up @@ -501,7 +506,7 @@ func collectMetric(
func() error {
req, err := http.NewRequest(
http.MethodGet,
fmt.Sprintf("http://%s/api/v1/query?%s", promAddr, url.Values{
fmt.Sprintf("%s/api/v1/query?%s", promAddr, url.Values{
"query": {fmt.Sprintf("%s[%ds]", query, querySec)},
"time": {queryEnd.Format(time.RFC3339)},
}.Encode()),
Expand Down
8 changes: 8 additions & 0 deletions pkg/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,11 @@ func MatchPrefixs(str string, prefixs []string) bool {
}
return false
}

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The function URL2Name is missing documentation. Since this is an exported function (starts with uppercase), it should have a doc comment explaining its purpose, parameters, and return value. For example:

// URL2Name converts a URL to a filesystem-safe name by removing the protocol
// prefix (http:// or https://) and replacing special characters (/ and :) with underscores.
func URL2Name(url string) string {
Suggested change
// URL2Name converts a URL to a filesystem-safe name by removing the protocol
// prefix (http:// or https://) and replacing special characters (/ and :) with underscores.

Copilot uses AI. Check for mistakes.
func URL2Name(url string) string {
url = strings.TrimPrefix(url, "http://")
url = strings.TrimPrefix(url, "https://")
url = strings.ReplaceAll(url, "/", "_")
url = strings.ReplaceAll(url, ":", "_")
return url
}