You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/10/09 00:37:33 UTC
[GitHub] [pinot] timsants opened a new pull request #7551: Removing DropWizard metrics library from distribution
timsants opened a new pull request #7551:
URL: https://github.com/apache/pinot/pull/7551
## Context
The Pinot OSS distribution is currently loading two metric library plugins (yammer and dropwizard). This is causing metrics to be missed on certain instances of servers, depending on which metric library was loaded.
## Description
Removing DropWizard metrics library from distribution. In addition, will throw an exception if more than 1 metrics library is found.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
We are assuming that there are no customers are attempting to use 2 or more different metric libraries at a time.
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
## Release Notes
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37134c4) into [master](https://codecov.io/gh/apache/pinot/commit/c36d2b975041e08656598d69e3984bd9b3bb0c81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c36d2b9) will **decrease** coverage by `57.31%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
=============================================
- Coverage 72.63% 15.31% -57.32%
+ Complexity 3411 80 -3331
=============================================
Files 1524 1478 -46
Lines 75699 73850 -1849
Branches 11039 10839 -200
=============================================
- Hits 54981 11310 -43671
- Misses 17017 61720 +44703
+ Partials 3701 820 -2881
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.31% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `0.00% <0.00%> (-88.16%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/core/data/table/BaseTable.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0Jhc2VUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1203 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c36d2b9...37134c4](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22d9a7b) into [master](https://codecov.io/gh/apache/pinot/commit/4ba806100565cd209cacba914acfe7d3e344765a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ba8061) will **decrease** coverage by `1.25%`.
> The diff coverage is `76.47%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.57% 71.31% -1.26%
- Complexity 3412 3422 +10
============================================
Files 1522 1522
Lines 75912 75917 +5
Branches 11094 11093 -1
============================================
- Hits 55090 54142 -948
- Misses 17126 18096 +970
+ Partials 3696 3679 -17
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `28.94% <76.47%> (-0.01%)` | :arrow_down: |
| unittests1 | `69.90% <73.33%> (+0.07%)` | :arrow_up: |
| unittests2 | `15.23% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.00% <73.33%> (+1.84%)` | :arrow_up: |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `78.18% <100.00%> (ø)` | |
| [.../main/java/org/apache/pinot/minion/MinionConf.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uQ29uZi5qYXZh) | `84.61% <100.00%> (-7.06%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| ... and [102 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ba8061...22d9a7b](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87e66cf) into [master](https://codecov.io/gh/apache/pinot/commit/493ed2c0433fdc40f605f0b06ce5f6f98ce62725?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (493ed2c) will **decrease** coverage by `2.70%`.
> The diff coverage is `75.00%`.
> :exclamation: Current head 87e66cf differs from pull request most recent head 09f7b12. Consider uploading reports for the commit 09f7b12 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.54% 69.84% -2.71%
+ Complexity 3416 3336 -80
============================================
Files 1522 1127 -395
Lines 75905 53670 -22235
Branches 11093 8109 -2984
============================================
- Hits 55069 37484 -17585
+ Misses 17144 13540 -3604
+ Partials 3692 2646 -1046
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.84% <75.00%> (-0.01%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.12% <75.00%> (+1.96%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [594 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [493ed2c...09f7b12](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726783256
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
+ @Nullable String configuredMetricsFactoryClassName) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
+
+ String metricsFactoryClassName = configuredMetricsFactoryClassName == null
+ ? DEFAULT_METRICS_FACTORY_CLASS_NAME : configuredMetricsFactoryClassName;
+
for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+ if (clazz.getName().equals(metricsFactoryClassName)) {
Review comment:
What do you think about using this functional style to find it?
```
Optional<Class<?>> maybeClazz = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
.findFirst();
maybeClazz.ifPresent(clazz -> {
MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
if (annotation.enabled()) {
try {
PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
pinotMetricsFactory.init(metricsConfiguration);
registerMetricsFactory(pinotMetricsFactory);
} catch (Exception e) {
LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
}
}
}
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727532941
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -70,21 +74,31 @@ public static void init(PinotConfiguration metricsConfiguration) {
private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
- for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+
+ String metricsFactoryClassName = metricsConfiguration.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
+ DEFAULT_METRICS_FACTORY_CLASS_NAME);
+ LOGGER.info("{} will be initialized as the PinotMetricsFactory", metricsFactoryClassName);
+
+ Optional<Class<?>> maybeClazz = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
Review comment:
Changing to `clazzFound`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
We could construct a new PinotConfiguration combine the exiting configs with the metrics factory config but seemed like a bit of overhead for just 1 property to be immediately retrieved again. It would look something like this:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22d9a7b) into [master](https://codecov.io/gh/apache/pinot/commit/4ba806100565cd209cacba914acfe7d3e344765a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ba8061) will **decrease** coverage by `6.67%`.
> The diff coverage is `64.70%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.57% 65.89% -6.68%
- Complexity 3412 3422 +10
============================================
Files 1522 1476 -46
Lines 75912 74066 -1846
Branches 11094 10893 -201
============================================
- Hits 55090 48806 -6284
- Misses 17126 21807 +4681
+ Partials 3696 3453 -243
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.90% <73.33%> (+0.07%)` | :arrow_up: |
| unittests2 | `15.23% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-78.19%)` | :arrow_down: |
| [.../main/java/org/apache/pinot/minion/MinionConf.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uQ29uZi5qYXZh) | `15.38% <0.00%> (-76.29%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.00% <73.33%> (+1.84%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [358 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ba8061...22d9a7b](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940198427
> Shall we add a config to specify the metricsRegistry to load?
We could do that, however, one issue I see is that setting instance level config is prone to having different configs for different instances in certain deployments. Also, we need to create a config per component (controller/broker/server/minion). If there's an better way to add a config, we can do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22d9a7b) into [master](https://codecov.io/gh/apache/pinot/commit/4ba806100565cd209cacba914acfe7d3e344765a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ba8061) will **decrease** coverage by `2.66%`.
> The diff coverage is `73.33%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.57% 69.90% -2.67%
+ Complexity 3412 3342 -70
============================================
Files 1522 1127 -395
Lines 75912 53671 -22241
Branches 11094 8106 -2988
============================================
- Hits 55090 37520 -17570
+ Misses 17126 13512 -3614
+ Partials 3696 2639 -1057
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.90% <73.33%> (+0.07%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.00% <73.33%> (+1.84%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [597 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ba8061...22d9a7b](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a67e4e6) into [master](https://codecov.io/gh/apache/pinot/commit/4ba806100565cd209cacba914acfe7d3e344765a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ba8061) will **decrease** coverage by `41.98%`.
> The diff coverage is `76.47%`.
> :exclamation: Current head a67e4e6 differs from pull request most recent head 22d9a7b. Consider uploading reports for the commit 22d9a7b to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
=============================================
- Coverage 72.57% 30.58% -41.99%
=============================================
Files 1522 1513 -9
Lines 75912 75569 -343
Branches 11094 11055 -39
=============================================
- Hits 55090 23116 -31974
- Misses 17126 50398 +33272
+ Partials 3696 2055 -1641
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.58% <76.47%> (+0.23%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (-26.32%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `86.25% <73.33%> (-1.91%)` | :arrow_down: |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `75.45% <100.00%> (-2.73%)` | :arrow_down: |
| [.../main/java/org/apache/pinot/minion/MinionConf.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uQ29uZi5qYXZh) | `76.92% <100.00%> (-14.75%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1067 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ba8061...22d9a7b](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (09f7b12) into [master](https://codecov.io/gh/apache/pinot/commit/493ed2c0433fdc40f605f0b06ce5f6f98ce62725?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (493ed2c) will **decrease** coverage by `57.32%`.
> The diff coverage is `18.18%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
=============================================
- Coverage 72.54% 15.22% -57.33%
+ Complexity 3416 80 -3336
=============================================
Files 1522 1476 -46
Lines 75905 74062 -1843
Branches 11093 10895 -198
=============================================
- Hits 55069 11274 -43795
- Misses 17144 61975 +44831
+ Partials 3692 813 -2879
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.22% <18.18%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `0.00% <0.00%> (-88.16%)` | :arrow_down: |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-78.19%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (-26.32%)` | :arrow_down: |
| [...e/pinot/broker/broker/helix/BaseBrokerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jhc2VCcm9rZXJTdGFydGVyLmphdmE=) | `71.50% <100.00%> (-2.09%)` | :arrow_down: |
| [...apache/pinot/controller/BaseControllerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9CYXNlQ29udHJvbGxlclN0YXJ0ZXIuamF2YQ==) | `78.84% <100.00%> (-4.76%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1205 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [493ed2c...09f7b12](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727511389
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
##########
@@ -28,7 +28,7 @@
public class MinionConf extends PinotConfiguration {
public static final String END_REPLACE_SEGMENTS_TIMEOUT_MS_KEY = "pinot.minion.endReplaceSegments.timeoutMs";
public static final int DEFAULT_END_REPLACE_SEGMENTS_SOCKET_TIMEOUT_MS = 10 * 60 * 1000; // 10 mins
-
+
Review comment:
nit: extra space
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
We could construct a new PinotConfiguration combine the exiting configs with the metrics factory config but seemed like a bit of overhead for just 1 property to be immediately retrieved again. It would look something like this:
```
Map<String, Object> brokerMetricsConfiguration = _brokerConf.subset(Broker.METRICS_CONFIG_PREFIX).toMap();
Map<String, String> metricsFactoryConfiguration = Collections.singletonMap(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
_brokerConf.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME));
PinotConfiguration metricsConfiguration = new PinotConfiguration(brokerMetricsConfiguration, metricsFactoryConfiguration);
PinotMetricUtils.init(metricsConfiguration);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
We could construct a new PinotConfiguration combine the exiting configs with the metrics factory config but seemed like a bit of overhead for just 1 property.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727477419
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -70,21 +74,31 @@ public static void init(PinotConfiguration metricsConfiguration) {
private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
- for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+
+ String metricsFactoryClassName = metricsConfiguration.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
+ DEFAULT_METRICS_FACTORY_CLASS_NAME);
+ LOGGER.info("{} will be initialized as the PinotMetricsFactory", metricsFactoryClassName);
+
+ Optional<Class<?>> maybeClazz = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
Review comment:
Perhaps better name than `maybeClazz`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e7a0b52) into [master](https://codecov.io/gh/apache/pinot/commit/c36d2b975041e08656598d69e3984bd9b3bb0c81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c36d2b9) will **decrease** coverage by `57.42%`.
> The diff coverage is `18.18%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
=============================================
- Coverage 72.63% 15.20% -57.43%
+ Complexity 3411 80 -3331
=============================================
Files 1524 1476 -48
Lines 75699 74041 -1658
Branches 11039 10887 -152
=============================================
- Hits 54981 11261 -43720
- Misses 17017 61970 +44953
+ Partials 3701 810 -2891
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.20% <18.18%> (-0.10%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `0.00% <0.00%> (-88.16%)` | :arrow_down: |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-78.19%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (-26.32%)` | :arrow_down: |
| [...e/pinot/broker/broker/helix/BaseBrokerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jhc2VCcm9rZXJTdGFydGVyLmphdmE=) | `71.50% <100.00%> (-2.09%)` | :arrow_down: |
| [...apache/pinot/controller/BaseControllerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9CYXNlQ29udHJvbGxlclN0YXJ0ZXIuamF2YQ==) | `78.84% <100.00%> (-4.76%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1208 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c36d2b9...e7a0b52](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
We could construct a new PinotConfiguration that combines the exiting subset configs with the metrics factory config but seemed like a bit of overhead for the sake of combining 2 parameters. It would look something like this:
```
Map<String, Object> brokerMetricsConfiguration = _brokerConf.subset(Broker.METRICS_CONFIG_PREFIX).toMap();
Map<String, String> metricsFactoryConfiguration = Collections.singletonMap(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
_brokerConf.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME));
PinotConfiguration metricsConfiguration = new PinotConfiguration(brokerMetricsConfiguration, metricsFactoryConfiguration);
PinotMetricUtils.init(metricsConfiguration);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37134c4) into [master](https://codecov.io/gh/apache/pinot/commit/c36d2b975041e08656598d69e3984bd9b3bb0c81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c36d2b9) will **decrease** coverage by `6.71%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.63% 65.91% -6.72%
- Complexity 3411 3414 +3
============================================
Files 1524 1478 -46
Lines 75699 73850 -1849
Branches 11039 10839 -200
============================================
- Hits 54981 48676 -6305
- Misses 17017 21706 +4689
+ Partials 3701 3468 -233
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.87% <0.00%> (+<0.01%)` | :arrow_up: |
| unittests2 | `15.31% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `89.33% <0.00%> (+1.17%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [350 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c36d2b9...37134c4](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jackjlli commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726727827
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
+1 on that. If there is such config specified from metricsConfiguration, then use the default one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726688520
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
+ @Nullable String configuredMetricsFactoryClassName) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
+
+ String metricsFactoryClassName = configuredMetricsFactoryClassName == null
+ ? DEFAULT_METRICS_FACTORY_CLASS_NAME : configuredMetricsFactoryClassName;
+
for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+ if (clazz.getName().equals(metricsFactoryClassName)) {
Review comment:
May be do an upfront `find` instead for better readability, and then just register the one class that needs to?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
We already have metricsConfiguration, can the config be part of that?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
+ @Nullable String configuredMetricsFactoryClassName) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
+
+ String metricsFactoryClassName = configuredMetricsFactoryClassName == null
+ ? DEFAULT_METRICS_FACTORY_CLASS_NAME : configuredMetricsFactoryClassName;
Review comment:
Can we log which one was picked?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv merged pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
mayankshriv merged pull request #7551:
URL: https://github.com/apache/pinot/pull/7551
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727511389
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
##########
@@ -28,7 +28,7 @@
public class MinionConf extends PinotConfiguration {
public static final String END_REPLACE_SEGMENTS_TIMEOUT_MS_KEY = "pinot.minion.endReplaceSegments.timeoutMs";
public static final int DEFAULT_END_REPLACE_SEGMENTS_SOCKET_TIMEOUT_MS = 10 * 60 * 1000; // 10 mins
-
+
Review comment:
nit: extra space
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e8a4c5b) into [master](https://codecov.io/gh/apache/pinot/commit/493ed2c0433fdc40f605f0b06ce5f6f98ce62725?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (493ed2c) will **decrease** coverage by `6.70%`.
> The diff coverage is `64.70%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.54% 65.84% -6.71%
+ Complexity 3416 3415 -1
============================================
Files 1522 1476 -46
Lines 75905 74059 -1846
Branches 11093 10892 -201
============================================
- Hits 55069 48765 -6304
- Misses 17144 21834 +4690
+ Partials 3692 3460 -232
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.83% <73.33%> (-0.02%)` | :arrow_down: |
| unittests2 | `15.23% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-78.19%)` | :arrow_down: |
| [.../main/java/org/apache/pinot/minion/MinionConf.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vTWluaW9uQ29uZi5qYXZh) | `15.38% <0.00%> (-76.29%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.00% <73.33%> (+1.84%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [356 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [493ed2c...e8a4c5b](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv merged pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
mayankshriv merged pull request #7551:
URL: https://github.com/apache/pinot/pull/7551
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-939199476
Shall we add a config to specify the metricsRegistry to load?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e7a0b52) into [master](https://codecov.io/gh/apache/pinot/commit/c36d2b975041e08656598d69e3984bd9b3bb0c81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c36d2b9) will **decrease** coverage by `1.23%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.63% 71.39% -1.24%
- Complexity 3411 3424 +13
============================================
Files 1524 1522 -2
Lines 75699 75893 +194
Branches 11039 11087 +48
============================================
- Hits 54981 54184 -797
- Misses 17017 18028 +1011
+ Partials 3701 3681 -20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `28.98% <66.66%> (-0.10%)` | :arrow_down: |
| unittests1 | `69.92% <75.00%> (+0.06%)` | :arrow_up: |
| unittests2 | `15.20% <18.18%> (-0.10%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.12% <75.00%> (+1.96%)` | :arrow_up: |
| [...e/pinot/broker/broker/helix/BaseBrokerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jhc2VCcm9rZXJTdGFydGVyLmphdmE=) | `73.74% <100.00%> (+0.14%)` | :arrow_up: |
| [...apache/pinot/controller/BaseControllerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9CYXNlQ29udHJvbGxlclN0YXJ0ZXIuamF2YQ==) | `82.69% <100.00%> (-0.91%)` | :arrow_down: |
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `78.37% <100.00%> (+0.19%)` | :arrow_up: |
| [...rg/apache/pinot/server/starter/ServerInstance.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9TZXJ2ZXJJbnN0YW5jZS5qYXZh) | `88.78% <100.00%> (-0.84%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [130 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c36d2b9...e7a0b52](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration, metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the desired metrics factory class. Will
+ * default to Yammer if not provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration. But the reason why I decided not to was because the Broker, Controller and Server instance starters each call `.subset` on the `PinotConfiguration` for their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the issue is we want to create 1 global config value `pinot.metrics.factory.className` that is not contained within a metrics configuration prefix.
We could construct a new PinotConfiguration combine the exiting configs with the metrics factory config but seemed like a bit of overhead for the sake of combining 2 parameters. It would look something like this:
```
Map<String, Object> brokerMetricsConfiguration = _brokerConf.subset(Broker.METRICS_CONFIG_PREFIX).toMap();
Map<String, String> metricsFactoryConfiguration = Collections.singletonMap(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
_brokerConf.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME));
PinotConfiguration metricsConfiguration = new PinotConfiguration(brokerMetricsConfiguration, metricsFactoryConfiguration);
PinotMetricUtils.init(metricsConfiguration);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7551: Removing DropWizard metrics library from distribution
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#issuecomment-940224151
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7551](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e7a0b52) into [master](https://codecov.io/gh/apache/pinot/commit/c36d2b975041e08656598d69e3984bd9b3bb0c81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c36d2b9) will **decrease** coverage by `6.73%`.
> The diff coverage is `72.72%`.
[](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7551 +/- ##
============================================
- Coverage 72.63% 65.89% -6.74%
- Complexity 3411 3424 +13
============================================
Files 1524 1476 -48
Lines 75699 74041 -1658
Branches 11039 10887 -152
============================================
- Hits 54981 48789 -6192
- Misses 17017 21808 +4791
+ Partials 3701 3444 -257
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.92% <75.00%> (+0.06%)` | :arrow_up: |
| unittests2 | `15.20% <18.18%> (-0.10%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/pinot/minion/BaseMinionStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vQmFzZU1pbmlvblN0YXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-78.19%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `90.12% <75.00%> (+1.96%)` | :arrow_up: |
| [...e/pinot/broker/broker/helix/BaseBrokerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jhc2VCcm9rZXJTdGFydGVyLmphdmE=) | `71.50% <100.00%> (-2.09%)` | :arrow_down: |
| [...apache/pinot/controller/BaseControllerStarter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9CYXNlQ29udHJvbGxlclN0YXJ0ZXIuamF2YQ==) | `78.84% <100.00%> (-4.76%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [372 more](https://codecov.io/gh/apache/pinot/pull/7551/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c36d2b9...e7a0b52](https://codecov.io/gh/apache/pinot/pull/7551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] timsants commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727532941
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -70,21 +74,31 @@ public static void init(PinotConfiguration metricsConfiguration) {
private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
- for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+
+ String metricsFactoryClassName = metricsConfiguration.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
+ DEFAULT_METRICS_FACTORY_CLASS_NAME);
+ LOGGER.info("{} will be initialized as the PinotMetricsFactory", metricsFactoryClassName);
+
+ Optional<Class<?>> maybeClazz = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
Review comment:
Changing to `clazzFound`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7551: Adding config for metrics library
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r727477419
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -70,21 +74,31 @@ public static void init(PinotConfiguration metricsConfiguration) {
private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
Set<Class<?>> classes = getPinotMetricsFactoryClasses();
if (classes.size() > 1) {
- LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+ LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
}
- for (Class<?> clazz : classes) {
- MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
- LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
- if (annotation.enabled()) {
- try {
- PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
- pinotMetricsFactory.init(metricsConfiguration);
- registerMetricsFactory(pinotMetricsFactory);
- } catch (Exception e) {
- LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+
+ String metricsFactoryClassName = metricsConfiguration.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
+ DEFAULT_METRICS_FACTORY_CLASS_NAME);
+ LOGGER.info("{} will be initialized as the PinotMetricsFactory", metricsFactoryClassName);
+
+ Optional<Class<?>> maybeClazz = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
Review comment:
Perhaps better name than `maybeClazz`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org