Generate metrics catalog #719
Conversation
WalkthroughThis change introduces an automated process for generating and maintaining a catalog of metrics used within the codebase. A new Go program is added to parse metric definitions from source files and output a Markdown documentation file summarizing these metrics. A supporting Bash script is provided to run the generator from the repository root. The GitHub Actions workflow for Go linting is updated to include a step that executes this script, ensuring the metrics catalog is kept up to date. The resulting documentation file lists all metrics with their types, descriptions, and source locations. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Bash Script
participant Go Generator
participant Source Files
participant Markdown Doc
GitHub Actions->>Bash Script: Run ./dev/generate-metrics-catalog
Bash Script->>Go Generator: Execute generator.go from repo root
Go Generator->>Source Files: Parse Go files for metric definitions
Source Files-->>Go Generator: Return metric metadata
Go Generator->>Markdown Doc: Write metrics_catalog.md
Go Generator->>Bash Script: Exit
Bash Script->>GitHub Actions: Step complete
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/lint-go.yml (1)
46-46: Add a newline at the end of the fileThe YAMLlint tool reports that the file is missing a newline at the end, which is a standard practice in most codebases.
command: ./dev/generate-metrics-catalog +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
dev/generate-metrics-catalog (1)
1-10: LGTM: Script looks good but could use minor improvementsThe script correctly sets up strict error handling and ensures it runs from the repository root. Consider these minor enhancements:
-#!/bin/bash +#!/bin/bash -o pipefail set -eu -# Work always from the root directory +# This script generates the metrics catalog documentation by running the Go generator +# It ensures we work from the repository root directory regardless of where it's called from script_dir=$(dirname "$(realpath "$0")") repo_root=$(realpath "${script_dir}/../") cd "${repo_root}" go run pkg/metrics/docs/generator.gopkg/metrics/docs/generator.go (2)
15-15: Follow Go naming conventions for constantsIn Go, constants typically use camelCase or PascalCase naming, not SCREAMING_SNAKE_CASE.
-const MARKDOWN_OUTPUT = "doc/metrics_catalog.md" +const markdownOutput = "doc/metrics_catalog.md"Remember to update all references to this constant throughout the code.
86-135: Consider more robust metric parsing for future code changesThe current implementation assumes a specific structure for metric declarations (composite literals with Name and Help fields). This might be fragile if the metric declaration pattern changes.
Consider adding:
- Error logging when a metric is found but name or description can't be extracted
- Support for different patterns of metric declaration
- Unit tests to verify the parsing logic works with different metric patterns
Example enhancement:
for _, elt := range firstArg.Elts { if kv, ok := elt.(*ast.KeyValueExpr); ok { key := fmt.Sprint(kv.Key) switch key { case "Name": if val, ok := kv.Value.(*ast.BasicLit); ok { metric.Name = strings.Trim(val.Value, `"`) + } else { + log.Printf("Warning: Found Name field with unexpected value type in %s", path) } case "Help": if val, ok := kv.Value.(*ast.BasicLit); ok { metric.Description = strings.Trim(val.Value, `"`) + } else { + log.Printf("Warning: Found Help field with unexpected value type in %s", path) } } } } if metric.Name != "" { results = append(results, metric) + } else { + log.Printf("Warning: Found metric declaration without Name in %s", path) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/lint-go.yml(1 hunks)dev/generate-metrics-catalog(1 hunks)doc/metrics_catalog.md(1 hunks)pkg/metrics/docs/generator.go(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/lint-go.yml
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
🪛 GitHub Check: Lint-Go
pkg/metrics/docs/generator.go
[failure] 79-79:
File is not properly formatted (gofumpt)
[failure] 44-44:
error is not nil (line 42) but it returns nil (nilerr)
🪛 GitHub Actions: Lint
pkg/metrics/docs/generator.go
[error] 79-79: File is not properly formatted (gofumpt)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Code Review
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
- GitHub Check: Test (Node)
- GitHub Check: Upgrade Tests
🔇 Additional comments (2)
.github/workflows/lint-go.yml (1)
42-46: LGTM: Added metrics catalog generation stepThe change follows the existing pattern in the workflow file, using the same diff-check action as other steps. This will ensure the metrics catalog documentation stays up-to-date.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
doc/metrics_catalog.md (1)
1-21: LGTM: Well-structured auto-generated metrics catalogThe auto-generated metrics catalog provides a clear and organized table of metrics with their types, descriptions, and source locations. This will be valuable documentation for understanding and maintaining the metrics system.
Add automated metrics catalog generation to document metrics usage across the codebase
📍Where to StartStart with the main generator implementation in generator.go which contains the core logic for parsing metric declarations and generating the documentation. Macroscope summarized 5445458. |
| name string | ||
| } | ||
|
|
||
| var metricTypes = map[string]MetricType{ |
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe worth including this as a comment so it doesn't get lost
|
@coderabbitai please re-review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll conduct a fresh review of your Pull Request that introduces an automated metrics catalog generation. Let me first gather the necessary information to understand the implementation details. 🏁 Script executed: Length of output: 6833 @mkysel I've completed a fresh review of your "Generate metrics catalog" PR. This is a well-structured implementation that automates documentation of application metrics. The PR introduces:
The implementation is clean and follows good practices like proper error handling, modular code organization, and appropriate use of Go's AST for parsing declarations. A few minor suggestions that could enhance this further in the future:
Overall, this is a valuable addition that will help keep metrics documentation in sync with code. The automated workflow integration ensures it stays updated. |
| func parseFile(path string) ([]Metric, error) { | ||
| var results []Metric | ||
| fs := token.NewFileSet() | ||
| node, err := parser.ParseFile(fs, path, nil, parser.AllErrors) |
There was a problem hiding this comment.
Neat. I've only done this kind of thing via reflect, but this is actually pretty tidy
| @@ -0,0 +1,139 @@ | |||
| package main | |||
There was a problem hiding this comment.
Small nit, but I think this belongs in the cmd folder
Add automated metrics catalog generation to document application metrics in a markdown table
Implements a metrics documentation system through:
📍Where to Start
Start with the main generator program in generator.go which contains the core logic for parsing metric declarations and generating the documentation.
Macroscope summarized c0e7286.
Summary by CodeRabbit