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

Inconsistency between the use of std::shared_ptr and nostd::shared_ptr #1877

Open
malkia opened this issue Dec 20, 2022 · 3 comments
Open

Inconsistency between the use of std::shared_ptr and nostd::shared_ptr #1877

malkia opened this issue Dec 20, 2022 · 3 comments
Assignees
Labels
bug Something isn't working do-not-stale good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers

Comments

@malkia
Copy link

malkia commented Dec 20, 2022

Describe your environment
Windows, MSVC, Using 1.8.1 tag

Steps to reproduce
Describe exactly how to reproduce the error. Include a code sample if applicable.

What is the expected behavior?
More consitent use of either std::shared_ptr or nostd::shared_ptr (e.g. same way of using it with respect to api and sdk/etc.)

What is the actual behavior?
Inconsistency where one class might return/deal with std:;shared_ptr along with nostd::shared_ptr

Additional context
Not much, and not a blocker at all. I'm working on Windows C++ dll version and re-exposing some of (what I think) are missing factories (PeriodicExportingMetricFactory, MeterProviderFactory) such that I can have single "otel.dll" that is safe to call and work with.

Please compare this:
https://github.com/open-telemetry/opentelemetry-cpp/blob/v1.8.1/sdk/include/opentelemetry/sdk/metrics/exemplar/filter.h#L35

With:
https://github.com/open-telemetry/opentelemetry-cpp/blob/v1.8.1/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h#L53

(and other examples)

Also I'm compiling/forcing HAVE_CPP_STDLIB so for me it doesn't matter much, but in general might be good to have consitent way of which one should be used or not (TBH, I can't fully comprehend all edge cases here, so this might've been a valid choice after all)

@malkia malkia added the bug Something isn't working label Dec 20, 2022
@lalitb
Copy link
Member

lalitb commented Dec 21, 2022

It's supposed to be nostd:: components at API interface, and std:: components at SDK interface. Thanks for raising the issue.

@lalitb lalitb self-assigned this Jan 8, 2023
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

This issue was marked as stale due to lack of activity.

@github-actions github-actions bot added the Stale label Mar 9, 2023
@lalitb lalitb added do-not-stale and removed Stale labels Mar 9, 2023
@lalitb lalitb added good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers labels May 17, 2023
@RichardChukwu
Copy link

RichardChukwu commented Oct 22, 2024

Is this still being worked on after the previous closed PR @bogdandrutu @malkia or is it something I take up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working do-not-stale good first issue Good for newcomers help wanted Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants