open-telemetry / open-telemetry/opentelemetry-cpp

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

Open
#1,877 3 comments 2 reactions 1 assignee View on GitHub

@lalitb is already working on this.

Since Jan 8, 2023.

bug do-not-stale good first issue help wanted
Dominant language
C++
Stars
1.4k
Forks
632
Avg merge
1d 13h
Merged PRs (30d)
75

Description

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)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.