-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Validate span names in SPM integration test #7830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate span names in SPM integration test #7830
Conversation
ffaee83 to
f336dcb
Compare
f336dcb to
100fe52
Compare
| case "$service" in | ||
| "driver") | ||
| echo "/FindNearest" | ||
| ;; | ||
| "customer") | ||
| echo "/customer" | ||
| ;; | ||
| "mysql") | ||
| echo "/sql_select" | ||
| ;; | ||
| "redis") | ||
| echo "/FindDriverIDs /GetDriver" | ||
| ;; | ||
| "frontend") | ||
| echo "/dispatch" | ||
| ;; | ||
| "route") | ||
| echo "/GetShortestRoute" | ||
| ;; | ||
| "ui") | ||
| echo "/" | ||
| ;; | ||
| *) | ||
| echo "" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I could've made this cleaner -- would've used a hashmap but I believe there's no such thing in bash
scripts/e2e/spm.sh
Outdated
|
|
||
| set -x | ||
| # Enable verbose logs if you want to debug the script. Else, keep logs clean. | ||
| # set -x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but the time the script fails in CI it's too late to enable logs. I prefer this option on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Reverted to on
|
lgtm |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7830 +/- ##
==========================================
+ Coverage 95.49% 95.51% +0.01%
==========================================
Files 307 307
Lines 15911 15911
==========================================
+ Hits 15195 15197 +2
+ Misses 562 561 -1
+ Partials 154 153 -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:
|
Metrics Comparison SummaryTotal changes across all snapshots: 0 Detailed changes per snapshotsummary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
|
|
please rebase on main |
Signed-off-by: Don Assamongkol <[email protected]>
100fe52 to
f632102
Compare
|
I will override the dco-lint, but you may want to fix your git vs. github settings to use the full name |
|
thanks |
Ah! Thanks for pointing that out. Will do |
Which problem is this PR solving?
Partially resolves #5608:
Description of the changes
validate_service_metricsfunction: grab the response from querying the metrics API then verify that the spans emitted have names for what we'd expect for that service.How was this change tested?
Run the e2e test locally:
(had to change registry port on my machine, but did not add that code change here)
Log excerpts:
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test