Skip to content

Commit

Permalink
[Bug]: duplicated http in rest metrics fetcher (#408) (#421)
Browse files Browse the repository at this point in the history
* bug fix: Duplicate 'http://' on calling RestMetricsFetcher.FetchPodMetrics (#408), Add abstractMetricsFetcher to make CustomMetricsFetcher, ResourceMetricsFetcher, and KubernetesMetricsFetcher comply MetricsFetcher.

* bug fix

---------

Co-authored-by: Jingyuan Zhang <jingyuan.zhang0929@bytedance.com>
  • Loading branch information
zhangjyr and Jingyuan Zhang authored Nov 22, 2024
1 parent 56b2fde commit c1e4c76
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions pkg/controller/podautoscaler/metrics/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,23 @@ type MetricFetcher interface {
FetchMetric(ctx context.Context, endpoint string, path string, metricName string) (float64, error)
}

type abstractMetricsFetcher struct{}

func (f *abstractMetricsFetcher) FetchMetric(ctx context.Context, pod v1.Pod, metricsPort int, metricName string) (float64, error) {
return 0.0, fmt.Errorf("not implemented")
}

// RestMetricsFetcher implements MetricFetcher to fetch metrics from Pod's /metrics endpoint.
type RestMetricsFetcher struct{}

func (f *RestMetricsFetcher) FetchPodMetrics(ctx context.Context, pod v1.Pod, metricsPort int, metricName string) (float64, error) {
// Use http to fetch pod's /metrics endpoint and parse the value
return f.FetchMetric(ctx, fmt.Sprintf("http://%s:%d", pod.Status.PodIP, metricsPort), "/metrics", metricName)
// Use /metrics to fetch pod's endpoint
return f.FetchMetric(ctx, fmt.Sprintf("%s:%d", pod.Status.PodIP, metricsPort), "/metrics", metricName)
}

func (f *RestMetricsFetcher) FetchMetric(ctx context.Context, endpoint string, path string, metricName string) (float64, error) {

// We should use the primary container port. In future, we can decide whether to use sidecar container's port
url := fmt.Sprintf("http://%s/%s", strings.TrimRight(endpoint, "/"), strings.TrimLeft(path, "/"))
// Use http to fetch endpoint
url := fmt.Sprintf("http://%s/%s", endpoint, strings.TrimLeft(path, "/"))

// Create request with context, so that the request will be canceled if the context is canceled
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
Expand Down Expand Up @@ -97,6 +102,7 @@ func (f *RestMetricsFetcher) FetchMetric(ctx context.Context, endpoint string, p

// ResourceMetricsFetcher fetches resource metrics from Kubernetes metrics API (metrics.k8s.io).
type ResourceMetricsFetcher struct {
abstractMetricsFetcher
metricsClient *versioned.Clientset
}

Expand Down Expand Up @@ -124,6 +130,7 @@ func (f *ResourceMetricsFetcher) FetchPodMetrics(ctx context.Context, pod v1.Pod

// CustomMetricsFetcher fetches custom metrics from Kubernetes' native Custom Metrics API.
type CustomMetricsFetcher struct {
abstractMetricsFetcher
customMetricsClient custom_metrics.CustomMetricsClient
}

Expand Down Expand Up @@ -157,6 +164,7 @@ func (f *CustomMetricsFetcher) FetchPodMetrics(ctx context.Context, pod v1.Pod,
}

type KubernetesMetricsFetcher struct {
abstractMetricsFetcher
resourceFetcher *ResourceMetricsFetcher
customFetcher *CustomMetricsFetcher
}
Expand Down

0 comments on commit c1e4c76

Please sign in to comment.