ci: Exclude HTTP 5xx metrics from comparisons#7671
ci: Exclude HTTP 5xx metrics from comparisons#7671yurishkuro merged 4 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: Tushar Anand <tusharannand@gmail.com>
Signed-off-by: Tushar Anand <tusharannand@gmail.com>
bd4ceed to
8c4232c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7671 +/- ##
=======================================
Coverage 95.56% 95.56%
=======================================
Files 307 307
Lines 15399 15399
=======================================
Hits 14716 14716
Misses 536 536
Partials 147 147
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:
|
scripts/e2e/compare_metrics.py
Outdated
| should_exclude= should_exclude_metric(sample.name, labels) | ||
| if should_exclude: |
There was a problem hiding this comment.
| should_exclude= should_exclude_metric(sample.name, labels) | |
| if should_exclude: | |
| if should_exclude_metric(sample.name, labels): |
There was a problem hiding this comment.
it would be good to count these exclusions in stats
There was a problem hiding this comment.
it would be good to count these exclusions in stats
Like the ones posted in PR, if some stats are excluded?
There was a problem hiding this comment.
yes (although I am not sure excluding full metrics is a good thing, per the other comment).
Signed-off-by: Tushar Anand <tusharannand@gmail.com>
|
can we address this ? #7671 (comment) |
Yes sure @yurishkuro , i would like to open another pr which will modify the necessary scripts that post the comment. |
|
ok |
## Which problem is this PR solving? - Part of #7617 and [comment ](#7671 (comment)) ## Description of the changes - This PR introduces the changes in scripts to also include count of exclusions in metrics if any. ## How was this change tested? - I used the artifacts form https://github.com/jaegertracing/jaeger/actions/runs/19589467818/job/56105108460 and https://github.com/jaegertracing/jaeger/actions/runs/19596403385/job/56121587792?pr=7666 to manually test the changes. The outputs were as in the below screenshots <img width="665" height="425" alt="image" src="https://github.com/user-attachments/assets/69a13af3-32d2-403e-b8a9-1fe33899202e" /> <img width="908" height="297" alt="image" src="https://github.com/user-attachments/assets/eae2b3e0-ad63-41d2-ace5-a3e911a41858" /> ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Tushar Anand <tusharannand@gmail.com>
Which problem is this PR solving?
Description of the changes
How was this change tested?
Before changes

After changes

Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test