Skip to content
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.

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

Bug Report - Duplicate 'http://' in RestMetricsFetcher #408

Closed
kr11 opened this issue Nov 20, 2024 · 1 comment · Fixed by #421
Closed

Bug Report - Duplicate 'http://' in RestMetricsFetcher #408

kr11 opened this issue Nov 20, 2024 · 1 comment · Fixed by #421
Assignees
Labels
area/autoscaling kind/bug Something isn't working priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@kr11
Copy link
Collaborator

kr11 commented Nov 20, 2024

🐛 Describe the bug

There seem a bug of URL formatted concatenation in RestMetricsFetcher. As following codes, when FetchPodMetrics calls FetchMetric, url may contain a duplicate "http://" prefix.

// aibrix/pkg/controller/podautoscaler/metrics/fetcher.go
func (f *RestMetricsFetcher) FetchPodMetrics(ctx context.Context, pod v1.Pod, metricsPort int, metricName string) (float64, error) {
        // pass into "http://{PodIP}:{Port}", 
	return f.FetchMetric(ctx, fmt.Sprintf("http://%s:%d", pod.Status.PodIP, metricsPort), "/metrics", metricName)
}

func (f *RestMetricsFetcher) FetchMetric(ctx context.Context, endpoint string, path string, metricName string) (float64, error) {
	// HERE. url will be like "http://http://{PodIP}:{Port}/metrics", 
	url := fmt.Sprintf("http://%s/%s", strings.TrimRight(endpoint, "/"), strings.TrimLeft(path, "/"))
    // blablabla
}

Steps to Reproduce

No response

Expected behavior

No response

Environment

No response

@kr11
Copy link
Collaborator Author

kr11 commented Nov 20, 2024

@zhangjyr please help to double check it.

@Jeffwan Jeffwan added area/autoscaling triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 20, 2024
@Jeffwan Jeffwan added this to the v0.2.0 milestone Nov 20, 2024
@Jeffwan Jeffwan added the kind/bug Something isn't working label Nov 20, 2024
zhangjyr pushed a commit that referenced this issue Nov 20, 2024
…trics (#408), Add abstractMetricsFetcher to make CustomMetricsFetcher, ResourceMetricsFetcher, and KubernetesMetricsFetcher comply MetricsFetcher.
Jeffwan pushed a commit that referenced this issue Nov 22, 2024
* 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 <[email protected]>
Jeffwan pushed a commit that referenced this issue Nov 27, 2024
* Integrate Vidur as a LLM simulator.

* Test deployment

* Fix debug log output
Support next request claim.

* Integrate gpu optimizer server that exposes customized pod metrics to guide autoscaling.
Update reconciler to support metricSources.

* bug fix: autoscaler with metricsSources now works.

* Decoupled workload monitoring and visualizer
Integrated visualizer

* Integrate ILP solver and profile to GPU optimizer. Aggregated traces are supported now.

* Add support for minimum replicas.

* Debugged GPU profile benchmark, generation, and loading from file.

* Add redis support for profile exchange.

* bug fix: Duplicate 'http://' on calling RestMetricsFetcher.FetchPodMetrics (#408), Add abstractMetricsFetcher to make CustomMetricsFetcher, ResourceMetricsFetcher, and KubernetesMetricsFetcher comply MetricsFetcher.

* bug fix

* Tuning the granularity of aggregated profile

* Adjust request trace to finer granularity.
Introduce meta info and version for compatibility

* Make request trace self-explanatory on time interval

* Apply new request trace schema.

* Add Readme.md for demo walkthrough.

* Remove model cache

* Remove TargetPort changes, which is not used.

* Fix deployment

* Organize Imports

* Python 3.9 format check

* Python 3.8 format check

* Add a40 deployment

* Bug fix

* Improve benchmark stability.

* Fix python file names.
Fix python package reference to start with aibrix.gpu_optimizer
Reuse aibrix/runtime image.

* python CI test bump to 3.10

---------

Co-authored-by: Jingyuan Zhang <[email protected]>
Co-authored-by: Ning Wang <[email protected]>
gangmuk pushed a commit that referenced this issue Jan 25, 2025
* 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 <[email protected]>
gangmuk pushed a commit that referenced this issue Jan 25, 2025
* Integrate Vidur as a LLM simulator.

* Test deployment

* Fix debug log output
Support next request claim.

* Integrate gpu optimizer server that exposes customized pod metrics to guide autoscaling.
Update reconciler to support metricSources.

* bug fix: autoscaler with metricsSources now works.

* Decoupled workload monitoring and visualizer
Integrated visualizer

* Integrate ILP solver and profile to GPU optimizer. Aggregated traces are supported now.

* Add support for minimum replicas.

* Debugged GPU profile benchmark, generation, and loading from file.

* Add redis support for profile exchange.

* bug fix: Duplicate 'http://' on calling RestMetricsFetcher.FetchPodMetrics (#408), Add abstractMetricsFetcher to make CustomMetricsFetcher, ResourceMetricsFetcher, and KubernetesMetricsFetcher comply MetricsFetcher.

* bug fix

* Tuning the granularity of aggregated profile

* Adjust request trace to finer granularity.
Introduce meta info and version for compatibility

* Make request trace self-explanatory on time interval

* Apply new request trace schema.

* Add Readme.md for demo walkthrough.

* Remove model cache

* Remove TargetPort changes, which is not used.

* Fix deployment

* Organize Imports

* Python 3.9 format check

* Python 3.8 format check

* Add a40 deployment

* Bug fix

* Improve benchmark stability.

* Fix python file names.
Fix python package reference to start with aibrix.gpu_optimizer
Reuse aibrix/runtime image.

* python CI test bump to 3.10

---------

Co-authored-by: Jingyuan Zhang <[email protected]>
Co-authored-by: Ning Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/autoscaling kind/bug Something isn't working priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants