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

Add tests to rds.go #67

Merged
merged 4 commits into from
Oct 5, 2022
Merged

Add tests to rds.go #67

merged 4 commits into from
Oct 5, 2022

Conversation

janboll
Copy link
Contributor

@janboll janboll commented Sep 23, 2022

Refactoring to enable testing, split up CollectLoop:

  • Move corrosponding code sections into individual methods
  • Make individual methods concurrent to speed up metrics collection
  • Refactor collectLogMetrics adapt concurrency to match pattern established in route53.go
  • Needed to refactor to be able to mock out AWS results. Reason being is the switch to pagination methods. Created new *All methods in awsclient.go. By adding them to the inteface, it is possible to mock out the result for these paginated methods.
  • Next refactoring was to make the fields in the RDSExporter struct simple variables, that way the tests can create simple RDSExporter objects that support only single tests cases. Otherwise, the NewRDSExporter Methods would have required some substantial refactoring.
  • In the end added a couple of basic tests
  • The last commit seems unrelated to adding tests but is required to restore the API request count. It moves the AwsExporterMetrics into the awsclient with the corresponding consequences.

Going commit by commit could make the review simpler.

related ticket: https://issues.redhat.com/browse/APPSRE-6310

Split up CollectLoop:

 * Move corrosponding code sections into individual methods
 * Make individual methods concurrent to speed up metrics collection
 * Refactor collectLogMetrics adapt concurrency to match pattern
   established in route53.go
Remove not used functions
@janboll janboll force-pushed the refactor-rds branch 4 times, most recently from 5b66423 to 9bb8d2d Compare September 29, 2022 17:17
@janboll janboll changed the title [WiP] Add tests to rds.go Add tests to rds.go Sep 29, 2022
Needed to refactor to be able to mock out AWS results. Reason being is
the switch to pagination methods. Created new *All methods in
awsclient.go. By adding them to the inteface, it is possible to mock out
the result for these paginated methods.

Next refactoring was to make the fields in the RDSExporter struct simple
variables, that way the tests can create simple RDSExporter objects that
support only single tests cases. Otherwise the NewRDSExporter Methods
would have required some substantial refactoring.
This is required to restore the metrics count for RDS requests. Since the describe methods where move to the *All Methods, we can't reliable call the number of counts in the rds.go anymore. So move the exporter to the awsclient package.
@suzana-nesic
Copy link
Contributor

lgtm, thanks!

@janboll janboll merged commit cef61ac into app-sre:master Oct 5, 2022
@janboll janboll deleted the refactor-rds branch October 5, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants