-
Notifications
You must be signed in to change notification settings - Fork 16
feat: Enricher wise metrics #265
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 Report
@@ Coverage Diff @@
## main #265 +/- ##
============================================
- Coverage 80.35% 80.20% -0.16%
Complexity 1162 1162
============================================
Files 102 102
Lines 4506 4512 +6
Branches 420 421 +1
============================================
- Hits 3621 3619 -2
- Misses 688 695 +7
- Partials 197 198 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
| // Must use linked hashmap | ||
| private final Map<String, Enricher> enrichers = new LinkedHashMap<>(); | ||
|
|
||
| private static final String ENRICHED_TRACES_COUNTER = "hypertrace.enriched.traces"; |
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.
nit: We can get rid of the hypertrace. prefix in both the metric names
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 is the metric convention we are using across hypertrace projects.
Also, this may cause compatibility issue if we change the name and existing dashboards may be broken.
@kotharironak @rish691: please confirm if changing and simplifying this metric name is okay?
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.
It is exposed in ready to use dashboard - https://github.com/hypertrace/hypertrace/tree/main/kubernetes/monitoring/dashboards. So would suggest keeping it as is now. And, we can clean up if we decided to go with metrics.reporter based prefix only in application.conf
Description
We need enricher-wise metrics to identify the bottlenecks specific to enricher implementations.
Testing
Tested on a dev SaaS cluster
Checklist: