ClusterLoader - Adding api reposiveness prometheus to tests #493
Conversation
krzysied
commented
Apr 11, 2019
krzysied
commented
- Making api responsiveness measurement non-failable.
- Addinf api responsiveness measurement to density and load test.
k8s-ci-robot
added
the
cncf-cla: yes
|
/assign @wojtek-t |
k8s-ci-robot
added
approved
| @@ -67,6 +67,12 @@ type apiResponsivenessMeasurementPrometheus struct { | |||
| func (a *apiResponsivenessMeasurementPrometheus) Execute(config *measurement.MeasurementConfig) ([]measurement.Summary, error) { | |||
| var summaries []measurement.Summary | |||
|
|
|||
| if config.PrometheusFramework == nil { | |||
| klog.Errorf("%s: prometheus framework is not provided!") | |||
| // TODO(krzysied): for the testing purpose metric is not returning error. | |||
There was a problem hiding this comment.
Instead of TODO(krzysied), please open an issue and do TODO(#issue)
| return metrics, errors.NewMetricViolationError("top latency metric", fmt.Sprintf("there should be no high-latency requests, but: %v", badMetrics)) | ||
| } | ||
| return metrics, nil | ||
| // TODO(krzysied): For testing purpose this metric will never return metric violation error. |
krzysied
force-pushed
the
cluster_loader_api_reposiveness_prometheus_nonblocking
branch
from
d4b6732
to
0d07406
Compare
clusterloader2/pkg/measurement/common/slos/api_responsiveness_prometheus.go
Show resolved
Hide resolved
| // TODO(#498): For testing purpose this metric will never return metric violation error. | ||
| // The code below should be | ||
| // if len(badMetrics) > 0 { | ||
| // return metrics, errors.NewMetricViolationError("top latency metric", fmt.Sprintf("there should be no high-latency requests, but: %v", badMetrics)) |
There was a problem hiding this comment.
apiResponsivenessPrometheus{metric}
| - Identifier: APIResponsivenessPrometheus | ||
| Method: APIResponsivenessPrometheus | ||
| Params: | ||
| action: start |
There was a problem hiding this comment.
We're getting to the point when it makes sense to introduce the "slo bundle" metric. Soon we will have network-programming latency SLI in this categoy, and more is comming - I don't want to grows those configs that much.
Probably a separate PR though.
There was a problem hiding this comment.
I will create separate PR.
krzysied
force-pushed
the
cluster_loader_api_reposiveness_prometheus_nonblocking
branch
from
0d07406
to
a1ae777
Compare
| } | ||
|
|
||
| // SummaryTime returns time when summary was created. | ||
| func (a *apiResponsivenessPrometheus) SummaryTime() time.Time { |
There was a problem hiding this comment.
For consistency with all other metrics - can you move it above PrintSummary?
krzysied
force-pushed
the
cluster_loader_api_reposiveness_prometheus_nonblocking
branch
from
a1ae777
to
a6cae43
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krzysied, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
krzysied
deleted the
cluster_loader_api_reposiveness_prometheus_nonblocking
branch
