feat: implement a Keptn metrics provider #1630
Conversation
docs/gitbook/usage/metrics.md
Outdated
| query: | | ||
| keptnmetric/my-namespace/response-time |
There was a problem hiding this comment.
| query: | | |
| keptnmetric/my-namespace/response-time | |
| query: "keptnmetric/my-namespace/response-time" |
I would make this a one-liner to suggest you can't have more than one object reference in there. I would also add an example for a real Istio metric like Istio 404 rate https://docs.flagger.app/usage/metrics#prometheus
pkg/controller/controller.go
Outdated
| @@ -18,6 +18,7 @@ package controller | |||
|
|
|||
| import ( | |||
| "fmt" | |||
| "k8s.io/client-go/rest" | |||
There was a problem hiding this comment.
Please sort the imports into groups, run make fmt
bacherfl
force-pushed
the
poc/keptn-provider
branch
from
202e636
to
7e567f8
Compare
pkg/metrics/providers/keptn.go
Outdated
|
|
||
| // delete the created analysis at the end of the function | ||
| defer func() { | ||
| _ = k.client. |
There was a problem hiding this comment.
instead of ignoring the error, we should log it.
| } | ||
| return &KeptnProvider{ | ||
| client: client, | ||
| analysisTimeout: 10 * time.Second, |
There was a problem hiding this comment.
has this been chosen randomly? how does this play with the Canary analysis's interval?
There was a problem hiding this comment.
This is the maximum time the provider will wait for the result of a Keptn Analysis to be available - I added this to prevent the provider from blocking the flagger controller indefinitely in case the created Analysis cannot be completed.
I chose this value as this should be sufficient in most cases for the created Keptn Analysis to be reconciled by the Keptn Analysis controller, however I can also adjust this If you'd like, or if there are some guidelines on how long a provider might take to retrieve a value.
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
bacherfl
force-pushed
the
poc/keptn-provider
branch
from
069ebd3
to
8b93127
Compare
Thanks for the review @aryan9600! I just addressed your comments and adapted the code accordingly - If there are some further questions feel free to reach out :) |
There was a problem hiding this comment.
took a second pass, its looking good, just had some minor nits.
can the docs be improved a bit more? a few suggestions:
- the
queryfield inMetricTemplateexample mentions the timeframe and arguments as well - make it clearer that resource name is actually that of the
AnalysisDefinitionand not the to be createdAnalysis. - an example around
Analysis, which shows a sampleAnalysisDefinitionand what theAnalysisobject generated by Flagger would look like
| } | ||
|
|
||
| var analysisResource = schema.GroupVersionResource{ | ||
| Group: "metrics.keptn.sh", |
There was a problem hiding this comment.
lets define a constant for this as well
| } | ||
| } | ||
| } | ||
| return 0, fmt.Errorf("could not retrieve KeptnMetric - no value found int resource %s/%s", queryObj.Namespace, queryObj.ResourceName) |
There was a problem hiding this comment.
| return 0, fmt.Errorf("could not retrieve KeptnMetric - no value found int resource %s/%s", queryObj.Namespace, queryObj.ResourceName) | |
| return 0, fmt.Errorf("could not retrieve KeptnMetric - no value found in resource %s/%s", queryObj.Namespace, queryObj.ResourceName) |
| select { | ||
| case <-ctx.Done(): | ||
| return 0, fmt.Errorf("encountered timeout while waiting for Keptn Analysis %s/%s to be finished", obj.Namespace, obj.ResourceName) | ||
| case <-time.After(500 * time.Millisecond): |
There was a problem hiding this comment.
| case <-time.After(500 * time.Millisecond): | |
| case <-time.After(time.Second): |
as mentioned in the above comment
Closes #1627
This PR adds the implementation of a
keptnmetrics providerThe main difference to other providers in Flagger is that this one is using a Kubernetes client to interact with Keptn resources (
KeptnMetricsandAnalyses). Therefore, in addition to the code inkeptn.go, theClusterRoleneeded to be adapted to be able to access the required Keptn resources.The provider supports the use of the following resources:
KeptnMetric: This resource represents the current value of a certain metric. The metric is retrieved by Keptn from one of the following data sources:prometheus,thanos,cortex,dynatrace,datadog.Analysis: This Keptn resource can be used to apply a more complex grading logic over a set of different metrics. More information about this concept can be found in the Keptn Docs