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 2022/07/28 01:20:23 UTC
[GitHub] [skywalking] sonatype-lift[bot] commented on a diff in pull request #9377: Add `SUM_PER_MIN`(sumHistogramPercentile) downsampling in MAL. Add eBPF Network Profiling E2E
sonatype-lift[bot] commented on code in PR #9377:
URL: https://github.com/apache/skywalking/pull/9377#discussion_r931700862
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sumpermin/SumPerMinFunction.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.meter.function.sumpermin;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.UnexpectedException;
+import org.apache.skywalking.oap.server.core.analysis.manual.instance.InstanceTraffic;
+import org.apache.skywalking.oap.server.core.analysis.meter.Meter;
+import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity;
+import org.apache.skywalking.oap.server.core.analysis.meter.function.AcceptableValue;
+import org.apache.skywalking.oap.server.core.analysis.meter.function.MeterFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.LongValueHolder;
+import org.apache.skywalking.oap.server.core.analysis.metrics.Metrics;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.query.sql.Function;
+import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteData;
+import org.apache.skywalking.oap.server.core.storage.annotation.BanyanDB;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity;
+import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage;
+import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder;
+
+import java.util.Objects;
+
+@ToString
+@MeterFunction(functionName = "sum_per_min")
+public abstract class SumPerMinFunction extends Meter implements AcceptableValue<Long>, LongValueHolder {
+ protected static final String VALUE = "value";
+ protected static final String TOTAL = "total";
+
+ @Setter
+ @Getter
+ @Column(columnName = ENTITY_ID, length = 512)
+ @BanyanDB.ShardingKey(index = 0)
+ private String entityId;
+
+ @Setter
+ @Getter
+ @Column(columnName = InstanceTraffic.SERVICE_ID)
+ private String serviceId;
+
+ @Getter
+ @Setter
+ @Column(columnName = VALUE, dataType = Column.ValueDataType.COMMON_VALUE, function = Function.Avg)
+ private long value;
+
+ @Getter
+ @Setter
+ @Column(columnName = TOTAL, storageOnly = true)
+ private long total;
+
+ @Entrance
+ public final void combine(@SourceFrom long value) {
+ this.total += value;
+ }
+
+ @Override
+ public boolean combine(Metrics metrics) {
+ final SumPerMinFunction sumPerMinFunction = (SumPerMinFunction) metrics;
+ combine(sumPerMinFunction.getTotal());
+ return true;
+ }
+
+ @Override
+ public void calculate() {
+ setValue(this.total / getDurationInMinute());
+ }
+
+ @Override
+ public Metrics toHour() {
+ final SumPerMinFunction metrics = (SumPerMinFunction) createNew();
+ metrics.setEntityId(getEntityId());
+ metrics.setTimeBucket(toTimeBucketInHour());
+ metrics.setServiceId(getServiceId());
+ metrics.setTotal(getTotal());
+ return metrics;
+ }
+
+ @Override
+ public Metrics toDay() {
+ final SumPerMinFunction metrics = (SumPerMinFunction) createNew();
+ metrics.setEntityId(getEntityId());
+ metrics.setTimeBucket(toTimeBucketInDay());
+ metrics.setServiceId(getServiceId());
+ metrics.setTotal(getTotal());
+ return metrics;
+ }
+
+ @Override
+ public int remoteHashCode() {
+ return getEntityId().hashCode();
+ }
+
+ @Override
+ public void deserialize(RemoteData remoteData) {
+ setTotal(remoteData.getDataLongs(0));
+ setTimeBucket(remoteData.getDataLongs(1));
+
+ setEntityId(remoteData.getDataStrings(0));
+ setServiceId(remoteData.getDataStrings(1));
+ }
+
+ @Override
+ public RemoteData.Builder serialize() {
+ final RemoteData.Builder remoteBuilder = RemoteData.newBuilder();
+
+ remoteBuilder.addDataLongs(getTotal());
+ remoteBuilder.addDataLongs(getTimeBucket());
+
+ remoteBuilder.addDataStrings(getEntityId());
+ remoteBuilder.addDataStrings(getServiceId());
+ return remoteBuilder;
+ }
+
+ @Override
+ protected String id0() {
+ return getTimeBucket() + Const.ID_CONNECTOR + getEntityId();
+ }
+
+ @Override
+ public void accept(MeterEntity entity, Long value) {
+ setEntityId(entity.id());
+ setServiceId(entity.serviceId());
+ setTotal(getTotal() + value);
+ }
+
+ @Override
+ public Class<? extends StorageBuilder> builder() {
+ return SumPerMinStorageBuilder.class;
+ }
+
+ public static class SumPerMinStorageBuilder implements StorageBuilder<SumPerMinFunction> {
+ @Override
+ public SumPerMinFunction storage2Entity(final Convert2Entity converter) {
+ final SumPerMinFunction metrics = new SumPerMinFunction() {
+ @Override
+ public AcceptableValue<Long> createNew() {
+ throw new UnexpectedException("createNew should not be called");
+ }
+ };
+ metrics.setValue(((Number) converter.get(VALUE)).longValue());
+ metrics.setTotal(((Number) converter.get(TOTAL)).longValue());
+ metrics.setTimeBucket(((Number) converter.get(TIME_BUCKET)).longValue());
+ metrics.setServiceId((String) converter.get(InstanceTraffic.SERVICE_ID));
+ metrics.setEntityId((String) converter.get(ENTITY_ID));
+ return metrics;
+ }
+
+ @Override
+ public void entity2Storage(final SumPerMinFunction storageData, final Convert2Storage converter) {
+ converter.accept(VALUE, storageData.getValue());
+ converter.accept(TOTAL, storageData.getTotal());
+ converter.accept(TIME_BUCKET, storageData.getTimeBucket());
+ converter.accept(InstanceTraffic.SERVICE_ID, storageData.getServiceId());
+ converter.accept(ENTITY_ID, storageData.getEntityId());
+ }
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof SumPerMinFunction)) {
+ return false;
+ }
+ final SumPerMinFunction function = (SumPerMinFunction) o;
+ return Objects.equals(getEntityId(), function.getEntityId())
+ && Objects.equals(getTimeBucket(), function.getTimeBucket());
Review Comment:
*[ObjectEqualsForPrimitives](https://errorprone.info/bugpattern/ObjectEqualsForPrimitives):* Avoid unnecessary boxing by using plain == for primitive types.
---
```suggestion
&& (getTimeBucket() == function.getTimeBucket());
```
---
Reply with *"**@sonatype-lift help**"* for info about LiftBot commands.
Reply with *"**@sonatype-lift ignore**"* to tell LiftBot to leave out the above finding from this PR.
Reply with *"**@sonatype-lift ignoreall**"* to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to **refresh** the page to see its response. [Click here](https://help.sonatype.com/lift/talking-to-lift) to get to know more about LiftBot commands.
---
Was this a good recommendation?
[ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=305706917&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=305706917&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=305706917&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=305706917&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=305706917&lift_comment_rating=5) ]
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sumpermin/SumPerMinFunction.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * 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.meter.function.sumpermin;
+
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.UnexpectedException;
+import org.apache.skywalking.oap.server.core.analysis.manual.instance.InstanceTraffic;
+import org.apache.skywalking.oap.server.core.analysis.meter.Meter;
+import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity;
+import org.apache.skywalking.oap.server.core.analysis.meter.function.AcceptableValue;
+import org.apache.skywalking.oap.server.core.analysis.meter.function.MeterFunction;
+import org.apache.skywalking.oap.server.core.analysis.metrics.LongValueHolder;
+import org.apache.skywalking.oap.server.core.analysis.metrics.Metrics;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.Entrance;
+import org.apache.skywalking.oap.server.core.analysis.metrics.annotation.SourceFrom;
+import org.apache.skywalking.oap.server.core.query.sql.Function;
+import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteData;
+import org.apache.skywalking.oap.server.core.storage.annotation.BanyanDB;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity;
+import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage;
+import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder;
+
+import java.util.Objects;
+
+@ToString
+@MeterFunction(functionName = "sum_per_min")
+public abstract class SumPerMinFunction extends Meter implements AcceptableValue<Long>, LongValueHolder {
+ protected static final String VALUE = "value";
+ protected static final String TOTAL = "total";
+
+ @Setter
+ @Getter
+ @Column(columnName = ENTITY_ID, length = 512)
+ @BanyanDB.ShardingKey(index = 0)
+ private String entityId;
+
+ @Setter
+ @Getter
+ @Column(columnName = InstanceTraffic.SERVICE_ID)
+ private String serviceId;
+
+ @Getter
Review Comment:
💬 2 similar findings have been found in this PR
---
*[MissingOverride](https://errorprone.info/bugpattern/MissingOverride):* getValue implements method in LongValueHolder; expected @Override
---
```suggestion
@Override @Getter
```
---
<details><summary><b>Expand here to view all instances of this finding</b></summary><br/>
<div align="center">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/EntityDescription/InstanceEntityDescription.java | [36](https://github.com/mrproliu/skywalking/blob/72fdb9857f7e852c254f665ad660071224aea253/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/EntityDescription/InstanceEntityDescription.java#L36)|
| oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sumpermin/SumPerMinFunction.java | [52](https://github.com/mrproliu/skywalking/blob/72fdb9857f7e852c254f665ad660071224aea253/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sumpermin/SumPerMinFunction.java#L52)|
<p><a href="https://lift.sonatype.com/results/github.com/mrproliu/skywalking/01G913XRTN16Y4MWNMF007VJR7?t=ErrorProne|MissingOverride" target="_blank">Visit the Lift Web Console</a> to find more details in your report.</p></div></details>
---
Reply with *"**@sonatype-lift help**"* for info about LiftBot commands.
Reply with *"**@sonatype-lift ignore**"* to tell LiftBot to leave out the above finding from this PR.
Reply with *"**@sonatype-lift ignoreall**"* to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to **refresh** the page to see its response. [Click here](https://help.sonatype.com/lift/talking-to-lift) to get to know more about LiftBot commands.
---
Was this a good recommendation?
[ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=305706908&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=305706908&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=305706908&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=305706908&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=305706908&lift_comment_rating=5) ]
--
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