Skip to content

Commit

Permalink
fix: Metrics endpoint returns correct HPA value (#3584)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jorge Turrado Ferrero authored Aug 25, 2022
1 parent 40da2e2 commit ef48177
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 4 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md

### Fixes

- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX))
- **General:** Metrics endpoint returns correct HPA values ([#3554](https://github.com/kedacore/keda/issues/3554))

### Deprecations

Expand Down
2 changes: 2 additions & 0 deletions config/metrics-server/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ spec:
name: https
- containerPort: 8080
name: http
- containerPort: 9022
name: metrics
volumeMounts:
- mountPath: /tmp
name: temp-vol
Expand Down
3 changes: 3 additions & 0 deletions config/metrics-server/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,8 @@ spec:
- name: http
port: 80
targetPort: 8080
- name: metrics
port: 9022
targetPort: 9022
selector:
app: keda-metrics-apiserver
4 changes: 2 additions & 2 deletions pkg/metrics/prometheus_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ func (metricsServer PrometheusMetricServer) NewServer(address string, pattern st
}

// RecordHPAScalerMetric create a measurement of the external metric used by the HPA
func (metricsServer PrometheusMetricServer) RecordHPAScalerMetric(namespace string, scaledObject string, scaler string, scalerIndex int, metric string, value int64) {
scalerMetricsValue.With(getLabels(namespace, scaledObject, scaler, scalerIndex, metric)).Set(float64(value))
func (metricsServer PrometheusMetricServer) RecordHPAScalerMetric(namespace string, scaledObject string, scaler string, scalerIndex int, metric string, value float64) {
scalerMetricsValue.With(getLabels(namespace, scaledObject, scaler, scalerIndex, metric)).Set(value)
}

// RecordHPAScalerError counts the number of errors occurred in trying get an external metric used by the HPA
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (p *KedaProvider) GetExternalMetric(ctx context.Context, namespace string,
logger.Error(err, "error getting metric for scaler", "scaledObject.Namespace", scaledObject.Namespace, "scaledObject.Name", scaledObject.Name, "scaler", scaler)
} else {
for _, metric := range metrics {
metricValue, _ := metric.Value.AsInt64()
metricValue := metric.Value.AsApproximateFloat64()
metricsServer.RecordHPAScalerMetric(namespace, scaledObject.Name, scalerName, scalerIndex, metric.MetricName, metricValue)
}
matchingMetrics = append(matchingMetrics, metrics...)
Expand Down
184 changes: 184 additions & 0 deletions tests/internals/prometheus_metrics/prometheus_metrics_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
//go:build e2e
// +build e2e

package prometheus_metrics_test

import (
"fmt"
"strings"
"testing"

"github.com/prometheus/common/expfmt"
"github.com/stretchr/testify/assert"

. "github.com/kedacore/keda/v2/tests/helper"
)

const (
testName = "prometheus-metrics-test"
)

var (
testNamespace = fmt.Sprintf("%s-ns", testName)
deploymentName = fmt.Sprintf("%s-deployment", testName)
monitoredDeploymentName = fmt.Sprintf("%s-monitored", testName)
scaledObjectName = fmt.Sprintf("%s-so", testName)
clientName = fmt.Sprintf("%s-client", testName)
)

type templateData struct {
TestNamespace string
DeploymentName string
ScaledObjectName string
MonitoredDeploymentName string
ClientName string
}

const (
monitoredDeploymentTemplate = `
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{.MonitoredDeploymentName}}
namespace: {{.TestNamespace}}
labels:
app: {{.MonitoredDeploymentName}}
spec:
replicas: 4
selector:
matchLabels:
app: {{.MonitoredDeploymentName}}
template:
metadata:
labels:
app: {{.MonitoredDeploymentName}}
spec:
containers:
- name: {{.MonitoredDeploymentName}}
image: nginx
`

deploymentTemplate = `
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{.DeploymentName}}
namespace: {{.TestNamespace}}
labels:
app: {{.DeploymentName}}
spec:
replicas: 1
selector:
matchLabels:
app: {{.DeploymentName}}
template:
metadata:
labels:
app: {{.DeploymentName}}
spec:
containers:
- name: {{.DeploymentName}}
image: nginx
`

scaledObjectTemplate = `
apiVersion: keda.sh/v1alpha1
kind: ScaledObject
metadata:
name: {{.ScaledObjectName}}
namespace: {{.TestNamespace}}
spec:
scaleTargetRef:
name: {{.DeploymentName}}
pollingInterval: 5
idleReplicaCount: 0
minReplicaCount: 1
maxReplicaCount: 2
cooldownPeriod: 10
triggers:
- type: kubernetes-workload
metadata:
podSelector: 'app={{.MonitoredDeploymentName}}'
value: '1'
`

clientTemplate = `
apiVersion: v1
kind: Pod
metadata:
name: {{.ClientName}}
namespace: {{.TestNamespace}}
spec:
containers:
- name: {{.ClientName}}
image: curlimages/curl
command:
- sh
- -c
- "exec tail -f /dev/null"`
)

func TestScaler(t *testing.T) {
// setup
t.Log("--- setting up ---")

// Create kubernetes resources
kc := GetKubernetesClient(t)
data, templates := getTemplateData()

CreateKubernetesResources(t, kc, testNamespace, data, templates)

// scaling to max replica count to ensure the counter is registered before we test it
assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, 2, 60, 2),
"replica count should be 2 after 2 minute")

testHPAScalerMetricValue(t)

// cleanup
DeleteKubernetesResources(t, kc, testNamespace, data, templates)
}

func getTemplateData() (templateData, []Template) {
return templateData{
TestNamespace: testNamespace,
DeploymentName: deploymentName,
ScaledObjectName: scaledObjectName,
MonitoredDeploymentName: monitoredDeploymentName,
ClientName: clientName,
}, []Template{
{Name: "deploymentTemplate", Config: deploymentTemplate},
{Name: "monitoredDeploymentTemplate", Config: monitoredDeploymentTemplate},
{Name: "scaledObjectTemplate", Config: scaledObjectTemplate},
{Name: "clientTemplate", Config: clientTemplate},
}
}

func testHPAScalerMetricValue(t *testing.T) {
t.Log("--- testing hpa scaler metric value ---")

out, _, err := ExecCommandOnSpecificPod(t, clientName, testNamespace, "curl --insecure http://keda-metrics-apiserver.keda:9022/metrics")
assert.NoErrorf(t, err, "cannot execute command - %s", err)

parser := expfmt.TextParser{}
// Ensure EOL
reader := strings.NewReader(strings.ReplaceAll(out, "\r\n", "\n"))
family, err := parser.TextToMetricFamilies(reader)
assert.NoErrorf(t, err, "cannot parse metrics - %s", err)

if val, ok := family["keda_metrics_adapter_scaler_metrics_value"]; ok {
var found bool
metrics := val.GetMetric()
for _, metric := range metrics {
labels := metric.GetLabel()
for _, label := range labels {
if *label.Name == "scaledObject" && *label.Value == scaledObjectName {
assert.Equal(t, float64(4), *metric.Gauge.Value)
found = true
}
}
}
assert.Equal(t, true, found)
} else {
t.Errorf("metric not available")
}
}

0 comments on commit ef48177

Please sign in to comment.