-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Added _total suffix to OTEL counter metrics. #5810
Conversation
Signed-off-by: Wise-Wizard <[email protected]>
Signed-off-by: Wise-Wizard <[email protected]>
Signed-off-by: Wise-Wizard <[email protected]>
Signed-off-by: Wise-Wizard <[email protected]>
Signed-off-by: Wise-Wizard <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5810 +/- ##
=======================================
Coverage 96.66% 96.66%
=======================================
Files 342 342
Lines 16521 16525 +4
=======================================
+ Hits 15970 15974 +4
Misses 362 362
Partials 189 189
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
please add a unit test
Sorry, can you specify which Unit test? Because, the tests are already written for counter using _total suffix. |
you are adding a new behavior. If you remove that behavior, some test should fail. |
Signed-off-by: Wise-Wizard <[email protected]>
No, that's the thing without adding that functionality itself the metric names are being added with _total suffix in the tests itself. |
The purpose of unit tests is to protect the code from degradation. If I revert the change made in this PR, no unit tests will fail, so you have no protection against functional degradation. We need to add one. |
Signed-off-by: Wise-Wizard <[email protected]>
Signed-off-by: Wise-Wizard <[email protected]>
**Which problem is this PR solving?** This PR addresses a part of the issue [jaegertracing#5633 ](jaegertracing#5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard <[email protected]> Signed-off-by: Jared Tan <[email protected]>
**Which problem is this PR solving?** This PR addresses a part of the issue [jaegertracing#5633 ](jaegertracing#5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard <[email protected]> Signed-off-by: Jared Tan <[email protected]>
**Which problem is this PR solving?** This PR addresses a part of the issue [jaegertracing#5633 ](jaegertracing#5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard <[email protected]> Signed-off-by: Jared Tan <[email protected]>
**Which problem is this PR solving?** This PR addresses a part of the issue [jaegertracing#5633 ](jaegertracing#5633) **Description of the changes** This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash CI actions and compare.py script ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: yarn lint` and `yarn test` --------- Signed-off-by: Wise-Wizard <[email protected]> Signed-off-by: Jared Tan <[email protected]>
Which problem is this PR solving?
This PR addresses a part of the issue #5633
Description of the changes
This is a PR to achieve Observability Parity in metrics between V1 and V2 components by configuring OTEL Collector to emit desired metrics.
How was this change tested?
The changes were tested by running the following command:
make test
Checklist
for jaeger: make lint test
for jaeger-ui: yarn lint
andyarn test