Add tracing spans to promql#4436
Conversation
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
|
|
||
| queueTimer.Stop() | ||
| ng.metrics.queryQueueTime.Observe(queueTimer.ElapsedTime().Seconds()) | ||
| queueSpanTimer.Finish() |
There was a problem hiding this comment.
Is it a problem that the if the gate.Start fails, this won't be stopped?
There was a problem hiding this comment.
Hmm, yep. But it wasn't stopped before either....
promql/engine.go
Outdated
| // spanTimer unifies tracing and timing, to reduce repetition. | ||
| type spanTimer struct { | ||
| timer *stats.Timer | ||
| observers []prometheus.Summary |
There was a problem hiding this comment.
Make the type of this []prometheus.Observer or []prometheus.ObserverFunc.
promql/engine.go
Outdated
| span opentracing.Span | ||
| } | ||
|
|
||
| func newSpanTimer(ctx context.Context, operation string, timer *stats.Timer, observers ...prometheus.Summary) (*spanTimer, context.Context) { |
There was a problem hiding this comment.
Since there are no counter stats that we do not want to trace, how about moving all the tracing code directly into query_stats.go in the stats package? Then you can also associate span names with the existing counter type constants there instead of having the caller determine that in the engine.
promql/engine.go
Outdated
| defer querier.Close() | ||
| } | ||
| { | ||
| // New block because we want to throw away this ctx. Propagating it further makes every call a child of prepare in traces. |
There was a problem hiding this comment.
Or just call it ctxPrepare / prepareCtx?
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
|
@juliusv Done. Could you take another look? |
|
👍 Thanks. (I think in the long term it'd make sense to move the promql-specific timer names back to the promql package, but keep the generic timer code in a separate package) |
3 participants
@tomwilkie @bboreham This'll help in prometheus when we add proper tracing support, but currently we'd like to use this for Cortex.