feat(foundations): use foundations-metrics as the macro backend + expose a foundations-metrics facade - #240
Draft
ethanolchik wants to merge 51 commits into
Draft
feat(foundations): use foundations-metrics as the macro backend + expose a foundations-metrics facade#240ethanolchik wants to merge 51 commits into
ethanolchik wants to merge 51 commits into
Conversation
Add owned fixed-bucket storage, direct protobuf encoding, and support for the legacy HistogramBuilder API (only classic histograms in this commit).
Choose to pre-sort histogram buckets over a sorted boolean variable. Also satisfies cargo clippy & cargo fmt
…omplete, non-empty names
… names Legacy metric and label names must match Prometheus' name grammars. Names outside these grammars cause Prometheus to reject the scrape, and the grammars exclude non-ASCII characters. Foundations now quotes label names and uses OpenMetrics' quoted metric-name syntax when required. This allows UTF-8 and other nonstandard names without affecting legacy compatibility.
Add registerable counter, classic histogram, and native histogram types that retain exemplars alongside observations. Support typed and legacy label sets, Family usage, OpenMetrics text, and Prometheus protobuf encoding. Validate exemplars during collection and report serialisation failures as non-fatal diagnostics.
`nan` and `inf` parse as floats, so an unranked weight reached the comparison: `NaN` lost none of them and pinned itself as the winner, and `inf` outranked a legitimate `q=1.0`. Bound the weight to the range RFC 9110 defines instead.
`CounterWithExemplar::get` and `inner` returned a `parking_lot::MappedRwLockReadGuard`, so naming either return type meant depending on `parking_lot` directly, which RFC-047 requires a reexport for. Generalise the guard `Family` already wrapped instead, so no lock type reaches the public API and none has to be reexported.
`service_name_format` is fixed for the process lifetime, but an unencodable label name was only found at collect time, which skips every metric family. The scrape that results parses as a healthy process exporting nothing, so the misconfiguration shows up as silent staleness. Check it during telemetry init and name the setting instead. Reexports `is_valid_name` and `NAME_REQUIREMENT` so the rule stays with the crate that enforces it rather than being duplicated.
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.
Overview
Wires
foundations-metricsin behindfoundations::telemetry::metricsvia a newfoundations-metrics-backendfeature, enabled by default, replacing the legacyprometheus-client/prometoolsstack while keeping call sites source-compatible. Also addsAccept-header negotiation to/metrics, a public API for custom metrics, and makes both new crates publishable.The metric types themselves (counters, gauges, classic/time/native histograms, exemplars, info metrics) already existed in
foundations-metricsbefore this branch point. This is the integration work, not the implementation.Behaviour changes
Worth reviewing closely - all follow from the backend being on by default.
Exposition output differs. Verified by capturing real
collect()output in both configs:metric 1metric 1.0# HELP name .Values are protobuf doubles, hence
1.0. Parsers see identical series, but consumers asserting on exact/metricsstrings will notice; three existing tests needed updating.The
prometheuscrate's global registry is no longer drained. The legacy collector exportedprometheus::gather()alongside its own. Anything registered there - including theprocessfeature's collector — silently disappears and must be re-registered throughregister. The most likely surprise for an existing service; documented infoundations-metrics/README.md./metricshonoursAccept. It previously ignored it and always returnedapplication/openmetrics-text; version=1.0.0; charset=utf-8- still what a scraper sending noAccept, or one accepting text, receives. New: length-delimited protobuf and UTF-8-unescaped OpenMetrics text on request. A weight invalidates its range rather than ranking it last when it isq=0, unparseable, or outside RFC 9110's 0–1 — the last case coveringnanandinf, which parse as floats but cannot be ranked. The negotiatedescapingis reported but not enforced.Negotiation never fails a scrape. An
Acceptmatching nothing available is served the text fallback with a logged warning. A406was rejected: a scraper counts it as a failed scrape and records nothing, whereas an unrequested format is at worst ignored. The log is the only signal, since the response is an ordinary200.Protobuf is withheld while any
ExtraProduceris registered, since that extension point is text-only and cannot be transcoded. All-or-nothing: one producer disables protobuf process-wide, and a protobuf-only client gets text rather than a truncated exposition. Evaluated per scrape, not a setting. Native histograms are not representable in OpenMetrics text, so a process with both cannot export the full set in either format.Telemetry init now fails on a service label name that cannot be encoded.
service_name_formatis fixed for the process lifetime, butLabelWithName("")was only caught at collect time, where it skips every family — a scrape that parses as a healthy process exporting nothing. Rejected at startup instead, naming the setting at fault.New API
Custom metrics: implement
EncodeMetricValue(widened frompub(crate)) and wrap inNamedMetric, or implementEncodeMetricdirectly to control naming. Register viaregister(Box<dyn EncodeMetric>, RegistrationMetadata).Newly public through
telemetry::metrics, only with the backend enabled — this module's surface is now feature-dependent:NativeHistogram,NativeHistogramBuilder,NativeHistogramWithExemplars,EncodeMetric,EncodeMetricValue,IntoMetrics,LabelError,MetricFamily,NamedMetric,RegistrationMetadata,proto,register,to_label_pairs.The 12 pre-existing names (
Counter,Family,Gauge, the histogram and exemplar types,MetricConstructor,GaugeGuard,RangeGauge,InfoMetric) now resolve to different concrete types. Source-compatible in typical use; affects code naming the underlyingprometheus_client/prometoolstypes or implementingMetricConstructorfor a foreign type.Deprecations
add_extra_producerandExtraProducer, in favour ofEncodeMetric+register— text output bypasses validation and is absent from protobuf scrapes. Still functional under both backends. Downstreams building with-D warningswill need#[allow(deprecated)].Other
OnceLockwithout initialising, so collecting beforeinitno longer permanently poisons the prefix withundefined_.#[metrics]names metric types through the facade rather thanreexports_for_macros, which it still uses for serde. No change to its accepted input — samecrate_path,unprefixed,#[ctor],#[optional],#[with_removal].foundations-metrics' public API, so RFC-047 needs noparking_lotreexport:CounterWithExemplar::get/innernow return the guardFamilyalready used, renamedFamilyMetricGuard→MetricReadGuard. Not exposed through the facade.foundations-metrics/README.mdwith a migration guide, and documented existing decisions that read as arbitrary (UTF-8 name validation, NUL rejection,f64::MAXas the terminal bucket bound).descriptionfields required bycargo publish. No new third-party dependencies.build.rsnow compiles thelinux/seccomp.hprobe into a scratch directory so its artefacts don't linger inOUT_DIR.Tests
New coverage for custom metrics registered from outside the crate and through the facade, info metrics keeping unprefixed names,
Acceptnegotiation including a genuinely binary protobuf body, protobuf withholding, the fallback warning, un-rankableAcceptweights, and startup rejection of an un-encodable service label name.Use
cargo nextest run. Two tests infoundations/tests/metrics.rsfail under plaincargo testbecause they share the global registry in one process; this reproduces at the base commit and is not introduced here.