You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2020/04/01 20:02:08 UTC
[GitHub] [storm] agresch opened a new pull request #3242: STORM-3614 switch
SystemBolt metrics to V2 API
agresch opened a new pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242
metrics-jvm metric sets contain all the GC/memory/thread info we were providing with some additional data. Replaced these metrics with their implementations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] Ethanlm commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r402598368
##########
File path: pom.xml
##########
@@ -295,6 +295,7 @@
<log4j.version>2.11.2</log4j.version>
<slf4j.version>1.7.26</slf4j.version>
<metrics.version>3.2.6</metrics.version>
+ <metrics.jvm.version>4.0.7</metrics.jvm.version>
Review comment:
Will there be any possible conflicts since it's pulling in two different major versions of dropwizard.metrics?
Even though they are for different artifacts, I am worried about transitive dependencies
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404871574
##########
File path: pom.xml
##########
@@ -295,6 +295,7 @@
<log4j.version>2.11.2</log4j.version>
<slf4j.version>1.7.26</slf4j.version>
<metrics.version>3.2.6</metrics.version>
+ <metrics.jvm.version>4.0.7</metrics.jvm.version>
Review comment:
I'll change the version.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404873491
##########
File path: docs/Metrics.md
##########
@@ -299,29 +299,28 @@ The value is a map where the key is a NodeInfo class for the downstream worker i
##### JVM Memory
-JVM memory usage is reported through `memory/nonHeap` for off heap memory and `memory/heap` for on heap memory. These values come from the [MemoryUsage](https://docs.oracle.com/javase/8/docs/api/index.html?java/lang/management/MemoryUsage.html) mxbean. Each of the metrics are reported as a map with the following keys, and values returned by the corresponding java code.
+JVM memory usage is reported through `memory.non-heap` for off heap memory, `memory.heap` for on heap memory and `memory.total` for combined values. These values come from the [MemoryUsage](https://docs.oracle.com/javase/8/docs/api/index.html?java/lang/management/MemoryUsage.html) mxbean. Each of the metrics are reported as a map with the following keys, and values returned by the corresponding java code.
| Key | Corresponding Code |
|--------|--------------------|
-| `maxBytes` | `memUsage.getMax()` |
-| `committedBytes` | `memUsage.getCommitted()` |
-| `initBytes` | `memUsage.getInit()` |
-| `usedBytes` | `memUsage.getUsed()` |
-| `virtualFreeBytes` | `memUsage.getMax() - memUsage.getUsed()` |
-| `unusedBytes` | `memUsage.getCommitted() - memUsage.getUsed()` |
+| `max` | `memUsage.getMax()` |
+| `committed` | `memUsage.getCommitted()` |
+| `init` | `memUsage.getInit()` |
+| `used` | `memUsage.getUsed()` |
+| `usage` | `Ratio.of(memUsage.getUsed(), memUsage.getMax())` |
##### JVM Garbage Collection
-The exact GC metric name depends on the garbage collector that your worker uses. The data is all collected from `ManagementFactory.getGarbageCollectorMXBeans()` and the name of the metrics is `"GC/"` followed by the name of the returned bean with white space removed. The reported metrics are just
+The exact GC metric name depends on the garbage collector that your worker uses. The data is all collected from `ManagementFactory.getGarbageCollectorMXBeans()` and the name of the metrics is `"GC"` followed by the name of the returned bean with white space removed. The reported metrics are just
Review comment:
They still do indirectly - https://github.com/infusionsoft/yammer-metrics/blob/master/metrics-jvm/src/main/java/com/codahale/metrics/jvm/GarbageCollectorMetricSet.java#L26
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] Ethanlm merged pull request #3242: STORM-3614 switch
SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
Ethanlm merged pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] Ethanlm commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r402608035
##########
File path: storm-core/test/clj/org/apache/storm/metrics_test.clj
##########
@@ -376,26 +376,5 @@
(assert-metric-running-sum! "mybolt" "__ack-count/myspout:default" 2 4 cluster)
(assert-metric-running-sum! "mybolt" "__execute-count/myspout:default" 3 4 cluster))))
-(deftest test-system-bolt
Review comment:
Do we want to add some unit tests in java code?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] Ethanlm commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404823024
##########
File path: docs/Metrics.md
##########
@@ -299,29 +299,28 @@ The value is a map where the key is a NodeInfo class for the downstream worker i
##### JVM Memory
-JVM memory usage is reported through `memory/nonHeap` for off heap memory and `memory/heap` for on heap memory. These values come from the [MemoryUsage](https://docs.oracle.com/javase/8/docs/api/index.html?java/lang/management/MemoryUsage.html) mxbean. Each of the metrics are reported as a map with the following keys, and values returned by the corresponding java code.
+JVM memory usage is reported through `memory.non-heap` for off heap memory, `memory.heap` for on heap memory and `memory.total` for combined values. These values come from the [MemoryUsage](https://docs.oracle.com/javase/8/docs/api/index.html?java/lang/management/MemoryUsage.html) mxbean. Each of the metrics are reported as a map with the following keys, and values returned by the corresponding java code.
| Key | Corresponding Code |
|--------|--------------------|
-| `maxBytes` | `memUsage.getMax()` |
-| `committedBytes` | `memUsage.getCommitted()` |
-| `initBytes` | `memUsage.getInit()` |
-| `usedBytes` | `memUsage.getUsed()` |
-| `virtualFreeBytes` | `memUsage.getMax() - memUsage.getUsed()` |
-| `unusedBytes` | `memUsage.getCommitted() - memUsage.getUsed()` |
+| `max` | `memUsage.getMax()` |
+| `committed` | `memUsage.getCommitted()` |
+| `init` | `memUsage.getInit()` |
+| `used` | `memUsage.getUsed()` |
+| `usage` | `Ratio.of(memUsage.getUsed(), memUsage.getMax())` |
##### JVM Garbage Collection
-The exact GC metric name depends on the garbage collector that your worker uses. The data is all collected from `ManagementFactory.getGarbageCollectorMXBeans()` and the name of the metrics is `"GC/"` followed by the name of the returned bean with white space removed. The reported metrics are just
+The exact GC metric name depends on the garbage collector that your worker uses. The data is all collected from `ManagementFactory.getGarbageCollectorMXBeans()` and the name of the metrics is `"GC"` followed by the name of the returned bean with white space removed. The reported metrics are just
Review comment:
These metrics are no longer from `ManagementFactory` right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on issue #3242: STORM-3614 switch
SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on issue #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#issuecomment-610018628
updated metrics documentation to reflect name changes and new threading metrics.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404317540
##########
File path: pom.xml
##########
@@ -295,6 +295,7 @@
<log4j.version>2.11.2</log4j.version>
<slf4j.version>1.7.26</slf4j.version>
<metrics.version>3.2.6</metrics.version>
+ <metrics.jvm.version>4.0.7</metrics.jvm.version>
Review comment:
I didn't see any problems when running a mvn dependency tree.
[INFO] +- io.dropwizard.metrics:metrics-jvm:jar:4.0.7:compile
[INFO] | +- (io.dropwizard.metrics:metrics-core:jar:3.2.6:compile - version managed from 4.0.7; omitted for duplicate)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404318810
##########
File path: storm-core/test/clj/org/apache/storm/metrics_test.clj
##########
@@ -376,26 +376,5 @@
(assert-metric-running-sum! "mybolt" "__ack-count/myspout:default" 2 4 cluster)
(assert-metric-running-sum! "mybolt" "__execute-count/myspout:default" 3 4 cluster))))
-(deftest test-system-bolt
Review comment:
I don't think it's worth the effort for getting this in. If you feel replacing this test is important enough, I would suggest opening a new JIRA.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] agresch commented on issue #3242: STORM-3614 switch
SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
agresch commented on issue #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#issuecomment-609976333
> Do we want to update the doc here https://github.com/apache/storm/blob/master/docs/Metrics.md ?
>
> Will the metric names change for memory metrics and thread metrics?
I will update this. Minor name changes and some additional metrics will be available.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [storm] Ethanlm commented on a change in pull request #3242:
STORM-3614 switch SystemBolt metrics to V2 API
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3242: STORM-3614 switch SystemBolt metrics to V2 API
URL: https://github.com/apache/storm/pull/3242#discussion_r404818266
##########
File path: pom.xml
##########
@@ -295,6 +295,7 @@
<log4j.version>2.11.2</log4j.version>
<slf4j.version>1.7.26</slf4j.version>
<metrics.version>3.2.6</metrics.version>
+ <metrics.jvm.version>4.0.7</metrics.jvm.version>
Review comment:
So there are two metrics-core in the dependencies, one is 4.0.7 and the other one is 3.2.6. I am afraid they could conflict each other at runtime since they are different at major version level.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services