feat(ci): add metrics snapshot to SPM E2E workflow#8192
Open
hharshhsaini wants to merge 2 commits intojaegertracing:mainfrom
Open
feat(ci): add metrics snapshot to SPM E2E workflow#8192hharshhsaini wants to merge 2 commits intojaegertracing:mainfrom
hharshhsaini wants to merge 2 commits intojaegertracing:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the SPM end-to-end CI workflow to capture and verify a Jaeger Prometheus metrics snapshot after SPM assertions succeed, enabling metrics-diff gating similar to other E2E suites.
Changes:
- Add
scrape_metrics()toscripts/e2e/spm.shto curlhttp://localhost:8888/metricsand write a per-metricstore snapshot under.metrics/. - Invoke
scrape_metrics()aftercheck_spmcompletes successfully. - Add a
verify-metrics-snapshotstep to the SPM GitHub Actions workflow for each metricstore matrix variant.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/e2e/spm.sh | Adds metrics scraping and writes a snapshot file keyed by metricstore. |
| .github/workflows/ci-e2e-spm.yml | Uploads/verifies the new SPM metrics snapshots via the shared composite action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8192 +/- ##
==========================================
- Coverage 95.62% 95.61% -0.02%
==========================================
Files 318 318
Lines 16771 16771
==========================================
- Hits 16038 16036 -2
- Misses 578 579 +1
- Partials 155 156 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hharshhsaini
force-pushed
the
feat/metrics-snapshot-spm
branch
from
ec1120e to
a183e62
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds metrics snapshot collection to the SPM E2E workflow so CI can upload and diff Jaeger’s /metrics output per metricstore variant, aligning SPM with the other E2E suites’ metrics regression checks.
Changes:
- Add
scrape_metrics()toscripts/e2e/spm.shto fetchhttp://localhost:8888/metricsand write a per-metricstore snapshot into./.metrics/. - Invoke metrics scraping after SPM assertions pass.
- Add the
verify-metrics-snapshotcomposite action step to the SPM GitHub Actions workflow, keyed per matrix variant.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
scripts/e2e/spm.sh |
Scrapes and stores a metrics snapshot after SPM validations complete. |
.github/workflows/ci-e2e-spm.yml |
Uploads/verifies the snapshot artifact per metricstore matrix entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add metrics scraping to the SPM (Service Performance Monitoring)
integration test. Unlike storage E2E tests, SPM runs Jaeger via
docker-compose and does not use e2e_integration.go, so a
scrape_metrics() function is added to scripts/e2e/spm.sh that
curls the Jaeger metrics endpoint (http://localhost:8888/metrics)
after the test assertions pass and saves the result to
.metrics/metrics_snapshot_spm_${METRICSTORE}.txt.
The snapshot name embeds the metricstore (prometheus, elasticsearch,
opensearch) so each matrix variant produces a distinct file, and
verify-metrics-snapshot is wired up in ci-e2e-spm.yml accordingly.
Part of jaegertracing#6278.
Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
hharshhsaini
force-pushed
the
feat/metrics-snapshot-spm
branch
from
836b2dd to
496887a
Compare
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.
Which problem is this PR solving?
Description of the changes
scrape_metrics()function toscripts/e2e/spm.shthatcurls the Jaeger metrics endpoint (
http://localhost:8888/metrics)after the SPM test assertions pass and saves the output to
.metrics/metrics_snapshot_spm_${METRICSTORE}.txtprometheus,elasticsearch,opensearch) so each matrix variant producesa distinct, non-conflicting file
verify-metrics-snapshotaction step toci-e2e-spm.ymlfor each matrix variant using the matching snapshot/artifact key
and does not use
e2e_integration.go, so the scraping is doneexplicitly in the shell script
How was this change tested?
standard metrics port
8888(same as all other E2E setups)handle metrics (curl to :8888/metrics, save to .metrics/)
scrape_metrics()is called aftercheck_spm()succeedsand is non-fatal (warns but does not fail) if the endpoint is
temporarily unavailable
Checklist
make lint testAI Usage in this PR (choose one)
See AI Usage Policy.