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

Add metrics field to autoscaler #1203

Closed
Irakus opened this issue Oct 24, 2022 · 9 comments
Closed

Add metrics field to autoscaler #1203

Irakus opened this issue Oct 24, 2022 · 9 comments
Labels
area:collector Issues for deploying collector help wanted Extra attention is needed

Comments

@Irakus
Copy link

Irakus commented Oct 24, 2022

Right now, OpenTelemetry Collector can only scale based on targetCPUUtilization field which has been replaced by metrics array. This will allow to scale on memory utilization as well.

@pavolloffay pavolloffay added area:collector Issues for deploying collector help wanted Extra attention is needed labels Oct 26, 2022
@pavolloffay
Copy link
Member

cc) @kevinearls

@kevinearls
Copy link
Member

@Irakus @pavolloffay Are there any metrics you'd like to prioritize? I've only taken an initial look at the code, but covering everything that autoscaler is capable of might be difficult.

@Irakus
Copy link
Author

Irakus commented Oct 28, 2022

@kevinearls the issue is based on this paragraph in k8 documentation Autoscaling on multiple metrics and custom metrics
It's mentioned that "Notice that the targetCPUUtilizationPercentage field has been replaced with an array called metrics." it would be nice if deployed collectors could be scaled like that. I see that RAM can be an issue when too many sources are connected to a single collector

@yuriolisa
Copy link
Contributor

@kevinearls, do we intend to add more metrics to the HPA?

@pavolloffay
Copy link
Member

If there will be use-case yes, but so far might be fine with what we have - CPU and memory metrics.

@yuriolisa
Copy link
Contributor

I'd suggest closing this issue for now, once we have enough metrics to handle the HPA. We could tackle new metrics in the future, once we have the use case.

@moh-osman3
Copy link
Contributor

moh-osman3 commented Mar 8, 2023

@yuriolisa @pavolloffay I created this new issue (#1560) to request autoscaling based on Pod custom metrics. HPA's support scaling on many different metric types, so I propose adding the necessary fields to the operator's autoscaler Spec to support the PodsMetricSourceType. Currently the HPA only supports targetCPUUtilization and targetMemoryUtilization which are ResourceMetricSourceType

@moh-osman3
Copy link
Contributor

Hi @yuriolisa @pavolloffay @iblancasa I wonder if y'all have any thoughts about supporting custom pod metrics? I have some time to work on this and want to get some input before I fully complete an initial PR for this.

My goal is to support custom pod metrics (for example scaling on otelcol_process_uptime for collector pod). In particular I'm wondering if we should deprecate targetCPUUtilization and targetMemoryUtilization in favor of the metrics array as previously mentioned. So

autoscaler:
  targetCPUUtilization: 50

Would become

autoscaler:
  metrics:
  - type: Resource
    resource:
      name: cpu
      target:
        type: Utilization
        averageUtilization: 50

This would also allow us to support the custom pod metrics I mention in #1560. However it would be a lot of work to validate all the possible metric types when we only really need the Pod and Resource Metric Types.

Another option would be to create an array specifically for Pod metrics without deprecating targetCPUUtilization and targetMemoryUtilization which are Resource metrics. So a potential config could look something like

autoscaler:
  targetCPUUtilization: 50
  targetMemoryUtilization: 70
  podMetrics:
  - pods:
     name: otelcol_process_uptime
       target:
         type: AverageValue
         averageValue: 500

However the drawback is the autoscaler config would continue to diverge from the documented Kubernetes HPA. Would it be best to just fully support the metrics array ([]MetricSpec)?

@jaronoff97
Copy link
Contributor

I believe this was closed by #1651, let me know if you have any issues or questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:collector Issues for deploying collector help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants