update OTLP to v0.8.0 #3572

codeboten · 2021-07-06T19:50:03Z

Description:
Updating OTLP to version 0.8.0. Here is a checklist of the progress so far:

bump submodule version
regenerate protos
update uses of DoubleGauge, DoubleSum, DoubleHistogram, DoubleSummary, DoubleDataPoint, DoubleHistogramDataPoint, DoubleSummaryDataPoint, and DoubleExemplar
update ExemplarSlice to use []*otlpmetrics.Exemplar
deprecate IntSum, IntGauge https://github.com/open-telemetry/opentelemetry-proto/blob/f3b0ee0861d304f8f3126686ba9b01c106069cb0/opentelemetry/proto/metrics/v1/metrics.proto#L156
deprecate IntDataPoint https://github.com/open-telemetry/opentelemetry-proto/blob/f3b0ee0861d304f8f3126686ba9b01c106069cb0/opentelemetry/proto/metrics/v1/metrics.proto#L554
deprecate IntExemplar https://github.com/open-telemetry/opentelemetry-proto/blob/f3b0ee0861d304f8f3126686ba9b01c106069cb0/opentelemetry/proto/metrics/v1/metrics.proto#L675
deprecate IntHistogram https://github.com/open-telemetry/opentelemetry-proto/blob/f3b0ee0861d304f8f3126686ba9b01c106069cb0/opentelemetry/proto/metrics/v1/metrics.proto#L170
deprecate filtered labels https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/metrics/v1/metrics.proto#L514

Link to tracking Issue: Part of #3534

…, DoubleDataPoint, DoubleHistogramDataPoint, DoubleSummaryDataPoint, DoubleExemplar

codeboten · 2021-07-06T21:30:18Z

I've started the work to upgrade OTLP here, but I would like some feedback from @open-telemetry/collector-approvers on the way to proceed forward. The linter is currently failing because of various deprecated types still in use:

pdata/metrics.go:193:58: SA1019: otlpmetrics.IntGauge is deprecated: Do not use. (staticcheck)
                ms.orig.Data = &otlpmetrics.Metric_IntGauge{IntGauge: &otlpmetrics.IntGauge{}}
                                                                       ^
pdata/metrics.go:197:54: SA1019: otlpmetrics.IntSum is deprecated: Do not use. (staticcheck)
                ms.orig.Data = &otlpmetrics.Metric_IntSum{IntSum: &otlpmetrics.IntSum{}}
                                                                   ^
pdata/metrics.go:201:66: SA1019: otlpmetrics.IntHistogram is deprecated: Do not use. (staticcheck)
                ms.orig.Data = &otlpmetrics.Metric_IntHistogram{IntHistogram: &otlpmetrics.IntHistogram{}}
                                                                               ^
pdata/metrics.go:261:51: SA1019: otlpmetrics.IntGauge is deprecated: Do not use. (staticcheck)
                data := &otlpmetrics.Metric_IntGauge{IntGauge: &otlpmetrics.IntGauge{}}
                                                                ^
pdata/metrics.go:269:47: SA1019: otlpmetrics.IntSum is deprecated: Do not use. (staticcheck)
                data := &otlpmetrics.Metric_IntSum{IntSum: &otlpmetrics.IntSum{}}
                                                            ^
pdata/metrics.go:277:59: SA1019: otlpmetrics.IntHistogram is deprecated: Do not use. (staticcheck)
                data := &otlpmetrics.Metric_IntHistogram{IntHistogram: &otlpmetrics.IntHistogram{}}
                                                                        ^
pdata/metrics_test.go:47:17: SA1019: otlpmetrics.IntGauge is deprecated: Do not use. (staticcheck)
                                        IntGauge: &otlpmetrics.IntGauge{},
                                                   ^
pdata/metrics_test.go:63:15: SA1019: otlpmetrics.IntSum is deprecated: Do not use. (staticcheck)
                                        IntSum: &otlpmetrics.IntSum{},
                                                 ^
pdata/metrics_test.go:79:21: SA1019: otlpmetrics.IntHistogram is deprecated: Do not use. (staticcheck)
                                        IntHistogram: &otlpmetrics.IntHistogram{},
                                                       ^
pdata/metrics_test.go:441:26: SA1019: otlpmetrics.IntDataPoint is deprecated: Do not use. (staticcheck)
                                                                                DataPoints: []*otlpmetrics.IntDataPoint{
                                                                                               ^
pdata/metrics_test.go:776:20: SA1019: otlpmetrics.IntDataPoint is deprecated: Do not use. (staticcheck)
                                DataPoints: []*otlpmetrics.IntDataPoint{

Would it be better for me to continue working through those messages in this PR or disable the linter for SA1019 to allow this code to merge and prevent the PR from getting too large? Happy to go either ways here.

tigrannajaryan · 2021-07-07T13:49:44Z

Thanks for starting this @codeboten !

We can probably silence the linter temporarily.

However, I think we need a more elaborate plan for this change.

Changing this repo is relatively easy. I think contrib is going to be much bigger volume of changes. We need to time the change such that it is fully doable in the period between the releases (2 weeks). Perhaps once this PR is ready, don't merge it yet, prepare the PR for corresponding contrib changes and only after ensuring contrib is also ready to be merged move forward. Contrib changes may turn out to be a lot of work and we may need to ask codeowners of each component to prepare a PR for their own component (or maybe not if it turns out to be not as laborious as I am imagining).

codeboten · 2021-07-07T19:53:20Z

I tested the contrib repo with the changes as they are in this PR currently and things worked, this is expected as I'm not yet renaming things in pdata. We could potentially move ahead with this PR, which gives us at least the new protos and is not too hard to review.

The remaining items in the checklist could be tackled in following PRs and potentially even done in parallel. Thoughts @tigrannajaryan & @bogdandrutu?

bogdandrutu

I would not "fix" #3534 since this change indeed does the upgrade, but does not do the right transformations to/from deprecated fields/metric types.

bogdandrutu · 2021-07-07T23:36:50Z

.golangci.yml

+    # disabling to suppress deprecation warnings when updating OTLP to v0.8.0
+    # in support of breaking up the work into more manageable PRs


We should create an issue, or document this in #3534 that is left to deal with for later.

tigrannajaryan · 2021-07-08T15:07:42Z

Reading the proto changes in 0.8.0 it seems like the wire format changes are not breaking? From what I see it should remain wire-format compatible with previous version (fields in some messages are added but not removed and some messages are renamed - all of which is backward compatible). Is that correct or I am missing a breaking change?

tigrannajaryan · 2021-07-08T15:10:40Z

model/pdata/generated_metrics.go

@@ -1192,18 +1192,18 @@ func (es DoubleDataPointSlice) RemoveIf(f func(DoubleDataPoint) bool) {
 // Must use NewDoubleDataPoint function to create new instances.
 // Important: zero-initialized instance is not valid for use.
 type DoubleDataPoint struct {


What's the future plan? Will this struct be renamed NumberDataPoint?

I plan on renaming this next and tackling removing IntDataPoint at the same time.

tigrannajaryan · 2021-07-08T15:11:21Z

model/pdata/generated_metrics.go

 func (ms DoubleDataPoint) Value() float64 {
-	return (*ms.orig).Value
+	return (*ms.orig).GetAsDouble()
 }


Do we plan to remove Value and expose 2 different types of values, the int and the double in a future PR?

bogdandrutu

I think we can merge this and do the rest of the work in followup PRs

Alex Boten added 4 commits July 6, 2021 10:19

update proto to v0.8.0

bf9f5db

regenerate protos using 0.8.0

c71d7f6

update uses of DoubleGauge, DoubleSum, DoubleHistogram, DoubleSummary…

c5550c7

…, DoubleDataPoint, DoubleHistogramDataPoint, DoubleSummaryDataPoint, DoubleExemplar

update ExemplarSlice to use []*otlpmetrics.Exemplar

dc874d5

Alex Boten added 3 commits July 7, 2021 08:48

suppress deprecation warnings for metrics

b31c22e

update pdatagen generator code to support AsDouble fields

af4e046

run make genpdata

3d3ebaa

codeboten marked this pull request as ready for review July 7, 2021 19:53

codeboten requested review from a team and dmitryax July 7, 2021 19:53

bogdandrutu reviewed Jul 7, 2021

View reviewed changes

tigrannajaryan reviewed Jul 8, 2021

View reviewed changes

bogdandrutu approved these changes Jul 8, 2021

View reviewed changes

bogdandrutu merged commit 642a8e0 into open-telemetry:main Jul 8, 2021

codeboten deleted the codeboten/proto-v0.8.0 branch July 8, 2021 19:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update OTLP to v0.8.0 #3572

update OTLP to v0.8.0 #3572

codeboten commented Jul 6, 2021 •

edited

Loading

codeboten commented Jul 6, 2021

tigrannajaryan commented Jul 7, 2021

codeboten commented Jul 7, 2021

bogdandrutu left a comment

bogdandrutu Jul 7, 2021

tigrannajaryan commented Jul 8, 2021

tigrannajaryan Jul 8, 2021

bogdandrutu Jul 8, 2021

codeboten Jul 8, 2021

tigrannajaryan Jul 8, 2021

bogdandrutu Jul 8, 2021

bogdandrutu left a comment

		# disabling to suppress deprecation warnings when updating OTLP to v0.8.0
		# in support of breaking up the work into more manageable PRs

update OTLP to v0.8.0 #3572

update OTLP to v0.8.0 #3572

Conversation

codeboten commented Jul 6, 2021 • edited Loading

codeboten commented Jul 6, 2021

tigrannajaryan commented Jul 7, 2021

codeboten commented Jul 7, 2021

bogdandrutu left a comment

Choose a reason for hiding this comment

bogdandrutu Jul 7, 2021

Choose a reason for hiding this comment

tigrannajaryan commented Jul 8, 2021

tigrannajaryan Jul 8, 2021

Choose a reason for hiding this comment

bogdandrutu Jul 8, 2021

Choose a reason for hiding this comment

codeboten Jul 8, 2021

Choose a reason for hiding this comment

tigrannajaryan Jul 8, 2021

Choose a reason for hiding this comment

bogdandrutu Jul 8, 2021

Choose a reason for hiding this comment

bogdandrutu left a comment

Choose a reason for hiding this comment

codeboten commented Jul 6, 2021 •

edited

Loading