Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sdk/metric: NewManualReader and NewPeriodicReader should return structs #4243

Closed
pellared opened this issue Jun 21, 2023 · 0 comments · Fixed by #4244
Closed

sdk/metric: NewManualReader and NewPeriodicReader should return structs #4243

pellared opened this issue Jun 21, 2023 · 0 comments · Fixed by #4244
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package

Comments

@pellared
Copy link
Member

From https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/metrics/sdk.md#compatibility-requirements:

All the metrics components SHOULD allow new methods to be added to existing components without introducing breaking changes.

Adding new methods to struct in Go is not a breaking change. Most of the Go components are structs. However some methods also accept interfaces.

Adding a new method to a interface is a breaking change.

NewManualReader and NewPeriodicReader are components returned as interfaces. It is would not possible to add a new methods (e.g. Pause and Continue) to periodic reader API without creating a new method like NewPeriodicReaderBis that would return a struct (or other interface). I think it would be more flexible if both methods return structs instead of Reader interface.

Originally posted by @pellared in #3671 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants