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 the postgres prom GC metrics to respect enable prom option #197

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

jakedt
Copy link
Member

@jakedt jakedt commented Oct 15, 2021

Signed-off-by: Jake Moshenko jacob.moshenko@gmail.com

Signed-off-by: Jake Moshenko <jacob.moshenko@gmail.com>
@github-actions github-actions bot added the area/datastore Affects the storage system label Oct 15, 2021
err = prometheus.Register(gcTransactionsClearedGauge)
if err != nil {
return nil, fmt.Errorf(errUnableToInstantiate, err)
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to make this a runtime error or just use promauto?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know... it's not clear to me when prom registering can fail just based on the API. It's going to bubble up to a fatal in our binary anyway but probably with better wrapping.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think this is fine, given it'll bubble up almost immediately if an error is returned here

Copy link
Member

@josephschorr josephschorr left a comment

Choose a reason for hiding this comment

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

LGTM

@jakedt jakedt merged commit 2ea1f2e into main Oct 19, 2021
@jakedt jakedt deleted the postgres-prom branch October 19, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datastore Affects the storage system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants