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

Update metrics that missed labels #866

Merged
merged 17 commits into from
Jan 16, 2024
Merged

Conversation

opan
Copy link
Contributor

@opan opan commented Jan 2, 2024

Update for PR #857

@coveralls
Copy link

coveralls commented Jan 2, 2024

Pull Request Test Coverage Report for Build 255

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 91.963%

Totals Coverage Status
Change from base Build 247: -0.1%
Covered Lines: 1968
Relevant Lines: 2140

💛 - Coveralls

@opan
Copy link
Contributor Author

opan commented Jan 2, 2024

This update is required. Without the labels, the metrics won't show up in /metrics. I have tested this locally. I will try to add some tests for this.

@oliver006
Copy link
Owner

Thanks.
PR looks good but this time let's add tests to make sure the metrics really get recorded.

@opan
Copy link
Contributor Author

opan commented Jan 8, 2024

I have try to add the test in here.

If this does not suffice, I will try to add more test regarding this.

@oliver006
Copy link
Owner

Ok, thanks, let me have another look.

Copy link
Owner

@oliver006 oliver006 left a comment

Choose a reason for hiding this comment

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

PR looks good - just one Q about the docker-compose changes.

contrib/docker-compose-for-tests.yml Outdated Show resolved Hide resolved
contrib/docker-compose-for-tests.yml Outdated Show resolved Hide resolved
@oliver006 oliver006 merged commit 809380b into oliver006:master Jan 16, 2024
4 of 5 checks passed
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.

4 participants