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

refactor: locally scope s3 metrics #39

Merged
merged 1 commit into from
Dec 7, 2023
Merged

refactor: locally scope s3 metrics #39

merged 1 commit into from
Dec 7, 2023

Conversation

skl
Copy link
Contributor

@skl skl commented Dec 7, 2023

  • Fix typo: NextScrapeGuage is now NextScrapeGauge
  • Refactored package-level metrics instances into a new Metrics type, which is recreated on each test case
  • Updated all test cases to include all metrics except NextScrapeGuage, now that request errors and total metric are independent.
    • N.B. results with two pages case now shows 2 requests

@skl skl requested review from Pokom and the-it December 7, 2023 14:40
Copy link
Contributor

@Pokom Pokom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this is a nice cleanup! It's a big diff but pretty clear what's happening. I've taken a few attempts at refactoring how we emit metrics to passing in a channel and emitting new const metrics to the channel, this will make it much easier for that refactor to happen since we have tests 😄

@skl skl merged commit 77ac2aa into main Dec 7, 2023
1 check passed
@skl skl deleted the skl/testutil-isolation branch December 7, 2023 14:57
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