feat(docs): add v2 metrics reference for backwards compatibility tracking#1071
feat(docs): add v2 metrics reference for backwards compatibility tracking#1071hharshhsaini wants to merge 1 commit intojaegertracing:mainfrom
Conversation
d8aafd5 to
9696dbe
Compare
There was a problem hiding this comment.
Pull request overview
Integrates an auto-generated Jaeger v2 metrics reference into the documentation site by fetching upstream-generated markdown tables and adding a dedicated docs page linked from the Operations section.
Changes:
- Added
scripts/fetch_metrics_reference.pyto download and assemble metrics mapping tables fromjaegertracing/jaeger. - Added
fetch-metricsMakefile target to generate the Hugo-compatible metrics reference page. - Added a new Operations nav entry and the generated
metrics-reference.mdpage under v2 docs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
scripts/fetch_metrics_reference.py |
New generator script that downloads upstream markdown and stitches it into a single Hugo page. |
Makefile |
Adds fetch-metrics target to run the generator script. |
content/docs/v2/_dev/operations/metrics-reference.md |
New generated metrics reference page content. |
content/docs/v2/_dev/operations/_index.md |
Adds “Metrics Reference (v2)” to the Operations sidebar navigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9696dbe to
fe8ec86
Compare
fe8ec86 to
1343775
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1343775 to
7fa0df0
Compare
|
This PR implements Task 5 of issue #6278 integrating the auto-generated Jaeger v1 → v2 metrics reference into the documentation website. Here's a quick summary of what's been done:
All CI checks are passing. All Copilot review suggestions have been addressed. I believe this PR is ready for merge. |
|
Please explain what this is trying to achieve. The linked ticket is not asking for comparison with v1 metrics - v1 is deprecated. |
|
Thanks for the feedback. Looking at the linked issue and the merged PRs (#5941, #6310, #6330, #7376), those PRs introduced an automated CI pipeline that generates markdown tables mapping Jaeger v1 metric names to their Jaeger v2 / OpenTelemetry equivalents (for the purpose of helping users migrating from v1 to v2). Task 5 of issue #6278 specifically says:
I interpreted "the report" as those auto-generated migration comparison tables. My goal was to surface them in the docs site so users migrating from v1 can find the mapping. If this is not the right interpretation, I'd appreciate clarification -> should this PR focus only on documenting the new v2/OTEL metrics (without any v1 comparison), or is the entire premise of this PR incorrect? Happy to adjust the direction based on your feedback. |
|
The whole issue is about maintaining backwards compatibility of metrics when new v2 releases are done. It has nothing to do with v1 metrics which are already not compatible. |
7fa0df0 to
ba7b19a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ba7b19a to
d31cae8
Compare
d31cae8 to
05f539e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
05f539e to
b9f7993
Compare
I've updated the approach accordingly. The page now documents only the current Jaeger v2 metrics and their labels, with the description clearly stating its purpose: tracking backwards compatibility across v2 releases (i.e. ensuring metric names and labels don't change unexpectedly between v2 versions). The v1-related framing has been completely removed. Updated commit: Please take another look . |
.cspell/project-words.txt
Outdated
| ingester | ||
| ingester's | ||
| ingesters | ||
| inuse |
There was a problem hiding this comment.
why do we need to add new words?
There was a problem hiding this comment.
These words (inuse, mallocs, mcache, memstats, memtable, mspan, valueloggc) were added to silence spellcheck errors originating from Go runtime metrics in the old generated content. Since the metrics-reference.md is now only a placeholder (actual data generated at release time), none of those runtime terms appear in the checked files anymore. Removed all of them.
|
|
||
| > **Note:** This page is auto-generated. Run `make fetch-metrics` to update it. | ||
|
|
||
| ## All-in-One Component Metrics |
There was a problem hiding this comment.
there is no need to reference all-in-one, there is just Jaeger that has different extensions.
There was a problem hiding this comment.
The section title in the script has been updated from All-in-One Component to simply Jaeger.
| @@ -0,0 +1,141 @@ | |||
| --- | |||
There was a problem hiding this comment.
we do not need to check this in under _dev (can check a placeholder if necessary). The actual metrics summary must be built during the release process.
There was a problem hiding this comment.
The fully-generated content has been replaced with a placeholder page that explains the purpose and how to regenerate it (make fetch-metrics). The script is now clearly documented as a release-time tool, not something that generates committed content during development
scripts/fetch_metrics_reference.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Base URL for the raw markdown files generated by the jaeger E2E workflow | ||
| BASE_RAW_URL = "https://raw.githubusercontent.com/jaegertracing/jaeger/main/cmd/jaeger/docs/migration/" |
There was a problem hiding this comment.
these are stale files that do not reflect the current state of Jaeger metrics.
There was a problem hiding this comment.
The cmd/jaeger/docs/migration/ files are the v1→v2 one-time migration artifacts. The script comment has been updated to clarify this and notes that during an actual release, a specific release tag ref should be passed for reproducible output. I understand the longer-term goal is for the CI to generate fresh metrics snapshots per release and have the documentation updated from those happy to align this script with whatever that release workflow looks like once it's in place.
b9f7993 to
d7d76ec
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d7d76ec to
a984206
Compare
|
I've addressed all the feedback from your latest review:
|
scripts/fetch_metrics_reference.py
Outdated
| logging.basicConfig(level=logging.INFO) | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Base URL template for the metrics reports generated by the Jaeger E2E integration tests. |
There was a problem hiding this comment.
metrics reports generated by the Jaeger E2E integration tests
CI does not store reports in the repository as docs, it stores them as GH artifacts.
There was a problem hiding this comment.
Fixed, @yurishkuro Updated the comment to accurately state that CI stores metrics reports as GitHub artifacts, not as docs committed to the repository. The comment now reads: "CI stores metrics reports as GitHub artifacts, not as docs in the repository."
a984206 to
732007b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…king Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
732007b to
c62cdec
Compare
Which problem is this PR solving?
Description of the changes
jaegertracing/jaegerand extracts the current Jaeger v2 metric names and their labels into a single reference page.fetch-metricsMakefile target so the page can be regenerated easily when a new Jaeger v2 release is made.How was this change tested?
make fetch-metricslocally to verify the script correctly pulls and parses the upstream metrics reports, outputting only v2 metric names and label dimensions.npm run check:spellingandnpm run check:links:allboth pass locally.Checklist
make lint testAI Usage in this PR (choose one)
See AI Usage Policy.