-
Notifications
You must be signed in to change notification settings - Fork 4.5k
stats/opentelemetry: Add RLS Metrics e2e tests #7655
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7655 +/- ##
==========================================
+ Coverage 81.73% 82.30% +0.57%
==========================================
Files 361 367 +6
Lines 27817 28934 +1117
==========================================
+ Hits 22735 23814 +1079
- Misses 3871 3884 +13
- Partials 1211 1236 +25 |
c56189c
to
e055c38
Compare
@zasweq : I'm not going to be able to get to this for at least a week. Please find another reviewer if you want it reviewed sooner. Thanks. |
As discussed offline: RLS's tests should go with RLS, not with OTel, and this PR is about testing RLS not OTel. That also prevents the need to fork a copy of / move the testing helper code around. |
Moved to balancer/rls. Thanks. |
b6790b5
to
c51df56
Compare
d024e73
to
832a2c6
Compare
Fixed the go.mods due to moving it by deleting the stats/opentelemetry go.mod. |
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.
This change makes us release the opentelemetry package and also CSM. We should make sure the release notes mention that. And make sure all the APIs contained in both are either marked as experimental or are believed to be stable.
Due to merge conflicts from updating repository dependencies opened #7759. |
This PR adds e2e tests for RLS Metrics, with verifications of the OpenTelemetry metrics atoms emitted.
RELEASE NOTES: