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

feat(metrics-operator): add analysis controller #1875

Merged
merged 84 commits into from
Sep 1, 2023

Conversation

RealAnna
Copy link
Contributor

@RealAnna RealAnna commented Aug 10, 2023

this PR implements a working version, but is far from ready, this is why the feature is hidden behind a flag which is disabled in the helm chart.

This PR

  • introduce an analysis controller:
    - first attempt at parallelizing queries to provider: we have different go routines per provider kind. This means that even if the provider of kind prometheus are 4 we query one at a time.
    - we store queries results in the status to reduce queries in case of one provider failure and reconciliation

  • adapts slo computation

  • adds status to analysis CRD

  • adds unit tests and a basic integration test

  • generates all docs

TODOs:

  • refactor controller to reconcile in case of timeout and use existing state to retrieve missing objectives
  • add unit test to check this
  • merge old status with new results before calculating score
  • refactor error propagation, in case of provider error controller should error and reconcile (throw everything missing or but attempt score computation anyhow? )
  • decide status struct and adapt it
  • prometheus seems not to like range queries where the step is not defined, I added a default step, should we make it configurable? no 1min
  • adapt integration test to a proper prometheus result, also have a more meaningful test with multiple objectives and at least two providers --> made separate issue Improve analysis integration tests #1992
  • introduce a Status spec in analysis crd

this an example run with integration test not so smart prometheus query and a range of 30 sec

image

@netlify
Copy link

netlify bot commented Aug 10, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 92aa5dd
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/64f0861e9b32990008467ef4
😎 Deploy Preview https://deploy-preview-1875--keptn-lifecycle-toolkit.netlify.app/docs/crd-ref/metrics/v1alpha3
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch from b3a03f0 to 7a8d49b Compare August 11, 2023 06:56
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #1875 (b801802) into main (eb23359) will increase coverage by 0.03%.
Report is 16 commits behind head on main.
The diff coverage is 75.16%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1875      +/-   ##
==========================================
+ Coverage   84.19%   84.23%   +0.03%     
==========================================
  Files         143      149       +6     
  Lines        9039     9531     +492     
==========================================
+ Hits         7610     8028     +418     
- Misses       1161     1218      +57     
- Partials      268      285      +17     
Files Changed Coverage Δ
metrics-operator/api/v1alpha3/analysis_types.go 100.00% <ø> (ø)
...perator/api/v1alpha3/analysisdefinition_webhook.go 83.52% <ø> (ø)
...perator/controllers/common/analysis/types/types.go 100.00% <ø> (ø)
...ontrollers/common/providers/dynatrace/dynatrace.go 83.87% <0.00%> (-0.69%) ⬇️
...ollers/common/providers/dynatrace/dynatrace_dql.go 80.75% <0.00%> (ø)
...-operator/controllers/common/providers/provider.go 100.00% <ø> (ø)
...etrics-operator/controllers/analysis/controller.go 65.95% <65.95%> (ø)
...trollers/common/providers/prometheus/prometheus.go 84.04% <66.66%> (+0.17%) ⬆️
...rator/controllers/analysis/objectives_evaluator.go 77.27% <77.27%> (ø)
...operator/controllers/analysis/provider_selector.go 78.31% <78.31%> (ø)
... and 6 more

... and 4 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 68.55% <ø> (ø)
component-tests 59.48% <ø> (+0.40%) ⬆️
keptn-lifecycle-operator ?
lifecycle-operator 84.92% <ø> (ø)
metrics-operator 85.85% <75.16%> (-0.69%) ⬇️
scheduler 32.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch from 4ea5188 to 0644623 Compare August 21, 2023 11:54
metrics-operator/main.go Outdated Show resolved Hide resolved
@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch 2 times, most recently from 76f1d35 to 0ee481b Compare August 24, 2023 13:32
@github-actions github-actions bot added ops documentation Improvements or additions to documentation labels Aug 25, 2023
@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch from ae97ede to 0b96337 Compare August 25, 2023 09:04
@RealAnna RealAnna marked this pull request as ready for review August 25, 2023 10:35
@RealAnna RealAnna requested a review from a team as a code owner August 25, 2023 10:35
@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch 2 times, most recently from a3b5840 to 3cfb8c7 Compare August 28, 2023 05:42
@RealAnna RealAnna marked this pull request as draft August 28, 2023 14:01
@RealAnna RealAnna force-pushed the feat/1820/add_analysis_controller branch 2 times, most recently from 40d541a to 5c2c079 Compare August 30, 2023 10:30
Signed-off-by: realanna <anna.reale@dynatrace.com>
Signed-off-by: realanna <anna.reale@dynatrace.com>
RealAnna and others added 2 commits August 31, 2023 13:59
Co-authored-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com>
Co-authored-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com>
helm/chart/doc.yaml Outdated Show resolved Hide resolved
metrics-operator/api/v1alpha3/analysis_types.go Outdated Show resolved Hide resolved
metrics-operator/controllers/analysis/controller.go Outdated Show resolved Hide resolved
metrics-operator/controllers/analysis/controller.go Outdated Show resolved Hide resolved
metrics-operator/controllers/analysis/controller.go Outdated Show resolved Hide resolved
RealAnna and others added 6 commits August 31, 2023 14:06
Co-authored-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com>
Signed-off-by: realanna <anna.reale@dynatrace.com>
Signed-off-by: realanna <anna.reale@dynatrace.com>
…ecting timeout

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants