kezhenxu94 commented on code in PR #10449: URL: https://github.com/apache/skywalking/pull/10449#discussion_r1118252273
########## oap-server/server-receiver-plugin/otel-receiver-plugin/src/test/java/org/apache/skywalking/oap/server/receiver/otel/otlp/OpenTelemetryMetricRequestProcessorTest.java: ########## @@ -0,0 +1,134 @@ +/* + * 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.receiver.otel.otlp; + +import io.opentelemetry.proto.metrics.v1.ExponentialHistogram; +import io.opentelemetry.proto.metrics.v1.ExponentialHistogramDataPoint; +import io.opentelemetry.proto.metrics.v1.Metric; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.library.util.prometheus.metrics.Histogram; +import org.apache.skywalking.oap.server.receiver.otel.OtelMetricReceiverConfig; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class OpenTelemetryMetricRequestProcessorTest { + + private OtelMetricReceiverConfig config; + + private ModuleManager manager; + + private OpenTelemetryMetricRequestProcessor metricRequestProcessor; + + private Map<String, String> nodeLabels; + + @BeforeEach + public void setUp() { + manager = new ModuleManager(); + config = new OtelMetricReceiverConfig(); + metricRequestProcessor = new OpenTelemetryMetricRequestProcessor(manager, config); + nodeLabels = new HashMap<>(); + } + + @Test + public void testAdaptExponentialHistogram() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + Class<OpenTelemetryMetricRequestProcessor> clazz = OpenTelemetryMetricRequestProcessor.class; + Method adaptMetricsMethod = clazz.getDeclaredMethod("adaptMetrics", Map.class, Metric.class); + adaptMetricsMethod.setAccessible(true); + + // number is 4, 7, 9 + ExponentialHistogramDataPoint.Buckets positiveBuckets = ExponentialHistogramDataPoint.Buckets.newBuilder() Review Comment: The type is a bit long, causing the entire line to be very long, consider using `var` so the lines can be shorten ########## oap-server/server-receiver-plugin/otel-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/otel/otlp/OpenTelemetryMetricRequestProcessor.java: ########## @@ -161,6 +163,54 @@ private static Map<Double, Long> buildBuckets( return result; } + /** + * ExponentialHistogram data points are an alternate representation to the Histogram data point in OpenTelemetry + * metric format(https://opentelemetry.io/docs/reference/specification/metrics/data-model/#exponentialhistogram). + * It uses scale, offset and bucket index to calculate the bound. Firstly, calculate the base using scale by + * formula: base = 2**(2**(-scale)). Then the upperBound of specific bucket can be calculated by formula: + * base**(offset+index+1). Above calculation way is about positive buckets. For the negative case, we just + * map them by their absolute value into the negative range using the same scale as the positive range. So the + * upperBound should be calculated as -base**(offset+index). + * + * @param positiveOffset corresponding to positive Buckets' offset in ExponentialHistogramDataPoint + * @param positiveBucketCounts corresponding to positive Buckets' bucket_counts in ExponentialHistogramDataPoint + * @param negativeOffset corresponding to negative Buckets' offset in ExponentialHistogramDataPoint + * @param negativeBucketCounts corresponding to negative Buckets' bucket_counts in ExponentialHistogramDataPoint + * @param scale corresponding to scale in ExponentialHistogramDataPoint + * @return The map is a bucket set for histogram, the key is specific bucket's upperBound, the value is item count + * in this bucket lower than or equals to key(upperBound) + */ + private static Map<Double, Long> buildBucketsFromExponentialHistogram( + int positiveOffset, final List<Long> positiveBucketCounts, + int negativeOffset, final List<Long> negativeBucketCounts, int scale) { + + final Map<Double, Long> result = new HashMap<>(); Review Comment: Looks like you didn't take into consideration of the `zero_count` field in OTEL exponential histgram data model? Can you elaborate why? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
