[Bug] OpenTelemetryMetricHandler Histogram buildBucket

[jinho-son]

Search before asking

• I had searched in the issues and found no similar issues.

Apache SkyWalking Component

OAP server (apache/skywalking)

What happened

When using OpenTelemetry Histogram metric it not working correctly

What you expected to happen

The metrics generate from opentelemetry client will be same result.

How to reproduce

Use default application.yaml

the opentelemetry client (java) create DoubleHistogram, and record 1 ~ 1000.
total count is 1000, and sum is 500500

Metric #0
Descriptor:
     -> Name: test
     -> Description: 
     -> Unit: 
     -> DataType: Histogram
     -> AggregationTemporality: AGGREGATION_TEMPORALITY_CUMULATIVE
HistogramDataPoints #0
StartTimestamp: 2022-09-27 04:41:01.914 +0000 UTC
Timestamp: 2022-09-27 04:42:01.921 +0000 UTC
Count: 1000
Sum: 500500.000000
ExplicitBounds #0: 5.000000
ExplicitBounds #1: 10.000000
ExplicitBounds #2: 25.000000
ExplicitBounds #3: 50.000000
ExplicitBounds #4: 75.000000
ExplicitBounds #5: 100.000000
ExplicitBounds #6: 250.000000
ExplicitBounds #7: 500.000000
ExplicitBounds #8: 750.000000
ExplicitBounds #9: 1000.000000
ExplicitBounds #10: 2500.000000
ExplicitBounds #11: 5000.000000
ExplicitBounds #12: 7500.000000
ExplicitBounds #13: 10000.000000
Buckets #0, Count: 5
Buckets #1, Count: 5
Buckets #2, Count: 15
Buckets #3, Count: 25
Buckets #4, Count: 25
Buckets #5, Count: 25
Buckets #6, Count: 150
Buckets #7, Count: 250
Buckets #8, Count: 250
Buckets #9, Count: 250
Buckets #10, Count: 0
Buckets #11, Count: 0
Buckets #12, Count: 0
Buckets #13, Count: 0
Buckets #14, Count: 0

Anything else

I think OpenTelemetry Metric Handler buildBucket function is bug or change it.

The process of OpenTelemetry Metric Handler is

1. get Metrics from opentelemetry metric export
2. change to Prometheus metric format
3. do analyze

In step#2 send to PrometheusConverter

Prometheus Histogram Metric using 'le', but opentelemetry histogram has no 'le' using explicit_bounds, so In PrometheusConverter create 'le' through explicit bounds

In opentelemetry-proto file the histogram is boundary,

opentelemetry like
0 ~ 100 : 20
100 ~ 200 : 30
200 ~ 300 : 10
300 ~ +Inf : 0

but prometheus like
le 100 : 20
le 200 : 50
le 300 : 60

The problem occurs when SampleFamily histogram method

I think it will be change to opentelemetry histogram build bucket method to cumulative.

Am I misunderstood opentelemetry-proto file?

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[wu-sheng]

@kezhenxu94 Could you take a look? I think we don't use Hisgram from OTEL yet.

[wu-sheng]

From the description, it looks like OTEL's histogram is more closing to SkyWalking's meter defintion.

[kezhenxu94]

@kezhenxu94 Could you take a look? I think we don't use Hisgram from OTEL yet.

How so? We did use Histogram from OTEL already

skywalking/oap-server/server-receiver-plugin/otel-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/otel/otlp/OpenTelemetryMetricHandler.java

Lines 207 to 218 in 41e8c30

	if (metric.hasHistogram()) {
	return metric.getHistogram().getDataPointsList().stream()
	.map(point -> new Histogram(
	metric.getName(),
	mergeLabels(nodeLabels,
	buildLabels(point.getAttributesList())),
	point.getCount(),
	point.getSum(),
	buildBuckets(point.getBucketCountsList(),
	point.getExplicitBoundsList()),
	point.getTimeUnixNano() / 1000000));
	}

Also, I think the Histogram in both OpenCensus and OpenTelemetry are cumulative, no? @jinho-son

[kezhenxu94]

After some local experiments I'm totally confused by this output, the descriptor says it's cumulative AggregationTemporality: AGGREGATION_TEMPORALITY_CUMULATIVE, but from the bucket counts it doesn't look so. Any input @hanahmily ?

Metric #0
Descriptor:
     -> Name: test
     -> Description: 
     -> Unit: 
     -> DataType: Histogram
     -> AggregationTemporality: AGGREGATION_TEMPORALITY_CUMULATIVE
HistogramDataPoints #0
StartTimestamp: 2022-09-27 04:41:01.914 +0000 UTC
Timestamp: 2022-09-27 04:42:01.921 +0000 UTC
Count: 1000
Sum: 500500.000000
ExplicitBounds #0: 5.000000
ExplicitBounds #1: 10.000000
ExplicitBounds #2: 25.000000
ExplicitBounds #3: 50.000000
ExplicitBounds #4: 75.000000
ExplicitBounds #5: 100.000000
ExplicitBounds #6: 250.000000
ExplicitBounds #7: 500.000000
ExplicitBounds #8: 750.000000
ExplicitBounds #9: 1000.000000
ExplicitBounds #10: 2500.000000
ExplicitBounds #11: 5000.000000
ExplicitBounds #12: 7500.000000
ExplicitBounds #13: 10000.000000
Buckets #0, Count: 5
Buckets #1, Count: 5
Buckets #2, Count: 15
Buckets #3, Count: 25
Buckets #4, Count: 25
Buckets #5, Count: 25
Buckets #6, Count: 150
Buckets #7, Count: 250
Buckets #8, Count: 250
Buckets #9, Count: 250
Buckets #10, Count: 0
Buckets #11, Count: 0
Buckets #12, Count: 0
Buckets #13, Count: 0
Buckets #14, Count: 0

[jinho-son]

I think in OpenTelemetry Histogram, it's not cumulative.(The meaning of 'cumulative' is cumulative increase bucket counts like prometheus)

There are several reasons.

1. bucket_counts
   In the opentelemetry proto file. proto-bucket_counts
   If opentelemetry bucket like prometheus style, only last bucket_count equal the value in the count field.
   But the opentelemetry-proto comments 'The sum of bucket_counts must equal the value in the count field'.

2. explicit_bounds
   In the opentelemetry proto file. proto-explicit_bounds
   If the opentelemetry bounds like prometheus style, it will be says lower or equal than specific bound.
   But the opentelemetry-proto comments like specific ranges '(explicit_bounds[i-1], explicit_bounds[i]] for 0 < i < size(explicit_bounds)'

3. opentelemetry docs
   The OpenTelemetry docs of Histogram
   That is, buckets express the number of values that are greater than their lower bound and less than or equal to their upper bound

Am I misunderstand?

Also
AggregationTemporality: AGGREGATION_TEMPORALITY_DELTA
AggregationTemporality: AGGREGATION_TEMPORALITY_CUMULATIVE

It means time based delta or cumulative

[jinho-son]

The meaning of time based delta or cumulative.

Such as like
time: t_0, t_1, t_2
t_0 ~ t_1: 3 request
t_1 ~ t_2 : 2 request

AggregationTemporality: AGGREGATION_TEMPORALITY_DELTA
t_1: 3 request
t_2: 2 request

AggregationTemporality: AGGREGATION_TEMPORALITY_CUMULATIVE
t_1: 3 request
t_2: 5 request

[wu-sheng]

In the SkyWalking OAP processing mode, we expect delta mode only. Because the UI would visualize the data in that mode.
Also, from @kezhenxu94 's post, I think even in AGGREGATION_TEMPORALITY_CUMULATIVE mode, the data layouts in delta mode.

[jinho-son]

I think I created some confusion by using the term 'cumulative'.

The difference 'AGGREGATION_TEMPORALITY_DELTA' and 'AGGREGATION_TEMPORALITY_CUMULATIVE', is time based cumulative. (below is the result, only different AGGREGATION_TEMPORALITY_DELTA and AGGREGATION_TEMPORALITY_CUMULATIVE')

Using AGGREGATION_TEMPORALITY_DELTA

//time: t0
name: "test"
description: "description"
histogram {
  data_points {
    start_time_unix_nano: 1664343839851000000
    time_unix_nano: 1664343899856000000
    count: 1000
    sum: 500500.0
    bucket_counts: 10
    bucket_counts: 90
    bucket_counts: 400
    bucket_counts: 500
    bucket_counts: 0
    bucket_counts: 0
    bucket_counts: 0
    explicit_bounds: 10.0
    explicit_bounds: 100.0
    explicit_bounds: 500.0
    explicit_bounds: 1000.0
    explicit_bounds: 1500.0
    explicit_bounds: 2000.0
  }
  aggregation_temporality: AGGREGATION_TEMPORALITY_DELTA
}

//time: t1
name: "test"
description: "description"
histogram {
  data_points {
    start_time_unix_nano: 1664343899856000000
    time_unix_nano: 1664343959853000000
    count: 1000
    sum: 1500500.0
    bucket_counts: 0
    bucket_counts: 0
    bucket_counts: 0
    bucket_counts: 0
    bucket_counts: 500
    bucket_counts: 500
    bucket_counts: 0
    explicit_bounds: 10.0
    explicit_bounds: 100.0
    explicit_bounds: 500.0
    explicit_bounds: 1000.0
    explicit_bounds: 1500.0
    explicit_bounds: 2000.0
  }
  aggregation_temporality: AGGREGATION_TEMPORALITY_DELTA
}

Using AGGREGATION_TEMPORALITY_CUMULATIVE

// time:t0
name: "test"
description: "description"
histogram {
  data_points {
    start_time_unix_nano: 1664343663351000000
    time_unix_nano: 1664343723357000000
    count: 1000
    sum: 500500.0
    bucket_counts: 10
    bucket_counts: 90
    bucket_counts: 400
    bucket_counts: 500
    bucket_counts: 0
    bucket_counts: 0
    bucket_counts: 0
    explicit_bounds: 10.0
    explicit_bounds: 100.0
    explicit_bounds: 500.0
    explicit_bounds: 1000.0
    explicit_bounds: 1500.0
    explicit_bounds: 2000.0
  }
  aggregation_temporality: AGGREGATION_TEMPORALITY_CUMULATIVE
}

// time: t1
name: "test"
description: "description"
histogram {
  data_points {
    start_time_unix_nano: 1664343663351000000
    time_unix_nano: 1664343783354000000
    count: 2000
    sum: 2001000.0
    bucket_counts: 10
    bucket_counts: 90
    bucket_counts: 400
    bucket_counts: 500
    bucket_counts: 500
    bucket_counts: 500
    bucket_counts: 0
    explicit_bounds: 10.0
    explicit_bounds: 100.0
    explicit_bounds: 500.0
    explicit_bounds: 1000.0
    explicit_bounds: 1500.0
    explicit_bounds: 2000.0
  }
  aggregation_temporality: AGGREGATION_TEMPORALITY_CUMULATIVE
}

Why I used 'cumulative', because the process of OpenTelemetryMetricHandler has send OpenTelemetry Metric to PrometheusConverter. and skip AGGREGATION_TEMPORALITY_CUMULATIVE (reason)

So before send to PrometheusConverter, OpenTelemetry Histogram should change to Prometheus histogram format, because it little be different. (If opencensus metrics different opentelemetry)

Like this

        final Map<Double, Long> result = new HashMap<>();
        long cumulate = 0;
        for (int i = 0; i < explicitBounds.size(); i++) {
            cumulate += bucketCounts.get(i);
            result.put(explicitBounds.get(i), cumulate);
        }
        result.put(Double.POSITIVE_INFINITY, bucketCounts.get(explicitBounds.size()) + cumulate);

Other option,

1. Histogram Builder
2. PrometheusConverter edit convertToSample method ( if opencensus has same issue)

[kezhenxu94]

Hi, @jinho-son , currently there is a implicit prerequisite for SkyWalking OTEL receiver, it assumes the metrics are sent from Prometheus and scraped by OTEL collector so the metrics are all cumulative, however in this issue (and my local test), we are using OpenTelemetry clients (you are using Java client and I was using Python client), this might be the gap between what we expect and how the SkyWalking deals with all existing metrics without problem, because all metrics in the existing SkyWalking examples/demos are scraped from Prometheus.

[wu-sheng]

Otel collector doesn't covert the metrics of Prometheus(which is SkyWalking 1st class usage for OTEL), which made us to provide metrics analysis in this way.

The only thing I think we could do is that we add a method in SampleFamily or configuration in MAL to provide a custom way to set up delta or cumultive. Of course, all current default ways should be kept.
We should add some docs on OTEL receiver mentioning this unexpected behavior of OTEL, and explain why we have this new(but strange) configuration, which looks like violating OTEL protocol, but actually right.

@kezhenxu94 @hanahmily WDYT?

[hanahmily]

Adding an overloaded method histogram(string le, timeunit unit, HistogramType type) to SampleFamily is a convenient path to support this. Current histogram could call this with a default type Cumultive.