-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Fix] Restore ES metrics #7006
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
[Fix] Restore ES metrics #7006
Conversation
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7006 +/- ##
==========================================
+ Coverage 96.12% 96.20% +0.07%
==========================================
Files 357 358 +1
Lines 21570 21596 +26
==========================================
+ Hits 20735 20777 +42
+ Misses 629 613 -16
Partials 206 206
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:
|
|
@yurishkuro I just wanted to know about my direction at moment. |
internal/storage/v1/elasticsearch/spanstore/mocks/CoreSpanWriter.go
Outdated
Show resolved
Hide resolved
Signed-off-by: anmol7344 <[email protected]>
f5011af to
17756d8
Compare
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Anmol <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
yurishkuro
left a comment
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.
I don't see any unit tests validating that the expected metrics are being emitted
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
While writing the unit tests for emitted metrices I was thinking of simulating the |
you can move the code of the callback into a top level function which you can call directly from the test |
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
Signed-off-by: anmol7344 <[email protected]>
## Which problem is this PR solving? - Resolves jaegertracing#6891 ## Description of the changes - Reintroduced metrics which were lost during jaegertracing#6883 ## How was this change tested? - Ran writer tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] 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: anmol7344 <[email protected]> Signed-off-by: Anmol <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
Which problem is this PR solving?
Description of the changes
CoreSpanWriterfromSpanWriter#6883How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test