RFR: 8203357 Container Metrics

[Bob Vandette]

> On May 31, 2018, at 11:36 PM, mandy chung <mandy.chung at oracle.com> wrote:
> 
> Hi Bob,
> 
> On 5/30/18 12:45 PM, Bob Vandette wrote:>
>> RFE: Container Metrics
>> https://bugs.openjdk.java.net/browse/JDK-8203357
>> WEBREV:
>> http://cr.openjdk.java.net/~bobv/8203357/webrev.00

Thanks for the review, here an updated webrev:

 http://cr.openjdk.java.net/~bobv/8203357/webrev.01/

> 
> Looks fine in general.  It's good to have this internal API ready
> for JFR and other library code to use.
> 
> I skimmed through the new tests.  It'd be good to add some comments
> to describe what it does (for example, set up a docker image etc).

DockerTestUtils.java does contain some comments describing what the
utility functions do.  I’ll add a brief comment in TestDockerCpuMetrics.java
and TestDockerMemoryMetrics.java explaining the test process.

> 
> launcher.properties
> 154 \    -XshowSettings:system (Linux Only) show host system or container\n\
> 155 \                      configuration and continue\n\
> 
> A newline can be placed after -XshowSettings:system consistent with
> other suboptions.

Done.  I also added a newline after the vm sub-option.

> 
> test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java
> 
> There are several long lines in the new test files such as:
>   MetricsCpuTester.java
>   MetricsMemoryTester.java
>   MetricsTester.java

> 
> It'd help future side-by-side review if they are wrapped. I think
> most of them are the construction of an exception.

Fixed.

> 
> I see a pattern of a name after @test and that is not strictly needed.
> 
> TestCgroupMetrics.java
>  25  * @test TestCgroupMetrics
> 
> TestDockerCpuMetrics.java
>  34  * @test TestSystemMetrics
> 
> TestDockerMemoryMetrics.java
>  30  * @test TestSystemMetrics
> 
> TestSystemMetrics.java
>  25  * @test TestSystemMetrics

Remove the names after @test.

> 
> This needs a CSR for the new -XshowSettings:system option.

I filed a CSR for this a few days ago.

https://bugs.openjdk.java.net/browse/JDK-8204107

Bob.



> 
> Mandy