Skip to content

metrics: parse Prometheus text exposition with best practices#1611

Closed
naoNao89 wants to merge 1 commit into
torrust:developfrom
naoNao89:feat/metrics-prometheus-deser-1582
Closed

metrics: parse Prometheus text exposition with best practices#1611
naoNao89 wants to merge 1 commit into
torrust:developfrom
naoNao89:feat/metrics-prometheus-deser-1582

Conversation

@naoNao89
Copy link
Copy Markdown
Contributor

  • Prefer parsed sample timestamps over fallback
  • Preserve HELP as description; keep unsupported families as errors (for now)
  • Remove gauge normalization in tests; only ensure trailing newline
  • Pin openmetrics-parser to 0.4.4 (latest)

Refs: #1582

- Prefer parsed sample timestamps over fallback
- Preserve HELP as description; keep unsupported families as errors (for now)
- Remove gauge normalization in tests; only ensure trailing newline
- Pin openmetrics-parser to 0.4.4 (latest)

Refs: torrust#1582
@da2ce7
Copy link
Copy Markdown
Contributor

da2ce7 commented Aug 25, 2025

@naoNao89 this looks much better and cleaner :)

@josecelano
Copy link
Copy Markdown
Member

josecelano commented May 4, 2026

Superseded by #1729

@josecelano josecelano closed this May 4, 2026
josecelano added a commit that referenced this pull request May 5, 2026
19f98d0 refactor(metrics): implement proposals 9-12 in prometheus pipeline (Jose Celano)
e2fa3fc docs(metrics): add follow-up refactor proposals 9-12 (Jose Celano)
bf76972 refactor(metrics): implement proposal 8 — normalize-parse-convert pipeline (Jose Celano)
0ccddc7 refactor(metrics): use From conversions for collection errors (Jose Celano)
d1f219b refactor(metrics): parse_prometheus_timestamp returns Option (Jose Celano)
43b4c57 refactor(metrics): use Cow for input normalization in from_prometheus (Jose Celano)
9de6444 refactor(metrics): extract description_from_help helper (Jose Celano)
04b0d74 refactor(metrics): add MetricDescription From conversions (Jose Celano)
3e91f69 refactor(metrics): extract is_whole_u64_representable predicate (Jose Celano)
bbedebd refactor(metrics): extract parse_family_samples via FromPrometheusValue trait (Jose Celano)
ffa6c27 docs(metrics): add refactoring proposals for metric_collection/prometheus.rs (Jose Celano)
886761f test(metrics): kill surviving mutants in remaining files (Jose Celano)
0140113 test(metrics): kill surviving mutants in label/set.rs (Jose Celano)
2bb5610 test(metrics): kill surviving mutants in sample_collection.rs and sample.rs (Jose Celano)
4644ec8 test(metrics): kill surviving mutants in counter.rs and gauge.rs (Jose Celano)
333e124 test(metrics): kill surviving mutants in metric_collection/mod.rs (Jose Celano)
a6b61c7 test(metrics): kill surviving mutants in metric_collection/prometheus.rs (Jose Celano)
92965d3 docs(metrics): add mutation testing plan for the metrics package (Jose Celano)
b8a131d test(metrics): add tests to close llvm-cov coverage gaps (Jose Celano)
8b38a65 docs(metrics): add unit test coverage improvement plan with llvm-cov baseline (Jose Celano)
7ba33c2 refactor(metrics): extract Prometheus impls into metric_collection/prometheus.rs (Jose Celano)
f85911e refactor(metrics): extract JSON serde impls into metric_collection/serde.rs (Jose Celano)
9ca54dd refactor(metrics): extract MetricKindCollection into kind_collection.rs (Jose Celano)
15cdb7e refactor(metrics): extract Error into metric_collection/error.rs (Jose Celano)
c3c42a1 docs(metrics): add refactor plan for metric_collection module split (Jose Celano)
9b3277e fix(metrics): address Copilot PR review feedback (Jose Celano)
94c53b9 refactor(metrics): extract Prometheus deserialization helpers (Jose Celano)
25f6eb3 feat(metrics): add Prometheus text format deserialization (naoNao89)
014f517 docs(metrics): add issue spec for Prometheus deserialization (#1582) (Jose Celano)

Pull request description:

  ## Summary

  Adds deserialization from Prometheus exposition text format to `MetricCollection`, completing the serialization ↔ deserialization symmetry alongside the existing JSON and `to_prometheus()` paths.

  Closes #1582

  ---

  ## What changed

  ### `packages/metrics/src/prometheus.rs`

  - New `PrometheusDeserializable` trait (mirrors `PrometheusSerializable`)
  - New `PrometheusDeserializationError` enum with fine-grained variants:
    - `ParseError` — syntax error in the input text
    - `UnsupportedType` — known but not yet supported type (Histogram, Summary, …)
    - `UnknownType` — unrecognised metric type
    - `ValueMismatch` — value type does not match the declared metric type
    - `UnknownValue` — `PrometheusValue::Unknown` sample (no silent zero-returns)
    - `LabelConversion` — label name/value conversion failure
    - `CollectionError` — structural error assembling the `MetricCollection`

  ### `packages/metrics/src/label/set.rs`

  - `TryFrom<openmetrics_parser::LabelSet<'_>> for LabelSet` — replaces ad-hoc inline label conversion

  ### `packages/metrics/src/metric_collection/mod.rs`

  - Private `parse_prometheus_timestamp(t: f64, fallback) -> DurationSinceUnixEpoch` — eliminates the duplicated timestamp-parsing block
  - `impl PrometheusDeserializable for MetricCollection` — parses Counter and Gauge families; uses `openmetrics-parser 0.4.4`

  ### `packages/metrics/Cargo.toml`

  - Added `openmetrics-parser = "0.4.4"`

  ---

  ## Tests

  - Timestamp helper: 8 edge-case unit tests (negative, NaN, ±Inf, nanosecond-boundary overflow, zero)
  - `TryFrom` for `LabelSet`: 3 unit tests (empty conversion, round-trip, empty-name error)
  - `MetricCollection`: round-trip serialize → deserialize, error-path tests for unsupported and malformed input
  - **222 tests pass**, 0 failed

  ---

  ## Prior art

  This is a clean re-implementation based on PR #1611 by @naoNao89, incorporating all maintainer feedback that was pending on that PR. @naoNao89 is preserved as the commit author.

ACKs for top commit:
  josecelano:
    ACK 19f98d0

Tree-SHA512: f3aee3e168e02da7dd4aa4dbd25af5ea723b9aacf24259ce9b2ac7d1ced66936e10ea41dcfccacaece502e8533120cd507617f827186dea44ee7ffda68356717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants