exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. #777

franciscovalentecastro · 2023-12-11T22:05:52Z

This PR adds the following functionality to the exporter/metric :

Added support to export ExponentialHistogram.
Added support to export TypedValue Distribution (either Histogram and ExponentialHistogram) with exemplars.
Adds exponential_histogram example with a dashboard.json to visualize it.

Details :

Add Exemplar to metricdata package open-telemetry/opentelemetry-go#3849 added the Exemplar data type and the Exemplars field to a HistogramDataPoint.
Add the experimental exemplar feature open-telemetry/opentelemetry-go#4871 added experimental Exemplar sampling support.

codecov · 2023-12-11T22:08:58Z

Codecov Report

Attention: Patch coverage is 4.21053% with 91 lines in your changes are missing coverage. Please review.

Project coverage is 59.79%. Comparing base (87124ac) to head (1590aff).

Files	Patch %	Lines
exporter/metric/metric.go	4.21%	89 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #777      +/-   ##
==========================================
- Coverage   60.73%   59.79%   -0.95%     
==========================================
  Files          56       56              
  Lines        5649     5743      +94     
==========================================
+ Hits         3431     3434       +3     
- Misses       2070     2159      +89     
- Partials      148      150       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dashpole · 2024-02-20T15:00:25Z

The feature has been implemented in the OTel SDK: https://github.com/open-telemetry/opentelemetry-go/blob/main/sdk/metric/internal/x/README.md#exemplars

Let me know if you are interested in contributing this.

franciscovalentecastro · 2024-02-23T22:02:25Z

exporter/metric/metric.go

+func toDistributionExemplar[N int64 | float64](Exemplars []metricdata.Exemplar[N]) []*distribution.Distribution_Exemplar {
+	var exemplars []*distribution.Distribution_Exemplar
+	for _, e := range Exemplars {
+		exemplars = append(exemplars, &distribution.Distribution_Exemplar{Value: float64(e.Value), Timestamp: timestamppb.New(e.Time)})


Verified Distribution_Exemplar only supports float64 values : https://pkg.go.dev/google.golang.org/genproto/googleapis/api/distribution#Distribution_Exemplar

franciscovalentecastro · 2024-02-23T22:04:53Z

exporter/metric/metric.go

+	positiveBucketCounts := 0
+	growthFactor := math.Exp2(math.Exp2(-float64(hist.Scale)))
+	for i, v := range hist.PositiveBucket.Counts {
+		counts[i] = int64(v)


Opentelemetry Exponential Buckets support Negative Indices too, but AFAIU the TypedValue Exponential Buckets only support Positive Indices.

Thats correct. All negative observations and zero observations should be in the underflow bucket. See

opentelemetry-operations-go/exporter/collector/metrics.go

Lines 989 to 993 in 87124ac

underflow := point.ZeroCount()

negativeBuckets := point.Negative().BucketCounts()

for i := 0; i < negativeBuckets.Len(); i++ {

underflow += negativeBuckets.At(i)

}

Fixed. I used as reference the implementation of collector/metrics.go : exponentialHistogramPoint to reimplement all the mentioned details of expHistToDistribution in this review, but using the correct types and methods.

franciscovalentecastro · 2024-02-26T15:31:36Z

example/metric/exponential_histogram/go.mod

+	google.golang.org/protobuf v1.31.0 // indirect
+)
+
+replace github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric => ../../../exporter/metric


Added this to pick current changes (before merging) to metric.go.

example/metric/exponential_histogram/example.go

exporter/metric/metric.go

dashpole · 2024-02-26T17:36:26Z

exporter/metric/metric.go

+	positiveBucketCounts := 0
+	growthFactor := math.Exp2(math.Exp2(-float64(hist.Scale)))
+	for i, v := range hist.PositiveBucket.Counts {
+		counts[i] = int64(v)


Thats correct. All negative observations and zero observations should be in the underflow bucket. See

opentelemetry-operations-go/exporter/collector/metrics.go

Lines 989 to 993 in 87124ac

underflow := point.ZeroCount()

negativeBuckets := point.Negative().BucketCounts()

for i := 0; i < negativeBuckets.Len(); i++ {

underflow += negativeBuckets.At(i)

}

exporter/metric/metric.go

…t` as reference for `expHistToDistribution`.

franciscovalentecastro · 2024-03-01T00:15:02Z

@dashpole Update on last changes to PR :

Added dashboard.json to visualize the exponential_histogram example. This may conflict/be confused with the other dashboard.json in the above folder. Is this the correct place and/or naming ? (I imported manually to GCM and it works correctly).
Fixed the open comments.
Added a TODO for the missing []attachments to the exemplar exporting (AFAIU, this will add the span_id, etc, though i'm not very knowledgable about how to do this correctly).

example/metric/exponential_histogram/example.go

exporter/metric/metric.go

Co-authored-by: David Ashpole <[email protected]> Signed-off-by: Francisco Valente Castro <[email protected]>

dashpole · 2024-03-04T16:57:21Z

Follow-up tasks required:

README for the example
Unit testing
Exemplar attachments
SumOfSquaredDeviations

@franciscovalentecastro can you add the README? You are welcome to work on the other tasks as well if you have time. Otherwise, I'll make sure they get done. Just let me know which you'd like to tackle

franciscovalentecastro changed the title ~~[Draft] exporter/metric: Add support to export Distribution with Exemplars.~~ [Draft] exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. Dec 12, 2023

jsuereth mentioned this pull request Dec 14, 2023

Exemplars should be sorted in Collector exporter #778

Closed

franciscovalentecastro mentioned this pull request Dec 14, 2023

There is an upper bound of number of Exponential Buckets that can be easily exceeded #779

Closed

franciscovalentecastro force-pushed the main branch from f1d62de to df2fd53 Compare February 23, 2024 20:41

franciscovalentecastro added 12 commits February 23, 2024 20:43

exporter/metric: Add support to export Distribution with Exemplars.

250de6b

example/metric: Add expoential histogram example.

fef5b26

Update example.go.

0cda63a

exporter/metric: Transform ExponentialHistogram to Distribution.

168b5a8

example: Update date.

29786bb

example: Use views for histogram.

0a7ce9f

Fixed ExponentialHist to Distribution translation.

bb3c563

Add more distribution metric views to example.go.

3a14acb

Fix typo.

e56a3b8

Remove local dev artifacts.

20d8ec7

Tidy go.mod.

13c65c3

Update dependencies.

aa41056

franciscovalentecastro force-pushed the main branch from df2fd53 to aa41056 Compare February 23, 2024 20:45

franciscovalentecastro added 4 commits February 23, 2024 21:08

Further update dependencies.

a4b0389

Go mod tidy.

76347ce

Removed debuggin and unused code.

3ca23f0

Fix example parameters and logs.

f396d98

franciscovalentecastro changed the title ~~[Draft] exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms.~~ exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. Feb 23, 2024

franciscovalentecastro marked this pull request as ready for review February 23, 2024 21:49

franciscovalentecastro requested a review from a team as a code owner February 23, 2024 21:49

franciscovalentecastro requested review from dashpole and jsuereth February 23, 2024 22:00

Use positiveBucketCounts for mean calculation.

83a6e6f

franciscovalentecastro commented Feb 23, 2024

View reviewed changes

Fix linter errors.

079b8bd

franciscovalentecastro commented Feb 26, 2024

View reviewed changes

dashpole reviewed Feb 26, 2024

View reviewed changes

franciscovalentecastro added 8 commits February 29, 2024 23:28

Use s units instead of ms.

760fafa

Use collector/metric.go implementation of `exponentialHistogramPoin…

ac6ebe5

…t` as reference for `expHistToDistribution`.

Write to workload.googleapis.com instead of custom.googleapis.com

047c379

Add dashboard.json to visualize example.go

f7dd87d

Fix expHistToDistribution

3579361

Use correct type ExponentialHistogramDataPoint.

c4e9fd4

Fix usage of math.IsNan

3274232

Add TODO for missing []attachments to exemplars exporting.

8eb2406

franciscovalentecastro requested a review from dashpole March 1, 2024 00:24

dashpole reviewed Mar 1, 2024

View reviewed changes

example/metric/exponential_histogram/example.go Outdated Show resolved Hide resolved

example/metric/exponential_histogram/example.go Outdated Show resolved Hide resolved

exporter/metric/metric.go Show resolved Hide resolved

franciscovalentecastro and others added 4 commits March 4, 2024 10:21

Update example/metric/exponential_histogram/example.go

9a732fd

Co-authored-by: David Ashpole <[email protected]> Signed-off-by: Francisco Valente Castro <[email protected]>

Add support for int64 ExponentialHistogram.

604f5b0

Pass all views once to WithView.

0bd8a8d

Remove workloadMetricsPrefixFormatter.

1590aff

dashpole approved these changes Mar 4, 2024

View reviewed changes

dashpole merged commit c7b17b6 into GoogleCloudPlatform:main Mar 4, 2024
30 of 32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. #777

exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. #777

franciscovalentecastro commented Dec 11, 2023 •

edited

Loading

codecov bot commented Dec 11, 2023 •

edited

Loading

dashpole commented Feb 20, 2024

franciscovalentecastro Feb 23, 2024

franciscovalentecastro Feb 23, 2024

dashpole Feb 26, 2024

franciscovalentecastro Mar 1, 2024

franciscovalentecastro Feb 26, 2024

dashpole Feb 26, 2024

franciscovalentecastro commented Mar 1, 2024 •

edited

Loading

dashpole commented Mar 4, 2024

	underflow := point.ZeroCount()
	negativeBuckets := point.Negative().BucketCounts()
	for i := 0; i < negativeBuckets.Len(); i++ {
	underflow += negativeBuckets.At(i)
	}

exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. #777

exporter/metric: Add support to export Distribution with Exemplars and Exponential Histograms. #777

Conversation

franciscovalentecastro commented Dec 11, 2023 • edited Loading

codecov bot commented Dec 11, 2023 • edited Loading

Codecov Report

dashpole commented Feb 20, 2024

franciscovalentecastro Feb 23, 2024

Choose a reason for hiding this comment

franciscovalentecastro Feb 23, 2024

Choose a reason for hiding this comment

dashpole Feb 26, 2024

Choose a reason for hiding this comment

franciscovalentecastro Mar 1, 2024

Choose a reason for hiding this comment

franciscovalentecastro Feb 26, 2024

Choose a reason for hiding this comment

dashpole Feb 26, 2024

Choose a reason for hiding this comment

franciscovalentecastro commented Mar 1, 2024 • edited Loading

dashpole commented Mar 4, 2024

franciscovalentecastro commented Dec 11, 2023 •

edited

Loading

codecov bot commented Dec 11, 2023 •

edited

Loading

franciscovalentecastro commented Mar 1, 2024 •

edited

Loading