Skip to content

Commit

Permalink
Correction to add the sum value into Timer metric. Introduced a new S…
Browse files Browse the repository at this point in the history
…umming interface
  • Loading branch information
Channyboy committed Oct 29, 2020
1 parent 30aa4e2 commit 4915786
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* @see <a href="http://www.johndcook.com/standard_deviation.html">Accurately computing running
* variance</a>
*/
public interface Histogram extends Metric, Sampling, Counting {
public interface Histogram extends Metric, Sampling, Counting, Summing {

/**
* Adds a recorded value.
Expand All @@ -52,11 +52,7 @@ public interface Histogram extends Metric, Sampling, Counting {
@Override
long getCount();

/**
* Returns the sum of values recorded.
*
* @return the sum of values recorded
*/
@Override
long getSum();

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
*
* The simple timer measures duration in nanoseconds.
*/
public interface SimpleTimer extends Metric, Counting {
public interface SimpleTimer extends Metric, Counting, Summing {
/**
* A timing context.
*
Expand Down Expand Up @@ -98,6 +98,12 @@ interface Context extends Closeable {

@Override
long getCount();

@Override
/**
* @return returns the cumulative recorded time durations in nanoseconds
*/
long getSum();

/**
* Get the maximum recorded time duration of the SimpleTimer for the previously completed full minute.
Expand Down
38 changes: 38 additions & 0 deletions api/src/main/java/org/eclipse/microprofile/metrics/Summing.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* ********************************************************************
* Copyright (c) 2020 Contributors to the Eclipse Foundation
*
* See the NOTICES file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* You may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* ********************************************************************
*
*/
package org.eclipse.microprofile.metrics;

/**
* An interface for metric types which have summable values.
*/
public interface Summing {

/**
* Returns the sum of values recorded
*
* @return the sum of values recorded
*/
long getSum();

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
*
* The timer measures duration in nanoseconds.
*/
public interface Timer extends Metered, Sampling {
public interface Timer extends Metered, Sampling, Summing {
/**
* A timing context.
*
Expand Down Expand Up @@ -100,6 +100,12 @@ interface Context extends Closeable {
@Override
long getCount();

@Override
/**
* @return returns the cumulative recorded time durations in nanoseconds
*/
long getSum();

@Override
double getFifteenMinuteRate();

Expand Down
14 changes: 8 additions & 6 deletions spec/src/main/asciidoc/changelog.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,18 @@
*** Added `Timer.getElapsedTime()` which returns `java.time.Duration`
** Removed `MetadataBuilder.withOptional*` methods
** Global tags and `_app` tag are no longer handled automatically by the `MetricID` class, the implementation is expected to add them by itself, for example during metric export
** Added the `Histogram.getSum()` which returns `long` (https://github.com/eclipse/microprofile-metrics/issues/597[#597)
** Introduce the `Summable` interface that contains the `getSum()` method which returns long. This interface is extended by the `Timer`, `Histogram` and `SimpleTimer` interfaces. (https://github.com/eclipse/microprofile-metrics/issues/597[#597])
*** Added the `Histogram.getSum()`
*** Added the `Timer.getSum()` which returns the duration in nanoseconds
*** Added the `SimpleTimer.getSum()` which returns the duration in nanoseconds

=== Functional Changes
** Simple Timer metrics now track the highest and lowest recorded timing duration of the previous completed minute (https://github.com/eclipse/microprofile-metrics/issues/523[#523])
** Timer now exposes total elapsed time duration as a metric value (https://github.com/eclipse/microprofile-metrics/issues/524[#524])
** Timer now exposes total elapsed time duration as a metric value. (https://github.com/eclipse/microprofile-metrics/issues/524[#524])
*** As of RC3 this value is exposed as a `sum` value in JSON format and is exposed as the `<metric_name>_<units>_sum`under the `summary` type in OpenMetrics format (https://github.com/eclipse/microprofile-metrics/issues/597[#597])
** Clarified that the existing REST metric `REST.request` will not monitor and track a REST request to a REST endpoint if an unmapped exception occurs.
** Introduced a new base REST metric `REST.request.unmappedException.total` that counts the occurrences of unmapped exceptions for each REST endpoint (https://github.com/eclipse/microprofile-metrics/issues/533[#533])
** Histogram now exposes the total sum of recorded values (https://github.com/eclipse/microprofile-metrics/issues/597[#597)
** Histogram and Timer now exposes the total sum of recorded values as a `sum` value (https://github.com/eclipse/microprofile-metrics/issues/597[#597])

=== Specification Changes
** Removed the concept of reusability
Expand All @@ -83,9 +87,7 @@
but the type is implied by the name of the registration method that is being called.
** Clarified that the existing REST metric `REST.request` will not monitor and track a REST request to a REST endpoint if an unmapped exception occurs
** Introduced a new base REST metric `REST.request.unmappedException.total` that counts the occurrences of unmapped exceptions for each REST endpoint (https://github.com/eclipse/microprofile-metrics/issues/533[#533])
issues/533[#533])
** Histogram now exposes the total sum of recorded values (https://github.com/eclipse/microprofile-metrics/issues/597[#297)
*** In OpenMetric's format the `sum` value is included under the `summary` type
** Histogram and Timer now exposes the total sum of recorded values as a `sum` value (https://github.com/eclipse/microprofile-metrics/issues/597[#597])

=== TCK enhancement
** Improved TCK - Use newly introduced `MetricRegistry` methods to retrieve single metrics and avoid use of the `getMetrics()` and `getMetadata()` methods
Expand Down
11 changes: 5 additions & 6 deletions spec/src/main/asciidoc/rest-endpoints.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ The JSON node is named `<metric-name>`. The JSON leafs are named `<key>[';'<tag-
| JSON Key | Value (Equivalent Timer method)

| `count` | `getCount()`
| `elapsedTime` | `getElapsedTime()`
| `sum` | `getSum()`
| `meanRate` | `getMeanRate()`
| `oneMinRate` | `getOneMinuteRate()`
| `fiveMinRate` | `getFiveMinuteRate()`
Expand All @@ -315,7 +315,7 @@ The JSON node is named `<metric-name>`. The JSON leafs are named `<key>[';'<tag-
{
"responseTime": {
"count": 29382,
"elapsedTime": 25608694,
"sum": 25608694,
"meanRate": 12.185627192860734,
"oneMinRate": 12.563,
"fiveMinRate": 12.364,
Expand All @@ -331,7 +331,7 @@ The JSON node is named `<metric-name>`. The JSON leafs are named `<key>[';'<tag-
"p99": 5608694.0,
"p999": 5608694.0,
"count;servlet=two": 29382,
"elapsedTime;servlet=two": 25608694,
"sum;servlet=two": 25608694,
"meanRate;servlet=two":12.185627192860734,
"oneMinRate;servlet=two": 12.563,
"fiveMinRate;servlet=two": 12.364,
Expand Down Expand Up @@ -724,12 +724,12 @@ The `quantile` OpenMetrics label is merged with the metric's tags.
| `one_min_rate_per_second` | Gauge | `getOneMinuteRate()` | PER_SECOND
| `five_min_rate_per_second` | Gauge | `getFiveMinuteRate()` | PER_SECOND
| `fifteen_min_rate_per_second` | Gauge | `getFifteenMinuteRate()` | PER_SECOND
| `elapsedTime_seconds` | Gauge | `getElapsedTime()` | SECONDS^1^
| `min_seconds` | Gauge | `getSnapshot().getMin()` | SECONDS^1^
| `max_seconds` | Gauge | `getSnapshot().getMax()` | SECONDS^1^
| `mean_seconds` | Gauge | `getSnapshot().getMean()` | SECONDS^1^
| `stddev_seconds` | Gauge | `getSnapshot().getStdDev()` | SECONDS^1^
| `seconds_count`^2^ | Summary | `getCount()` | N/A
| `seconds_sum`^2^ | Summary | `getSum()` | SECONDS^1^
| `seconds{quantile="0.5"}`^2^ | Summary | `getSnapshot().getMedian()` | SECONDS^1^
| `seconds{quantile="0.75"}`^2^ | Summary | `getSnapshot().get75thPercentile()` | SECONDS^1^
| `seconds{quantile="0.95"}`^2^ | Summary | `getSnapshot().get95thPercentile()` | SECONDS^1^
Expand All @@ -753,8 +753,6 @@ application_response_time_one_min_rate_per_second 2.794076465421066E-14
application_response_time_five_min_rate_per_second 4.800392614619373E-4
# TYPE application_response_time_fifteen_min_rate_per_second gauge
application_response_time_fifteen_min_rate_per_second 0.01063191047532505
# TYPE application_response_time_elapsedTime_seconds gauge
application_response_time_elapsedTime_seconds 0.023
# TYPE application_response_time_mean_seconds gauge
application_response_time_mean_seconds 0.000415041
# TYPE application_response_time_max_seconds gauge
Expand All @@ -766,6 +764,7 @@ application_response_time_stddev_seconds 0.000652907
# TYPE application_response_time_seconds summary
# HELP application_response_time_seconds Server response time for /index.html
application_response_time_seconds_count 80
application_response_time_seconds_sum 0.023
application_response_time_seconds{quantile="0.5"} 0.0002933240
application_response_time_seconds{quantile="0.75"} 0.000344914
application_response_time_seconds{quantile="0.95"} 0.000543647
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -117,6 +118,8 @@ public void callSimplyTimedMethodOnce() throws InterruptedException {
// Make sure that the simpleTimer has been called
assertThat("SimplyTimed count is incorrect", simpleTimer.getCount(), is(equalTo(SIMPLE_TIMER_COUNT.incrementAndGet())));
TestUtils.assertEqualsWithTolerance(2000000000L, simpleTimer.getElapsedTime().toNanos());
assertEquals("The SimpleTimer's sum and elapsedTime values are supposed to be the same value", simpleTimer.getSum(),
simpleTimer.getElapsedTime().toNanos());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -111,6 +112,8 @@ public void callSimplyTimedMethodOnce() throws InterruptedException {
// Make sure that the simpleTimer has been called
assertThat("SimpleTimer count is incorrect", simpleTimer.getCount(), is(equalTo(SIMPLE_TIMER_COUNT.incrementAndGet())));
TestUtils.assertEqualsWithTolerance(2000000000L, simpleTimer.getElapsedTime().toNanos());
assertEquals("The SimpleTimer's sum and elapsedTime values are supposed to be the same value", simpleTimer.getSum(),
simpleTimer.getElapsedTime().toNanos());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -109,6 +110,7 @@ public void callTimedMethodOnce() throws InterruptedException {
// Make sure that the timer has been called
assertThat("Timer count is incorrect", timer.getCount(), is(equalTo(TIMER_COUNT.incrementAndGet())));
TestUtils.assertEqualsWithTolerance(2000000000L, timer.getElapsedTime().toNanos());
assertEquals("The Timer's sum and elapsedTime values are supposed to be the same value", timer.getSum(), timer.getElapsedTime().toNanos());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -102,6 +103,7 @@ public void callTimedMethodOnce() throws InterruptedException {
// Make sure that the timer has been called
assertThat("Timer count is incorrect", timer.getCount(), is(equalTo(TIMER_COUNT.incrementAndGet())));
TestUtils.assertEqualsWithTolerance(2000000000L, timer.getElapsedTime().toNanos());
assertEquals("The Timer's sum and elapsedTime values are supposed to be the same value", timer.getSum(), timer.getElapsedTime().toNanos());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ public void testApplicationMetricsJSON() {
.body("'metricTest.test1.timer'", hasKey("fiveMinRate;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("meanRate;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("oneMinRate;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("elapsedTime;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("sum;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("max;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("mean;tier=integration"))
.body("'metricTest.test1.timer'", hasKey("min;tier=integration"))
Expand All @@ -574,7 +574,7 @@ public void testApplicationMetricsJSON() {
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("fiveMinRate;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("meanRate;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("oneMinRate;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("elapsedTime;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("sum;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("max;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("mean;tier=integration"))
.body("'org.eclipse.microprofile.metrics.test.MetricAppBean.timeMeA'", hasKey("min;tier=integration"))
Expand Down Expand Up @@ -692,11 +692,11 @@ public void testApplicationTimerUnitOpenMetrics() {
.and()
.body(containsString("# TYPE application_" + prefix + "seconds summary"))
.body(containsString(prefix + "seconds_count"))
.body(containsString(prefix + "seconds_sum"))
.body(containsString(prefix + "rate_per_second"))
.body(containsString(prefix + "one_min_rate_per_second"))
.body(containsString(prefix + "five_min_rate_per_second"))
.body(containsString(prefix + "fifteen_min_rate_per_second"))
.body(containsString(prefix + "elapsedTime"))
.body(containsString(prefix + "mean_seconds"))
.body(containsString(prefix + "min_seconds"))
.body(containsString(prefix + "max_seconds"))
Expand Down Expand Up @@ -1130,7 +1130,7 @@ public void testMultipleTaggedMetricsJSON() {
.body("'taggedTimer'", hasKey("fiveMinRate;tier=integration"))
.body("'taggedTimer'", hasKey("meanRate;tier=integration"))
.body("'taggedTimer'", hasKey("oneMinRate;tier=integration"))
.body("'taggedTimer'", hasKey("elapsedTime;tier=integration"))
.body("'taggedTimer'", hasKey("sum;tier=integration"))
.body("'taggedTimer'", hasKey("max;tier=integration"))
.body("'taggedTimer'", hasKey("mean;tier=integration"))
.body("'taggedTimer'", hasKey("min;tier=integration"))
Expand All @@ -1147,7 +1147,7 @@ public void testMultipleTaggedMetricsJSON() {
.body("'taggedTimer'", hasKey("fiveMinRate;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("meanRate;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("oneMinRate;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("elapsedTime;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("sum;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("max;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("mean;number=one;tier=integration"))
.body("'taggedTimer'", hasKey("min;number=one;tier=integration"))
Expand All @@ -1164,7 +1164,7 @@ public void testMultipleTaggedMetricsJSON() {
.body("'taggedTimer'", hasKey("fiveMinRate;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("meanRate;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("oneMinRate;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("elapsedTime;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("sum;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("max;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("mean;number=two;tier=integration"))
.body("'taggedTimer'", hasKey("min;number=two;tier=integration"))
Expand Down

0 comments on commit 4915786

Please sign in to comment.