Distributor: small tracing improvements #8613
Conversation
| span, ctx := opentracing.StartSpanFromContext(ctx, "Distributor.sendWriteRequestToIngesters") | ||
| span.SetTag("write.series", len(req.Timeseries)) | ||
| defer span.Finish() |
There was a problem hiding this comment.
The functions inside ring.DoBatchWithOptions here do some funky dancing with the context (note the remoteRequestContext). Are you sure this span will be linked as the parent of those that are created down the c.Push calls below?
There was a problem hiding this comment.
That's a good call-out, but I think the code I wrote is ok.
remoteRequestContext() creates a child context of ctx so things ought to look OK in the trace viewer.
And remoteRequestContext() is intended to be called once the first call is ready, so we don't want to do it at the beginning of this function.
There was a problem hiding this comment.
Only to double-check: looking at an example of a trace below 👇 I believe, the changes here will put the new Distributor.sendWriteRequestToIngesters span to be parallel to GEM's Distributor.biPushWrapper and the first /cortex.Ingester/Push (i.e. won't make the /cortex.Ingester/Push spans the children of the sendWriteRequestToIngesters span).
Is this still fine? Because this seems going against to what you mentioned in the description ("This helps to group potentially hundreds of spans when viewing a trace")
There was a problem hiding this comment.
Probably best not to discuss GEM in the public repo, but I am removing that span in the PR you can see linked.
Add spans for sending to ingesters and partitions. This helps to group potentially hundreds of spans when viewing a trace. Also break out the number of histograms and put that as a tag on the trace; it can be of interest.
bboreham
force-pushed
the
distributor-tracing
branch
from
986b224 to
a513435
Compare
|
The CHANGELOG has just been cut to prepare for the next release. Please rebase |
What this PR does
Add spans for sending to ingesters and partitions. This helps to group potentially hundreds of spans when viewing a trace.
Also break out the number of histograms and put that as a tag on the trace; it can be of interest.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.