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

[Metrics API/SDK] Change Meter API/SDK to return nostd::unique_ptr for Sync Instruments #1700

Closed
lalitb opened this issue Oct 19, 2022 · 2 comments · Fixed by #1707
Closed
Assignees
Labels
bug Something isn't working

Comments

@lalitb
Copy link
Member

lalitb commented Oct 19, 2022

The Sync Instruments created through Meter API are not used internally within SDK. So it would be good to return nostd::unique_ptr for them. Changes are required in API and SDK for

  • Counter ( UINT64 and Double)
  • Histogram ( UINT64 and Double)
  • UpDownCounter ( INT64 and Double)

The Async Instruments can continue to return nostd::shared_ptr.
As part of this or separate PR, we can also maintain the shared_ptr of Async Instrument (instead of current raw pointer) in Observable Registry

opentelemetry::metrics::ObservableInstrument *instrument;

@lalitb lalitb added the bug Something isn't working label Oct 19, 2022
@lalitb lalitb changed the title Change Meter API/SDK to return nostd::unique_ptr for Sync Instruments [Metrics API/SDK] Change Meter API/SDK to return nostd::unique_ptr for Sync Instruments Oct 19, 2022
@lalitb lalitb added this to the Metrics v1.0.0 (GA) milestone Oct 19, 2022
@esigo
Copy link
Member

esigo commented Oct 22, 2022

It's not possible to maintain a shared_ptr of Async Instrument

opentelemetry::metrics::ObservableInstrument *instrument;

as we need to enable shared from this, which doesn't work with nostd::shared_ptr.

@lalitb
Copy link
Member Author

lalitb commented Oct 22, 2022

as we need to enable shared from this, which doesn't work with nostd::shared_ptr.

Thanks for trying. We can handle this as separate PR. As this is internal SDK change, should be fine to handle post GA. Please raise a issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants