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

fix: Metrics endpoint returns correct HPA value #3584

Merged
merged 7 commits into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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")
}
}