docs: document trace exemplars on the duration histograms (MAPCO-11496) - #16
Open
NivGreenstein wants to merge 12 commits into
Open
NivGreenstein wants to merge 12 commits into
NivGreenstein wants to merge 12 commits into
Conversation
The page said `[tracing]` and `[observer]` "have nothing to say to each other", which exemplars falsify: with both enabled the duration histograms carry the active trace, so a latency spike in Grafana is one click from the trace behind it. That claim is now scoped to what is still true — neither section implies the other, and tracing publishes no metric family of its own. The new section carries what an operator has to get right, because all three requirements fail quietly: the scrape format, exemplar storage on the server, and the datasource link. And the sampling asymmetry with log records, which looks like an inconsistency until the reason is stated — an exemplar is only a link, and prometheus keeps one per bucket. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four pages describe something exemplars touch. The logging page names the same two ids and now says where they differ; the layered cache page's tier-latency section is the one exemplars are most useful on, and its bucket-identity warning has a second instalment now that the le values changed again; the /metrics endpoint answers in a different format; and the tracing config section claimed independence from the observer. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The claim was wrong in the direction that understates an upgrade break. The client appends ".0" only when the shortest 'g' rendering contains neither "." nor "e", so 2.5 was never affected — and the response-size families are, even though they carry no exemplars, because the format is negotiated once per scrape rather than per family. Both pages now carry the derived table, with the two counterintuitive parts named rather than left to be noticed. The code repo derives the same list in a test, so these pages cannot drift from the bucket sets again. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page said a push_url deployment "pushes through the classic text format to a Pushgateway, which has no notion of them". The push client actually sends protobuf, which does carry exemplars — so the page was wrong about the mechanism and asserted a conclusion this repo cannot verify. It now says what is checkable: the exemplars go out on the wire, whether they are stored and re-exposed is the Pushgateway's business, Shigola does not test it, and push_url is meant for ephemeral jobs rather than the serving path the section describes. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ea98942 was committed with `git add -A` and swept up three files that were sitting uncommitted in the working tree and have nothing to do with MAPCO-11496: a new .github/workflows/preview.yaml, a rewrite of .github/workflows/gh-pages.yml, and a wholesale reformat of docusaurus.config.js carrying a baseUrl change. None of it was mine to commit, the commit message described none of it, and it changed the Pages deploy topology inside a docs-wording PR. This restores all three to origin/master. The content is not lost — it is in ea98942, and has been put back in the working tree as the uncommitted work it was. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The section named the respelled boundaries in prose, which made a fifth hand-maintained copy of a list only one place derives. It now says both families are affected and links to the table. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Two limits" said neither, while layered-cache.md linked to it as "which boundaries exactly". The heading now names the changed le labels and the pushed metrics, and the inbound link follows it. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page framed the OpenMetrics break as le-only over Shigola's own families. It belongs to the encoder rather than to histograms, which writes summary quantile labels through the same formatter — so go_gc_duration_seconds, a Go runtime metric Shigola never touches and every Go service publishes, has quantile="0" respelled to "0.0". A dashboard pinned to either is affected and the page did not say so. MAPCO-11496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page said every duration observation inside a sampled trace carries its trace and span. Two do not: shigola_mvt_provider_sql_query_seconds and shigola_provider_sql_query_seconds are measured inside their own query span and attach nothing, because a provider cannot reach exemplarFrom without importing the Prometheus observer, which the noPrometheusObserver build tag exists to compile out. Their le boundaries move all the same — 1, 5 and 20 — and the respelled table did not list them either, which understated the upgrade break for anyone running the PostGIS provider. Both corrected. Pairs with shigola's tracing/README.md and observability/prometheus/README.md (MAPCO-11496). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three write-ups framed the respelling as a consequence of exemplars — one of them literally "when the metrics route began negotiating OpenMetrics for exemplars" — and exemplars are an off-by-default feature. The switch is not conditional on them: the metrics route negotiates OpenMetrics whenever the observer is enabled. So the operator most likely to be caught out is the one running metrics with tracing off, who reads the tracing page's warning and concludes it is not about them. Said plainly on all three pages, and the /metrics row now names the break rather than only the exemplars. Pairs with shigola's observability/prometheus/README.md (MAPCO-11496). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shigola_provider_sql_query_seconds is registered but never observed, so it emits no family at all. This page listed it among the respelled le series and said its latency was readable as its query span's duration; neither is true, and both arrived on this branch two commits ago. Its sibling shigola_mvt_provider_sql_query_seconds is the one that is really observed, and its row is unchanged. Pairs with shigola's observability/prometheus/README.md (MAPCO-11496). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit cut shigola_provider_sql_query_seconds from the respelled table but left the paragraph beneath it plural, still claiming the provider query families' le labels move. Only one of them has le labels at all. Pairs with shigola's observability/prometheus/README.md (MAPCO-11496). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs half of MAPCO-11496. Code: MapColonies/shigola#24
The duration histograms now carry the active trace as a Prometheus exemplar, so a latency spike is one click from the trace behind it.
What changed, and one retraction
docs/tracing.mdsaid[tracing]and[observer]"have nothing to say to each other". Exemplars falsify that, so the claim is now scoped to what remains true: neither section implies the other, and tracing still publishes no metric family of its own. The page's new### Trace exemplarsand### Wiring it up in Grafanasections are the substance.Everything an operator has to get right is stated, because all three requirements fail quietly:
--enable-feature=exemplar-storage, or Mimir's equivalent;trace_idtargeting Tempo, or the dots render with nothing behind them.The
exemplarTraceIdDestinationssnippet is included, so this is copy-pasteable rather than a description of a thing to go and find.The asymmetry that looks like a bug
Exemplars are attached only for sampled traces; log records carry ids either way. Stated as a
:::warningon the tracing page and cross-referenced from the logging page, because encountering one convention after the other reads as an inconsistency until the reason is given: an exemplar is only ever a link, and Prometheus keeps one per bucket, overwritten by the next observation — so unfiltered, clicking a bucket would open nothing ~99 times out of 100. A log line's trace id still groups that request's lines whether or not Tempo kept the trace.Breaking change, recorded twice
Negotiating OpenMetrics respells boundaries that render as whole numbers:
le="1"is nowle="1.0". The tracing page carries the full table under### Two limits, anddocs/layered-cache.mdgets a second instalment of its bucket-identity warning — where the MAPCO-11495 bucket change already warned thatleseries do not line up across the upgrade. Someone reading only that warning would otherwise think it was over.Two things in that table are counterintuitive, and the first draft of these docs got both wrong:
2.5is not affected (the client appends.0only when the shortest rendering contains neither.nore), and the response-size families are, despite carrying no exemplars, because the format is negotiated per scrape rather than per family. Both are called out explicitly, and the code repo now derives the list in a test so these pages cannot drift from it.Cross-references
Four other pages describe something exemplars touch:
logging.md(same two ids, now says where they differ),layered-cache.md(its tier-latency section is what exemplars are most useful on),http-endpoints.md(/metricsanswers in a different format),configuration.md(the tracing section claimed independence from[observer]).Verified
npm run buildclean, which is the real check — it throws on broken links and anchors, and this adds several of both, including a deep link tolayered-cache.md#tier-latency-and-why-it-used-to-look-identical-everywhere.Correction after review
The page originally said a
push_urldeployment "pushes through the classic text format to a Pushgateway, which has no notion of them". That was wrong on the mechanism — the push client sends protobuf, which does carry exemplars — and asserted a conclusion about Pushgateway that this repo cannot verify. It now states only what is checkable, and says explicitly that Shigola does not test it.🤖 Generated with Claude Code