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

Fix and update metrics in badger #1948

Merged
merged 23 commits into from Jul 18, 2023
Merged

Conversation

harshil-goel
Copy link
Contributor

Problem

Our current metrics are outdated, and some of them are not being invoked at all right now.

Solution

Added new metrics, and fixed old ones.

y/metrics.go Show resolved Hide resolved
y/metrics.go Outdated Show resolved Hide resolved
y/metrics.go Show resolved Hide resolved
Copy link

@damonfeldman damonfeldman left a comment

Choose a reason for hiding this comment

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

This looks like very important work, both extending and fixing existing metrics.

I don't read golang very well, so my comments may be off target, but thought it best to ask.

db.go Show resolved Hide resolved
db.go Show resolved Hide resolved
levels.go Show resolved Hide resolved
memtable.go Outdated Show resolved Hide resolved
y/metrics.go Outdated Show resolved Hide resolved
y/metrics.go Show resolved Hide resolved
y/metrics.go Outdated Show resolved Hide resolved
y/metrics.go Show resolved Hide resolved
Copy link

@nswamy nswamy left a comment

Choose a reason for hiding this comment

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

have a few comments/questions

y/metrics.go Outdated Show resolved Hide resolved
db.go Show resolved Hide resolved
levels.go Outdated Show resolved Hide resolved
memtable.go Outdated Show resolved Hide resolved
mangalaman93
mangalaman93 previously approved these changes Jul 12, 2023
levels.go Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

I have some pending questions from last review and added a few more questions around names. I am just trying to figure out whether they are consistent names.

metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
metrics_test.go Outdated Show resolved Hide resolved
y/metrics.go Outdated Show resolved Hide resolved
@mangalaman93 mangalaman93 merged commit ec80d3d into main Jul 18, 2023
7 checks passed
@mangalaman93 mangalaman93 deleted the harshil/change_metric_name branch July 18, 2023 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants