-
Notifications
You must be signed in to change notification settings - Fork 285
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
More spanner observability / Go Scheduler metrics #1609
Conversation
vroldanbet
commented
Oct 24, 2023
•
edited
Loading
edited
- adds opentelemetry integration into Spanner client
- exposes Spanner client metrics via prometheus
- enables All Go runtime metrics (including scheduler)
2788119
to
65b5e32
Compare
This caused an incident as it basically doubles Spanner CPU usage This reverts commit 75b8f9f.
this replaces prometheus client_golang default collector with one that includes Go scheduler metrics
81e6af5
to
21b85c7
Compare
// | ||
// in order to register this, the package must be imported anonymously | ||
func init() { | ||
prometheus.DefaultRegisterer.Unregister(collectors.NewGoCollector()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nice, I spent a bunch of time trying to register the scheduler metrics but they kept including metrics that conflicted with existing ones as a side-effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spent most of the time dealing with the same 😅 We could also create a new promhttp handler with a Gatherer
, but would require a different HTTP endpoint and that's a mess. The reason we have to do this is because we rely heavily on the default prometheus Registry - we may want to revisit that.