-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(promhttp): add CoalesceGather option to deduplicate concurrent Gather calls #1969
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
base: main
Are you sure you want to change the base?
Changes from all commits
b3123ec
4a2c8ea
71e5c81
4f8159a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,10 +37,12 @@ import ( | |
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "slices" | ||
| "strconv" | ||
| "sync" | ||
| "time" | ||
|
|
||
| dto "github.com/prometheus/client_model/go" | ||
| "github.com/prometheus/common/expfmt" | ||
|
|
||
| "github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil" | ||
|
|
@@ -79,6 +81,113 @@ var gzipPool = sync.Pool{ | |
| }, | ||
| } | ||
|
|
||
| // coalescingGatherer wraps a TransactionalGatherer to deduplicate concurrent | ||
| // Gather calls. When a Gather is already in flight, new callers join the | ||
| // existing cycle and receive the same result once it completes. The underlying | ||
| // done function is called exactly once, when the last joined caller releases. | ||
| // | ||
| // This prevents goroutine pile-up when the scrape rate is faster than the | ||
| // time collectors need to produce metrics. | ||
| type coalescingGatherer struct { | ||
| g prometheus.TransactionalGatherer | ||
| mu sync.Mutex | ||
| cycle *gatherCycle | ||
| } | ||
|
|
||
| // gatherCycle tracks a single in-flight Gather and all HTTP handlers sharing it. | ||
| type gatherCycle struct { | ||
| ready chan struct{} // closed when Gather completes; happens-before reads of mfs/err/done | ||
| mfs []*dto.MetricFamily // canonical result, set before ready is closed; callers get a slices.Clone, the element values stay shared and must not be mutated | ||
| err error // set before ready is closed | ||
| done func() // underlying done callback; set before ready is closed | ||
| refs int // number of handlers using this cycle; protected by coalescingGatherer.mu | ||
| } | ||
|
|
||
| var _ prometheus.TransactionalGatherer = (*coalescingGatherer)(nil) // compile-time interface check | ||
|
|
||
| // errGatherPanicked is returned to callers that joined an in-flight coalesced | ||
| // Gather whose underlying gatherer panicked. See the panic guard in Gather for | ||
| // why joiners receive this error instead of the panic itself. | ||
| var errGatherPanicked = errors.New("coalesced gather panicked") | ||
|
|
||
| func (c *coalescingGatherer) Gather() ([]*dto.MetricFamily, func(), error) { | ||
| c.mu.Lock() | ||
| if cy := c.cycle; cy != nil { | ||
| // c.cycle is non-nil while Gather runs or handlers are still consuming its results. | ||
| cy.refs++ | ||
| c.mu.Unlock() | ||
| <-cy.ready | ||
| // Each caller gets its own slice header so it can filter or reorder | ||
| // without racing other callers sharing this cycle. The *dto.MetricFamily | ||
| // values remain shared and must not be mutated in place. | ||
| return slices.Clone(cy.mfs), c.releaseFunc(cy), cy.err | ||
| } | ||
| cy := &gatherCycle{ | ||
| ready: make(chan struct{}), | ||
| done: func() {}, | ||
| refs: 1, | ||
| } | ||
| c.cycle = cy | ||
| c.mu.Unlock() | ||
|
|
||
| // Guard against a panic in c.g.Gather. The common case, a panicking | ||
| // Collector, never reaches here: Registry.Gather recovers Collector panics | ||
| // and returns them as an error. This guard only covers the rare case where | ||
| // the wrapped gatherer itself panics. | ||
| // | ||
| // We deliberately do not recover: the leader's panic propagates and is | ||
| // handled by net/http exactly as it would be without coalescing. We only | ||
| // set cy.err before closing cy.ready so joiners waiting on <-cy.ready fail | ||
| // with that error instead of silently returning an empty, successful | ||
| // response, and we clear c.cycle so the next Gather starts a fresh cycle. | ||
| // | ||
| // The leader never runs its own releaseFunc on this path, so its ref is | ||
| // not decremented; that is harmless because the cycle is detached (c.cycle | ||
| // = nil) and cy.done is still the no-op set at construction (c.g.Gather | ||
| // panicked before assigning a real done). If cy.done is ever made non-nil | ||
| // before c.g.Gather runs, this path would need to release it. | ||
| panicked := true | ||
| defer func() { | ||
| if panicked { | ||
| c.mu.Lock() | ||
| if c.cycle == cy { | ||
| c.cycle = nil | ||
| } | ||
| c.mu.Unlock() | ||
| cy.err = errGatherPanicked // set before close: happens-before joiners' reads | ||
| close(cy.ready) | ||
| } | ||
| }() | ||
| cy.mfs, cy.done, cy.err = c.g.Gather() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One question: if c.g.Gather() panics, do the joiners actually see the panic, or do they just unblock and return as if everything was ok?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit of defensive coding here. They just unblock and clean up. If there is no Feel free to challenge and propose better alternative. Would you prefer us to capture the error?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a suggestion above to capture the error. This is because I was thinking that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I set a sentinel error ( |
||
| panicked = false | ||
| close(cy.ready) // happens-before joiners' reads of cy.mfs/err/done | ||
|
|
||
| // Clone here too so cy.mfs stays the write-once canonical slice: joiners | ||
| // read it concurrently via slices.Clone, so the leader must not hand out | ||
| // (and potentially reorder) the same backing array. | ||
| return slices.Clone(cy.mfs), c.releaseFunc(cy), cy.err | ||
| } | ||
|
|
||
| // releaseFunc returns the done callback for one caller sharing cy. | ||
| // When the last caller releases, the underlying done is invoked and the | ||
| // cycle is cleared so the next Gather starts fresh. | ||
| func (c *coalescingGatherer) releaseFunc(cy *gatherCycle) func() { | ||
| return func() { | ||
| c.mu.Lock() | ||
| cy.refs-- | ||
| if cy.refs > 0 { | ||
| c.mu.Unlock() | ||
| return | ||
| } | ||
| // Last caller. | ||
| if c.cycle == cy { | ||
| c.cycle = nil | ||
| } | ||
| c.mu.Unlock() | ||
| cy.done() // called outside the lock to avoid holding it during done | ||
| } | ||
| } | ||
|
|
||
| // Handler returns an http.Handler for the prometheus.DefaultGatherer, using | ||
| // default HandlerOpts, i.e. it reports the first error as an HTTP error, it has | ||
| // no error logging, and it applies compression if requested by the client. | ||
|
|
@@ -125,6 +234,10 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { | |
| // Multiple metric names can be specified by providing the parameter multiple times. | ||
| // When no name[] parameters are provided, all metrics are returned. | ||
| func HandlerForTransactional(reg prometheus.TransactionalGatherer, opts HandlerOpts) http.Handler { | ||
| if opts.CoalesceGather { | ||
| reg = &coalescingGatherer{g: reg} | ||
| } | ||
|
|
||
| var ( | ||
| inFlightSem chan struct{} | ||
| errCnt = prometheus.NewCounterVec( | ||
|
|
@@ -428,15 +541,47 @@ type HandlerOpts struct { | |
| // Service Unavailable and a suitable message in the body. If | ||
| // MaxRequestsInFlight is 0 or negative, no limit is applied. | ||
| MaxRequestsInFlight int | ||
| // CoalesceGather, if true, deduplicates concurrent Gather calls so that | ||
| // only one collection runs at a time. Additional requests that arrive | ||
| // while a Gather is in flight will receive the same result once it | ||
| // completes. This prevents goroutine pile-up when the scrape rate is | ||
| // faster than the time collectors need to produce metrics. | ||
| // | ||
| // When enabled, concurrent scrapers share a single metric snapshot per | ||
| // collection cycle. Each request receives its own copy of the returned | ||
| // slice, so filtering or reordering it (for example via name[] query | ||
| // parameters) is safe. The pointed-to MetricFamily values are still | ||
| // shared: the built-in handler only reads them, so this is safe in | ||
| // practice, but a custom TransactionalGatherer that mutates the returned | ||
| // families in place after Gather returns must not use this option. | ||
| // | ||
| // Because the snapshot is shared, a request that arrives while a cycle is | ||
| // in flight receives that cycle's result even though collection began | ||
| // before the request; two scrapers joined to one cycle observe the same | ||
| // timestamps rather than independently gathered data. | ||
| // | ||
| // Consider using CoalesceGather together with Timeout. Timeout bounds the | ||
| // client-facing response time and keeps at most one collection running at | ||
| // a time, but it does not cancel the underlying Gather: a joined request | ||
| // that times out still holds a MaxRequestsInFlight slot until the shared | ||
| // collection completes. | ||
| // | ||
| // Panic handling: a panicking Collector is already turned into an error by | ||
| // the registry, so joiners receive that error like any other. In the rare | ||
| // case where the wrapped gatherer itself panics, the panicking request's | ||
| // panic propagates as usual (handled by net/http), while requests that | ||
| // joined the same cycle receive an error rather than an empty response. | ||
| CoalesceGather bool | ||
| // If handling a request takes longer than Timeout, it is responded to | ||
| // with 503 ServiceUnavailable and a suitable Message. No timeout is | ||
| // applied if Timeout is 0 or negative. Note that with the current | ||
| // implementation, reaching the timeout simply ends the HTTP requests as | ||
| // described above (and even that only if sending of the body hasn't | ||
| // started yet), while the bulk work of gathering all the metrics keeps | ||
| // running in the background (with the eventual result to be thrown | ||
| // away). Until the implementation is improved, it is recommended to | ||
| // implement a separate timeout in potentially slow Collectors. | ||
| // away). When CoalesceGather is enabled, only one such background Gather | ||
| // can be in flight at a time. It is also recommended to implement a | ||
| // separate timeout in potentially slow Collectors. | ||
| Timeout time.Duration | ||
| // If true, the experimental OpenMetrics encoding is added to the | ||
| // possible options during content negotiation. Note that Prometheus | ||
|
|
||
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.