You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "wankai123 (via GitHub)" <gi...@apache.org> on 2023/03/30 02:39:00 UTC

[GitHub] [skywalking] wankai123 opened a new pull request, #10616: Support isEmptyValue flag for metrics query.

wankai123 opened a new pull request, #10616:
URL: https://github.com/apache/skywalking/pull/10616

   And Fix `NPE` when query the not exist series indexes in ElasticSearch storage.
   
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [X] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #10501.
   - [X] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking] wu-sheng merged pull request #10616: Support isEmptyValue flag for metrics query.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #10616:
URL: https://github.com/apache/skywalking/pull/10616


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking] kezhenxu94 commented on a diff in pull request #10616: Support isEmptyValue flag for metrics query.

Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #10616:
URL: https://github.com/apache/skywalking/pull/10616#discussion_r1152701988


##########
test/e2e-v2/cases/php/expected/metrics-has-value.yml:
##########
@@ -15,5 +15,11 @@
 
 {{- contains . }}
 - key: {{ notEmpty .key }}
-  value: {{ ge .value 2 }}
+  value:
+    value: 0
+    isemptyvalue: true

Review Comment:
   Why do we need to assert that there is empty value?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking] wankai123 commented on a diff in pull request #10616: Support isEmptyValue flag for metrics query.

Posted by "wankai123 (via GitHub)" <gi...@apache.org>.
wankai123 commented on code in PR #10616:
URL: https://github.com/apache/skywalking/pull/10616#discussion_r1152704952


##########
test/e2e-v2/cases/php/expected/metrics-has-value.yml:
##########
@@ -15,5 +15,11 @@
 
 {{- contains . }}
 - key: {{ notEmpty .key }}
-  value: {{ ge .value 2 }}
+  value:
+    value: 0
+    isemptyvalue: true

Review Comment:
   Because the default query time range is lookback 30m, should have empty 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.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10616: Support isEmptyValue flag for metrics query.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10616:
URL: https://github.com/apache/skywalking/pull/10616#discussion_r1152751050


##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricsQueryService.java:
##########
@@ -50,7 +51,7 @@ private IMetricsQueryDAO getMetricQueryDAO() {
     /**
      * Read metrics single value in the duration of required metrics
      */
-    public long readMetricsValue(MetricsCondition condition, Duration duration) throws IOException {
+    public NullableValue readMetricsValue(MetricsCondition condition, Duration duration) throws IOException {

Review Comment:
   I think we don't change this API. Somehow we don't tested this?
   
   We just added a new API --> **readNullableMetricsValue**



##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricsQueryService.java:
##########
@@ -50,7 +51,7 @@ private IMetricsQueryDAO getMetricQueryDAO() {
     /**
      * Read metrics single value in the duration of required metrics
      */
-    public long readMetricsValue(MetricsCondition condition, Duration duration) throws IOException {
+    public NullableValue readMetricsValue(MetricsCondition condition, Duration duration) throws IOException {

Review Comment:
   I think we don't change this API. Somehow we didn't test this?
   
   We just added a new API --> **readNullableMetricsValue**



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking] wu-sheng commented on a diff in pull request #10616: Support isEmptyValue flag for metrics query.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #10616:
URL: https://github.com/apache/skywalking/pull/10616#discussion_r1152710104


##########
test/e2e-v2/cases/php/expected/metrics-has-value.yml:
##########
@@ -15,5 +15,11 @@
 
 {{- contains . }}
 - key: {{ notEmpty .key }}
-  value: {{ ge .value 2 }}
+  value:
+    value: 0
+    isemptyvalue: true

Review Comment:
   I think the point is, from a test perspective, the initial query should cover the time range from no telenmetry data to having data.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org