You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/01/10 14:15:51 UTC
[GitHub] [skywalking] wu-sheng opened a new pull request #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
wu-sheng opened a new pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214
## New query protocol
```graphql
# Query the type of metrics including multiple values, and format them as multiple linears.
# The seq of these multiple lines base on the calculation func in OAL
# Such as, should us this to query the result of func percentile(50,75,90,95,99) in OAL,
# then five lines will be responsed, p50 is the first element of return value.
getMultipleLinearIntValues(metric: MetricCondition!, numOfLinear: Int!, duration: Duration!): [IntValues!]!
```
- numOfLinear, represents how many lines expected. This related to the metrics name.
## New OAL
> all_percentile = from(All.latency).percentile(10);
This will replace all existing Pxx metrics, which makes indexes merged and the number of index decreased. `percentile` is a new func in OAL, which calculate p50/75/90/95/99 all, and clearly reduce the performance of Pxx calculation.
### One side effect
We will remove the old PXX metrics in default, will affect dashboard, topology, and metrics comparison pages. cc @Fine0830
### TODO
Add the e2e tests for new query protocol.
Resolves #4190
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365573087
##########
File path: docs/en/setup/backend/backend-alarm.md
##########
@@ -51,13 +54,22 @@ rules:
op: <
period: 10
count: 4
+ service_resp_time_percentile_rule:
+ # Metrics value need to be long, double or int
+ metrics-name: service_percentile
+ op: ">"
+ threshold: 1000,1000,1000,1000,1000
Review comment:
> Do you want me to put comments here? Or put doc at some places?
I prefer to point out the order here, users don't need to refer to other doc when they're uncertain with the order 😄
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365489283
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
Simply iterate over the “stream” without creating new object
----------------------------------------------------------------
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] [skywalking] codecov-io commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.55%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6879
Misses ? 18414
Partials ? 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `48.71% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...be9d4bd](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365564913
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
Sorry, my mistake. All thresholds must be set at current implementation.
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.46% 26.45% -0.02%
==========================================
Files 1179 1179
Lines 25828 25836 +8
Branches 3758 3760 +2
==========================================
- Hits 6835 6834 -1
- Misses 18394 18398 +4
- Partials 599 604 +5
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `100% <100%> (+25%)` | :arrow_up: |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `65.9% <42.85%> (+3.92%)` | :arrow_up: |
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (ø)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `59.01% <0%> (-18.04%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `67.85% <0%> (-17.86%)` | :arrow_down: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `28.08% <0%> (-3.38%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `80.88% <0%> (-1.48%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...cf6ea41](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365572760
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetricsQueryEsDAO.java
##########
@@ -121,7 +134,43 @@ protected void functionAggregation(Function function, TermsAggregationBuilder pa
return intValues;
}
- @Override public Thermodynamic getThermodynamic(String indName, Downsampling downsampling, List<String> ids, String valueCName) throws IOException {
+ @Override public IntValues[] getMultipleLinearIntValues(String indName, Downsampling downsampling,
+ List<String> ids, int numOfLinear, String valueCName) throws IOException {
+ String indexName = ModelName.build(downsampling, indName);
+
+ SearchResponse response = getClient().ids(indexName, ids.toArray(new String[0]));
+ Map<String, Map<String, Object>> idMap = toMap(response);
+
+ IntValues[] intValuesArray = new IntValues[numOfLinear];
+ for (int i = 0; i < intValuesArray.length; i++) {
+ intValuesArray[i] = new IntValues();
+ }
+
+ for (String id : ids) {
+ for (int i = 0; i < intValuesArray.length; i++) {
+ KVInt kvInt = new KVInt();
+ kvInt.setId(id);
+ kvInt.setValue(0);
+ intValuesArray[i].addKVInt(kvInt);
+ }
+
+ if (idMap.containsKey(id)) {
+ Map<String, Object> source = idMap.get(id);
+ IntKeyLongValueHashMap multipleValues = new IntKeyLongValueHashMap(5);
Review comment:
What do you expect? 5 should be suitable for 5-10 elements.
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365299327
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
What do you suggest?
----------------------------------------------------------------
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] [skywalking] aderm commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365490490
##########
File path: oap-server/server-bootstrap/src/main/resources/official_analysis.oal
##########
@@ -17,22 +17,14 @@
*/
// All scope metrics
-all_p99 = from(All.latency).p99(10);
-all_p95 = from(All.latency).p95(10);
-all_p90 = from(All.latency).p90(10);
-all_p75 = from(All.latency).p75(10);
-all_p50 = from(All.latency).p50(10);
Review comment:
Regarding pxx, we now use the percentile query at one time. From getMultipleLinearIntValues, we look at one index of the query, but now the back-end index is still divided into five Pxx. This is the next step of transformation? Or is it a merged index that has already been queried?
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/75b9446827a44842ed3a3a562b607f4c6b412ae0?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=======================================
Coverage 26.47% 26.47%
=======================================
Files 1179 1179
Lines 25908 25908
Branches 3781 3781
=======================================
Hits 6858 6858
Misses 18435 18435
Partials 615 615
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [75b9446...b1cc881](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.46%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6856
Misses ? 18436
Partials ? 616
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `43.58% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...857a9a9](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365267087
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
Review comment:
```suggestion
* PercentileMetrics is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
```
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365297516
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
This is only a fail-safe mechanism, in case user doesn't set enough values.
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.55%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6879
Misses ? 18414
Partials ? 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `48.71% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...be9d4bd](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365572111
##########
File path: docs/en/setup/backend/backend-alarm.md
##########
@@ -13,7 +13,11 @@ Alarm rule is constituted by following keys
endpoint name.
- **Exclude names**. The following entity names are excluded in this rule. Such as Service name,
endpoint name.
-- **Threshold**. The target value.
+- **Threshold**. The target value.
+For multiple values metrics, such as **percentile**, the threshold is an array. Described like `value1, value2, value3, value4, value5`.
+Each value could the threshold for each value of the metrics. Set the value to `-` if don't want to trigger alarm by this or some of the values.
+Such as in **percentile**, `value1` is threshold of P50, and `-, -, value3, value4, value5` means, there is no threshold for P50 and P75 in percentile alarm rule.
Review comment:
[1] (for reference)
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573419032
I'll open a follow-up PR to do some refactoring work
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365572964
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetricsQueryEsDAO.java
##########
@@ -121,7 +134,43 @@ protected void functionAggregation(Function function, TermsAggregationBuilder pa
return intValues;
}
- @Override public Thermodynamic getThermodynamic(String indName, Downsampling downsampling, List<String> ids, String valueCName) throws IOException {
+ @Override public IntValues[] getMultipleLinearIntValues(String indName, Downsampling downsampling,
+ List<String> ids, int numOfLinear, String valueCName) throws IOException {
+ String indexName = ModelName.build(downsampling, indName);
+
+ SearchResponse response = getClient().ids(indexName, ids.toArray(new String[0]));
+ Map<String, Map<String, Object>> idMap = toMap(response);
+
+ IntValues[] intValuesArray = new IntValues[numOfLinear];
+ for (int i = 0; i < intValuesArray.length; i++) {
+ intValuesArray[i] = new IntValues();
+ }
+
+ for (String id : ids) {
+ for (int i = 0; i < intValuesArray.length; i++) {
+ KVInt kvInt = new KVInt();
+ kvInt.setId(id);
+ kvInt.setValue(0);
+ intValuesArray[i].addKVInt(kvInt);
+ }
+
+ if (idMap.containsKey(id)) {
+ Map<String, Object> source = idMap.get(id);
+ IntKeyLongValueHashMap multipleValues = new IntKeyLongValueHashMap(5);
Review comment:
> What do you expect? 5 should be suitable for 5-10 elements.
oops, didn't notice it is a map, I thought it was array mistakenly, ignore me 😄
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/75b9446827a44842ed3a3a562b607f4c6b412ae0?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=======================================
Coverage 26.47% 26.47%
=======================================
Files 1179 1179
Lines 25908 25908
Branches 3781 3781
=======================================
Hits 6858 6858
Misses 18435 18435
Partials 615 615
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [75b9446...c1f0605](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **decrease** coverage by `0.08%`.
> The diff coverage is `72.13%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.46% 26.38% -0.09%
==========================================
Files 1179 1179
Lines 25828 25833 +5
Branches 3758 3760 +2
==========================================
- Hits 6835 6815 -20
- Misses 18394 18420 +26
+ Partials 599 598 -1
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.27% <0%> (-0.72%)` | :arrow_down: |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `63.15% <0%> (-11.85%)` | :arrow_down: |
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (ø)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `59.01% <0%> (-18.04%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `67.85% <0%> (-17.86%)` | :arrow_down: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `28.08% <0%> (-3.38%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `80.88% <0%> (-1.48%)` | :arrow_down: |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `66.66% <0%> (-1.29%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...da11ad3](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365287588
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
Do we need `.toArray`?
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573052506
FYI @aderm
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365567950
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,38 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ Integer[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK:
+ for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ Integer iNullableExpected = 0;
+ if (iaexpected.length > i) {
+ iNullableExpected = iaexpected[i];
+ if (iNullableExpected == null) {
+ continue;
+ }
+ }
+ switch (op) {
+ case LESS:
+ if (ivalue < iNullableExpected) {
+ matchCount++;
+ break MULTI_VALUE_CHECK;
+ }
+ case GREATER:
+ if (ivalue > iNullableExpected) {
+ matchCount++;
+ break MULTI_VALUE_CHECK;
+ }
+ case EQUAL:
+ if (ivalue == iNullableExpected) {
+ matchCount++;
+ break MULTI_VALUE_CHECK;
+ }
Review comment:
Same here
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573283799
Maybe need to change avg response time? Due to cpm is less than 1?
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/1ed179c0bd7f1b179e83a33c024c2a8c3e1c8f36?src=pr&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `26.24%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.63% 26.46% -0.18%
==========================================
Files 1178 1179 +1
Lines 25773 25908 +135
Branches 3748 3781 +33
==========================================
- Hits 6865 6856 -9
- Misses 18292 18436 +144
Partials 616 616
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ap/server/exporter/provider/grpc/GRPCExporter.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9leHBvcnRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2V4cG9ydGVyL3Byb3ZpZGVyL2dycGMvR1JQQ0V4cG9ydGVyLmphdmE=) | `77.77% <0%> (-5.76%)` | :arrow_down: |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <0%> (-7.3%)` | :arrow_down: |
| [...server/core/storage/annotation/ValueColumnIds.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc3RvcmFnZS9hbm5vdGF0aW9uL1ZhbHVlQ29sdW1uSWRzLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...alking/oap/query/graphql/resolver/MetricQuery.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL01ldHJpY1F1ZXJ5LmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [.../storage/plugin/jdbc/h2/dao/H2MetricsQueryDAO.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1qZGJjLWhpa2FyaWNwLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2pkYmMvaDIvZGFvL0gyTWV0cmljc1F1ZXJ5REFPLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...alking/oap/server/core/query/entity/IntValues.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L0ludFZhbHVlcy5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [.../plugin/elasticsearch/query/MetricsQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvTWV0cmljc1F1ZXJ5RXNEQU8uamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
| [...king/oap/server/core/query/MetricQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvTWV0cmljUXVlcnlTZXJ2aWNlLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...p/server/core/alarm/provider/MetricsValueType.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9NZXRyaWNzVmFsdWVUeXBlLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `75% <20%> (-25%)` | :arrow_down: |
| ... and [8 more](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [1ed179c...857a9a9](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365490753
##########
File path: oap-server/server-bootstrap/src/main/resources/official_analysis.oal
##########
@@ -17,22 +17,14 @@
*/
// All scope metrics
-all_p99 = from(All.latency).p99(10);
-all_p95 = from(All.latency).p95(10);
-all_p90 = from(All.latency).p90(10);
-all_p75 = from(All.latency).p75(10);
-all_p50 = from(All.latency).p50(10);
Review comment:
> This is the next step of transformation? Or is it a merged index that has already been queried?
UI will change after this merged. @Fine0830 will do that part.
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365518952
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
I copied most of the codes from Pxx, so, I prefer don't change it in this PR at least. If you want to change, let's move this to another new PR about code refactor, OK?
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573267833
To all reviewers
Currently, all tests are not including the new `percentile` func, and the old `Pxx` OAL scripts have been removed from the OAL scripts.
I and @kezhenxu94 will find a time today to fill the gap of e2e test.
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365328119
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(RANKS.length);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
+ for (IntKeyLongValue element : sortedData) {
+ count += element.getValue();
+ for (int i = index; i < roofs.length; i++) {
+ int roof = roofs[i];
+
+ if (count >= roof) {
Review comment:
Why? I am not following. Please provide test case to describe the concern.
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365489467
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
I am not familiar with the `stream` API. Do you have a proposal about how to change?
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365490173
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
Review comment:
Did not open in IDE just now, and the comparator can be simplified too, trivial though:
```java
dataset.values().stream().sorted(Comparator.comparingInt(IntKeyLongValue::getKey)).forEach(element -> {
count += element.getValue();
for (int i = index; i < roofs.length; i++) {
int roof = roofs[i];
if (count >= roof) {
percentileValues.put(index, new IntKeyLongValue(index, element.getKey() * precision));
index++;
} else {
break;
}
}
});
```
> Simply iterate over the “stream” without creating new object
Since you're using several variables that are non-final, it may make it uncomfortable to iterate over the `stream` directly, just ignore my previous comment, but the comparator can be simplified indeed
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269635
This is the query I used for testing
```
{
getMultipleLinearIntValues(metric : {
name: "all_percentile"
}, duration: {step:MINUTE, start: "2020-01-10 2300", end: "2020-01-10 2330"},
numOfLinear: 5) {
values {
id,
value
}
}
}
```
Then, I got the response
```
{
"data": {
"getMultipleLinearIntValues": [
{
"values": [
{
"id": "202001102300",
"value": 0
},
{
"id": "202001102301",
"value": 0
},
{
"id": "202001102302",
"value": 0
},
{
"id": "202001102303",
"value": 0
},
{
"id": "202001102304",
"value": 0
},
{
"id": "202001102305",
"value": 0
},
{
"id": "202001102306",
"value": 0
},
{
"id": "202001102307",
"value": 0
},
{
"id": "202001102308",
"value": 0
},
{
"id": "202001102309",
"value": 0
},
{
"id": "202001102310",
"value": 0
},
{
"id": "202001102311",
"value": 0
},
{
"id": "202001102312",
"value": 0
},
{
"id": "202001102313",
"value": 0
},
{
"id": "202001102314",
"value": 0
},
{
"id": "202001102315",
"value": 0
},
{
"id": "202001102316",
"value": 0
},
{
"id": "202001102317",
"value": 0
},
{
"id": "202001102318",
"value": 0
},
{
"id": "202001102319",
"value": 0
},
{
"id": "202001102320",
"value": 4500
},
{
"id": "202001102321",
"value": 0
},
{
"id": "202001102322",
"value": 0
},
{
"id": "202001102323",
"value": 0
},
{
"id": "202001102324",
"value": 0
},
{
"id": "202001102325",
"value": 0
},
{
"id": "202001102326",
"value": 0
},
{
"id": "202001102327",
"value": 0
},
{
"id": "202001102328",
"value": 0
},
{
"id": "202001102329",
"value": 0
},
{
"id": "202001102330",
"value": 0
}
]
},
.... Another 4 elements of array ignored
]
}
}
```
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365565043
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
> I haven't found a better way to support setting part of it, maybe set `-` to represent skipping?
Sounds good to me
----------------------------------------------------------------
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] [skywalking] aderm edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573100138
can add default？ `getMultipleLinearIntValues(metric: MetricCondition!, duration: Duration!): [IntValues!]!` ，most are all. If the back end pxx is increased, the front end does not need to be adjusted
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365320285
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
Yes. Or set to 0, then percentile alarm wouldn't be triggered by it. Because you will use `>` as OP.
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/dcb71cdeee7ad14188fcecb3674172e18d0c6158?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `25.17%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
- Coverage 26.47% 26.37% -0.1%
=========================================
Files 1178 1179 +1
Lines 25693 25834 +141
Branches 3725 3760 +35
=========================================
+ Hits 6802 6815 +13
- Misses 18294 18420 +126
- Partials 597 599 +2
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ap/server/exporter/provider/grpc/GRPCExporter.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9leHBvcnRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2V4cG9ydGVyL3Byb3ZpZGVyL2dycGMvR1JQQ0V4cG9ydGVyLmphdmE=) | `77.77% <0%> (-5.76%)` | :arrow_down: |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `60.91% <0%> (-8.37%)` | :arrow_down: |
| [...server/core/storage/annotation/ValueColumnIds.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc3RvcmFnZS9hbm5vdGF0aW9uL1ZhbHVlQ29sdW1uSWRzLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...alking/oap/query/graphql/resolver/MetricQuery.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL01ldHJpY1F1ZXJ5LmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [.../storage/plugin/jdbc/h2/dao/H2MetricsQueryDAO.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1qZGJjLWhpa2FyaWNwLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2pkYmMvaDIvZGFvL0gyTWV0cmljc1F1ZXJ5REFPLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...alking/oap/server/core/query/entity/IntValues.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L0ludFZhbHVlcy5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [.../plugin/elasticsearch/query/MetricsQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvTWV0cmljc1F1ZXJ5RXNEQU8uamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
| [...king/oap/server/core/query/MetricQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvTWV0cmljUXVlcnlTZXJ2aWNlLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...p/server/core/alarm/provider/MetricsValueType.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9NZXRyaWNzVmFsdWVUeXBlLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `63.15% <12.5%> (-36.85%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [dcb71cd...d2ed279](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365567916
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,38 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ Integer[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK:
+ for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ Integer iNullableExpected = 0;
+ if (iaexpected.length > i) {
+ iNullableExpected = iaexpected[i];
+ if (iNullableExpected == null) {
+ continue;
+ }
+ }
+ switch (op) {
+ case LESS:
+ if (ivalue < iNullableExpected) {
+ matchCount++;
+ break MULTI_VALUE_CHECK;
+ }
Review comment:
Missing `break` after `if` statement
```suggestion
}
break;
```
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365298016
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
Review comment:
This should be optimized by the JDK, from my understanding.
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.46% 26.44% -0.02%
==========================================
Files 1179 1179
Lines 25828 25836 +8
Branches 3758 3760 +2
==========================================
- Hits 6835 6832 -3
- Misses 18394 18399 +5
- Partials 599 605 +6
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `100% <100%> (+25%)` | :arrow_up: |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `65.9% <42.85%> (+3.92%)` | :arrow_up: |
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (ø)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `59.01% <0%> (-18.04%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `67.85% <0%> (-17.86%)` | :arrow_down: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `28.08% <0%> (-3.38%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `80.88% <0%> (-1.48%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...35cf76e](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573414131
@Fine0830 I will merge this tomorrow, then ask @ascrutae to deploy on the demo UI. Which will break part of the demo UI, but should be good for your UI update.
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365571766
##########
File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetricsQueryEsDAO.java
##########
@@ -121,7 +134,43 @@ protected void functionAggregation(Function function, TermsAggregationBuilder pa
return intValues;
}
- @Override public Thermodynamic getThermodynamic(String indName, Downsampling downsampling, List<String> ids, String valueCName) throws IOException {
+ @Override public IntValues[] getMultipleLinearIntValues(String indName, Downsampling downsampling,
+ List<String> ids, int numOfLinear, String valueCName) throws IOException {
+ String indexName = ModelName.build(downsampling, indName);
+
+ SearchResponse response = getClient().ids(indexName, ids.toArray(new String[0]));
+ Map<String, Map<String, Object>> idMap = toMap(response);
+
+ IntValues[] intValuesArray = new IntValues[numOfLinear];
+ for (int i = 0; i < intValuesArray.length; i++) {
+ intValuesArray[i] = new IntValues();
+ }
+
+ for (String id : ids) {
+ for (int i = 0; i < intValuesArray.length; i++) {
+ KVInt kvInt = new KVInt();
+ kvInt.setId(id);
+ kvInt.setValue(0);
+ intValuesArray[i].addKVInt(kvInt);
+ }
+
+ if (idMap.containsKey(id)) {
+ Map<String, Object> source = idMap.get(id);
+ IntKeyLongValueHashMap multipleValues = new IntKeyLongValueHashMap(5);
Review comment:
I'm not very comfortable with the magic number here
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.55%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6879
Misses ? 18414
Partials ? 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `48.71% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...be9d4bd](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] aderm removed a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm removed a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573100138
can add default？ `getMultipleLinearIntValues(metric: MetricCondition!, duration: Duration!): [IntValues!]!` ，most are all. If the back end pxx is increased, the front end does not need to be adjusted
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365572898
##########
File path: docs/en/setup/backend/backend-alarm.md
##########
@@ -51,13 +54,22 @@ rules:
op: <
period: 10
count: 4
+ service_resp_time_percentile_rule:
+ # Metrics value need to be long, double or int
+ metrics-name: service_percentile
+ op: ">"
+ threshold: 1000,1000,1000,1000,1000
Review comment:
Do you want me to put comments here? Or put doc at some places?
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365316959
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
So the users are expected to set all thresholds for all percentile 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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/75b9446827a44842ed3a3a562b607f4c6b412ae0?src=pr&el=desc) will **decrease** coverage by `0.08%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.47% 26.38% -0.09%
==========================================
Files 1179 1179
Lines 25908 25828 -80
Branches 3781 3758 -23
==========================================
- Hits 6858 6815 -43
+ Misses 18435 18414 -21
+ Partials 615 599 -16
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../server/core/analysis/metrics/IntKeyLongValue.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9JbnRLZXlMb25nVmFsdWUuamF2YQ==) | `45.83% <0%> (-4.17%)` | :arrow_down: |
| [...pm/plugin/trace/ignore/matcher/AntPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvQW50UGF0aE1hdGNoZXIuamF2YQ==) | | |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <0%> (ø)` | |
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <0%> (+17.35%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [75b9446...8f31c41](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/75b9446827a44842ed3a3a562b607f4c6b412ae0?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
+ Coverage 26.47% 26.54% +0.07%
==========================================
Files 1179 1179
Lines 25908 25908
Branches 3781 3781
==========================================
+ Hits 6858 6877 +19
+ Misses 18435 18416 -19
Partials 615 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../server/core/analysis/metrics/IntKeyLongValue.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9JbnRLZXlMb25nVmFsdWUuamF2YQ==) | `45.83% <0%> (-4.17%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `82.35% <0%> (+1.47%)` | :arrow_up: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `31.46% <0%> (+3.37%)` | :arrow_up: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `85.71% <0%> (+17.85%)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `77.04% <0%> (+18.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [75b9446...c1f0605](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365565822
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
Done, please review.
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365571957
##########
File path: docs/en/setup/backend/backend-alarm.md
##########
@@ -13,7 +13,11 @@ Alarm rule is constituted by following keys
endpoint name.
- **Exclude names**. The following entity names are excluded in this rule. Such as Service name,
endpoint name.
-- **Threshold**. The target value.
+- **Threshold**. The target value.
+For multiple values metrics, such as **percentile**, the threshold is an array. Described like `value1, value2, value3, value4, value5`.
+Each value could the threshold for each value of the metrics. Set the value to `-` if don't want to trigger alarm by this or some of the values.
+Such as in **percentile**, `value1` is threshold of P50, and `-, -, value3, value4, value5` means, there is no threshold for P50 and P75 in percentile alarm rule.
+Could be an array, such as `value1, value2, value3` if the metrics are multiple values metrics. Element of the array could be NULL, then
Review comment:
Is this sentence completed? forget to update?
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573412403
@kezhenxu94 I add more docs to ease your concern, please recheck.
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365564964
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
I haven't found a better way to support setting part of it, maybe set `-` to represent skipping?
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.46%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6856
Misses ? 18437
Partials ? 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `43.58% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...df57014](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365572206
##########
File path: docs/en/setup/backend/backend-alarm.md
##########
@@ -51,13 +54,22 @@ rules:
op: <
period: 10
count: 4
+ service_resp_time_percentile_rule:
+ # Metrics value need to be long, double or int
+ metrics-name: service_percentile
+ op: ">"
+ threshold: 1000,1000,1000,1000,1000
Review comment:
I don't know how other people think of this, I'm confused with the order of the thresholds at first glance, `p99` -> `p50` or `p50` -> `p99`, and seems only [1] tells the order, vaguely though
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=======================================
Coverage 26.46% 26.46%
=======================================
Files 1179 1179
Lines 25828 25828
Branches 3758 3758
=======================================
Hits 6835 6835
Misses 18394 18394
Partials 599 599
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...8cf6147](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] aderm commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365322440
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(RANKS.length);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
+ for (IntKeyLongValue element : sortedData) {
+ count += element.getValue();
+ for (int i = index; i < roofs.length; i++) {
+ int roof = roofs[i];
+
+ if (count >= roof) {
Review comment:
may be if condition `count >= roofs[i] && count<roofs[i+1]`? also change for i;
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365266365
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/MultiIntValuesHolder.java
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+/**
+ * IntValueHolder always holds a set of int(s).
Review comment:
```suggestion
* MultiIntValuesHolder always holds a set of int(s).
```
----------------------------------------------------------------
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] [skywalking] aderm commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269337
> > can add default？
>
> What do you mean?
@wu-sheng Wrongly written, can be ignored
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365286579
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
Review comment:
```suggestion
percentileValues = new IntKeyLongValueHashMap(RANKS.length);
```
----------------------------------------------------------------
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] [skywalking] aderm commented on a change in pull request #4214:
Support multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365588946
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(RANKS.length);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
+ }
+
+ int count = 0;
+ IntKeyLongValue[] sortedData = dataset.values().stream().sorted(new Comparator<IntKeyLongValue>() {
+ @Override public int compare(IntKeyLongValue o1, IntKeyLongValue o2) {
+ return o1.getKey() - o2.getKey();
+ }
+ }).toArray(IntKeyLongValue[]::new);
+ for (IntKeyLongValue element : sortedData) {
+ count += element.getValue();
+ for (int i = index; i < roofs.length; i++) {
+ int roof = roofs[i];
+
+ if (count >= roof) {
Review comment:
Should be correct, logical thinking is a bit problematic. above roofs should all hit
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.55%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6879
Misses ? 18414
Partials ? 615
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `48.71% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...be9d4bd](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573282859
@kezhenxu94 TTL still fails, I have changed the metrics from p99 to cpm, But still fail. What do I miss?
----------------------------------------------------------------
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] [skywalking] aderm commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573100138
can add default？ `getMultipleLinearIntValues(metric: MetricCondition!, duration: Duration!): [IntValues!]!` ，most are all.
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365571790
##########
File path: oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2MetricsQueryDAO.java
##########
@@ -129,6 +143,48 @@ public IntValues getValues(String indName, Downsampling downsampling, long start
return orderWithDefault0(intValues, ids);
}
+ @Override public IntValues[] getMultipleLinearIntValues(String indName, Downsampling downsampling,
+ List<String> ids,
+ int numOfLinear,
+ String valueCName) throws IOException {
+ String tableName = ModelName.build(downsampling, indName);
+
+ StringBuilder idValues = new StringBuilder();
+ for (int valueIdx = 0; valueIdx < ids.size(); valueIdx++) {
+ if (valueIdx != 0) {
+ idValues.append(",");
+ }
+ idValues.append("'").append(ids.get(valueIdx)).append("'");
+ }
+
+ IntValues[] intValuesArray = new IntValues[numOfLinear];
+ for (int i = 0; i < intValuesArray.length; i++) {
+ intValuesArray[i] = new IntValues();
+ }
+
+ try (Connection connection = h2Client.getConnection()) {
+ try (ResultSet resultSet = h2Client.executeQuery(connection, "select id, " + valueCName + " from " + tableName + " where id in (" + idValues.toString() + ")")) {
+ while (resultSet.next()) {
+ String id = resultSet.getString("id");
+
+ IntKeyLongValueHashMap multipleValues = new IntKeyLongValueHashMap(5);
Review comment:
Same here
----------------------------------------------------------------
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] [skywalking] kezhenxu94 merged pull request #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214
----------------------------------------------------------------
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] [skywalking] wu-sheng commented on issue #4214: Support multiple
linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573101860
> can add default？
What do you mean?
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365283971
##########
File path: oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java
##########
@@ -343,6 +345,31 @@ private boolean isMatch() {
break;
}
break;
+ case MULTI_INTS:
+ int[] ivalueArray = ((MultiIntValuesHolder)metrics).getValues();
+ int[] iaexpected = RunningRule.this.threshold.getIntValuesThreshold();
+ MULTI_VALUE_CHECK: for (int i = 0; i < ivalueArray.length; i++) {
+ ivalue = ivalueArray[i];
+ iexpected = 0;
+ if (iaexpected.length > i) {
+ iexpected = iaexpected[i];
+ }
Review comment:
Is it reasonable to make it `0` by default? My intuition of the following configs:
```yml
threshold: 1000,1000
```
may be just monitor and alarm the `p99` and `p95` only, ignoring others percentile metrics, and your implementation seems to always send alarm? (because the other metrics may be almostly always > 0).
And are there any possibility when the users want to monitor `p99` and `p75`, how to config the threshold?
----------------------------------------------------------------
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] [skywalking] kezhenxu94 commented on a change in pull request
#4214: Support multiple linear values and merging p50/75/90/95/99 into
percentile
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#discussion_r365286956
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/PercentileMetrics.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ *
+ */
+
+
+package org.apache.skywalking.oap.server.core.analysis.metrics;
+
+import java.util.Comparator;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Arg;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.MetricsFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+
+/**
+ * Percentile is a better implementation than {@link PxxMetrics}. It is introduced since 7.0.0, it could calculate the
+ * multiple P50/75/90/95/99 values once for all.
+ *
+ * @author wusheng
+ */
+@MetricsFunction(functionName = "percentile")
+public abstract class PercentileMetrics extends GroupMetrics implements MultiIntValuesHolder {
+ protected static final String DATASET = "dataset";
+ protected static final String VALUE = "value";
+ protected static final String PRECISION = "precision";
+
+ private static final int[] RANKS = {50, 75, 90, 95, 99};
+
+ @Getter @Setter @Column(columnName = VALUE, isValue = true) private IntKeyLongValueHashMap percentileValues;
+ @Getter @Setter @Column(columnName = PRECISION) private int precision;
+ @Getter @Setter @Column(columnName = DATASET) private IntKeyLongValueHashMap dataset;
+
+ private boolean isCalculated;
+
+ public PercentileMetrics() {
+ percentileValues = new IntKeyLongValueHashMap(5);
+ dataset = new IntKeyLongValueHashMap(30);
+ }
+
+ @Entrance
+ public final void combine(@SourceFrom int value, @Arg int precision) {
+ this.isCalculated = false;
+ this.precision = precision;
+
+ int index = value / precision;
+ IntKeyLongValue element = dataset.get(index);
+ if (element == null) {
+ element = new IntKeyLongValue(index, 1);
+ dataset.put(element.getKey(), element);
+ } else {
+ element.addValue(1);
+ }
+ }
+
+ @Override
+ public void combine(Metrics metrics) {
+ this.isCalculated = false;
+
+ PercentileMetrics percentileMetrics = (PercentileMetrics)metrics;
+ combine(percentileMetrics.getDataset(), this.dataset);
+ }
+
+ @Override
+ public final void calculate() {
+
+ if (!isCalculated) {
+ int total = dataset.values().stream().mapToInt(element -> (int)element.getValue()).sum();
+
+ int index = 0;
+ int[] roofs = new int[RANKS.length];
+ for (int i = 0; i < RANKS.length; i++) {
+ roofs[i] = Math.round(total * RANKS[i] * 1.0f / 100);
Review comment:
may reduce one multiply operation, I'm not sure though?
```suggestion
roofs[i] = Math.round(total * RANKS[i] / 100.0);
```
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `68.75%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
- Coverage 26.46% 26.45% -0.01%
==========================================
Files 1179 1179
Lines 25828 25836 +8
Branches 3758 3760 +2
==========================================
Hits 6835 6835
- Misses 18394 18402 +8
Partials 599 599
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `60.22% <0%> (-1.77%)` | :arrow_down: |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `63.15% <0%> (-11.85%)` | :arrow_down: |
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (ø)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...da11ad3](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> Merging [#4214](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/b1e600712b9112daa300e25bd9e18f34486ae397?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `93.61%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=======================================
Coverage 26.46% 26.46%
=======================================
Files 1179 1179
Lines 25828 25828
Branches 3758 3758
=======================================
Hits 6835 6835
Misses 18394 18394
Partials 599 599
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (ø)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [b1e6007...567621d](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [skywalking] codecov-io edited a comment on issue #4214: Support
multiple linear values and merging p50/75/90/95/99 into percentile
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4214: Support multiple linear values and merging p50/75/90/95/99 into percentile
URL: https://github.com/apache/skywalking/pull/4214#issuecomment-573269476
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a2113b1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `11.11%`.
[](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4214 +/- ##
=========================================
Coverage ? 26.47%
=========================================
Files ? 1179
Lines ? 25908
Branches ? 3781
=========================================
Hits ? 6858
Misses ? 18436
Partials ? 614
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `61.98% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `43.58% <0%> (ø)` | |
| [...erver/core/analysis/metrics/PercentileMetrics.java](https://codecov.io/gh/apache/skywalking/pull/4214/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvbWV0cmljcy9QZXJjZW50aWxlTWV0cmljcy5qYXZh) | `87.5% <100%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=footer). Last update [a2113b1...e24e07d](https://codecov.io/gh/apache/skywalking/pull/4214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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