test: validate metrics compatibility with Grafana scrapper #6150
Conversation
WalkthroughAdds a new Go-based Prometheus metrics validator tool, test coverage, CI steps to run the tool's tests and set up Go, appends metrics validation to the calibnet test script, and documents local usage and .gitignore entries for metric files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Script as calibnet_other_check.sh
participant Node as Forest Node (:6116)
participant Validator as prometheus_metrics_validator (go run)
Script->>Node: GET /metrics
Node-->>Script: metrics text
Script->>Validator: go run main.go metrics.log
Validator->>Validator: Validate(metrics)
alt parse ok
Validator-->>Script: exit 0 (valid)
else parse error
Validator-->>Script: exit 1 (invalid)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
Comment |
hanabi1224
requested review from
akaladarshi and
elmattic
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tools/README.md (1)
1-10: Clarify the working directory for test commands.The unit test command
go test -v .implies it should be run fromtools/prometheus_metrics_validator/, but this isn't explicitly stated. Consider adding a note about the working directory or using the full path as in the CI workflow.Apply this diff to clarify:
### Unit tests -- run `go test -v .` +Run from the repository root: +```bash +go test -v ./tools/prometheus_metrics_validator +``` + +Or from the `tools/prometheus_metrics_validator/` directory: +```bash +go test -v . +``` ### Validate Forest metricstools/prometheus_metrics_validator/main.go (1)
45-60: Consider documenting the textparse.New boolean parameters.The
textparse.Newcall uses three boolean parameters (false, false, true) whose purpose is unclear without referring to the Prometheus documentation. Adding a brief comment would improve code maintainability.Apply this diff to add clarity:
func Validate(metrics []byte) error { + // textparse.New parameters: content, contentType, timestamp, parseClassicHistograms, skipMetrics, enableUTF8, symbolTable p, err := textparse.New(metrics, "text/plain", "", false, false, true, labels.NewSymbolTable()) if err != nil { return err
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.workis excluded by!**/*.worktools/prometheus_metrics_validator/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/forest.yml(2 hunks)scripts/tests/calibnet_other_check.sh(1 hunks)tools/README.md(1 hunks)tools/prometheus_metrics_validator/.gitignore(1 hunks)tools/prometheus_metrics_validator/go.mod(1 hunks)tools/prometheus_metrics_validator/main.go(1 hunks)tools/prometheus_metrics_validator/main_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tools/prometheus_metrics_validator/main_test.go (1)
tools/prometheus_metrics_validator/main.go (1)
Validate(45-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Diff snapshot export checks
- GitHub Check: V1 snapshot export checks
🔇 Additional comments (7)
.github/workflows/forest.yml (2)
86-88: LGTM! Go test step is correctly configured.The test command targets the correct path and will run the validator's unit tests during CI.
161-163: LGTM! Go setup is required for metrics validation.The calibnet-check job correctly sets up Go before running the validation script that invokes the metrics validator.
tools/prometheus_metrics_validator/main.go (1)
15-43: LGTM! CLI implementation is clean and functional.The CLI properly handles file reading, validation, and error reporting. The use of
os.Exit(1)on validation failure is appropriate for a CLI tool.tools/prometheus_metrics_validator/main_test.go (1)
9-45: LGTM! Test coverage is appropriate.The tests cover key scenarios:
- Metrics without EOF marker
- Metrics with proper metadata (HELP, TYPE, UNIT) and EOF
- Invalid syntax with unsupported characters
This provides good baseline coverage for the validator's core functionality.
scripts/tests/calibnet_other_check.sh (1)
83-85: LGTM! Metrics validation is properly integrated.The script correctly:
- Downloads metrics from the Forest node
- Invokes the validator to check compatibility
Using
go runis appropriate for the CI environment and keeps the script simple.tools/prometheus_metrics_validator/go.mod (2)
6-6: Prometheus module tag and parser import are correct v0.306.0 maps to Prometheus v3.6.0; import the text parser from "github.com/prometheus/prometheus/model/textparse".
3-3: Go version 1.25.2 is valid Thego versioncommand downloads and runs go1.25.2 without error.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@hanabi1224 CI check is failing |
2 participants
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #6064
Other information and links
Change checklist
Summary by CodeRabbit