Initial readme and contributing docs by reyang · Pull Request #3 · open-telemetry/opentelemetry-cpp

reyang · 2019-10-02T05:34:01Z

as title

tigrannajaryan

LGTM (after addressing comments from @z1c0).

meastp · 2019-10-03T09:35:38Z

+# OpenTelemetry C/C++
+[![Gitter chat][gitter-image]][gitter-url]
+
+[gitter-image]: https://badges.gitter.im/open-telemetry/opentelemetry-cpp.svg
+[gitter-url]: https://gitter.im/open-telemetry/opentelemetry-cpp?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge
+
+The C/C++ [OpenTelemetry](https://opentelemetry.io/) client.
+
+## Installation
+
+TBD
+
+## Quick Start
+
+TBD
+
+## Contributing
+
+See [CONTRIBUTING.md](CONTRIBUTING.md)
+
+## Release Schedule
+
+OpenTelemetry C/C++ is under active development.


Is this supposed to be a combined library for both C and C++ users? Are C users/libraries willing to depend on a C++ runtime for their libraries/products? If this is not the case (which I hope), please do not mix C into the naming, and use just "OpenTelemetry C++"

This is a "to-be-discussed" topic for our 1st community meeting.

I could imagine the potential-future C API belongs in its own repository.

Yes, we've discussed this during the community meeting.
For now we will use this repo for the C++ SDK, and build a C API (no exception, no STL in the function signature) on top of the C++ SDK.
Later we might want to split the C API to its own repo.

jmacd · 2019-10-14T22:22:57Z

+# OpenTelemetry C/C++
+[![Gitter chat][gitter-image]][gitter-url]
+
+[gitter-image]: https://badges.gitter.im/open-telemetry/opentelemetry-cpp.svg
+[gitter-url]: https://gitter.im/open-telemetry/opentelemetry-cpp?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge
+
+The C/C++ [OpenTelemetry](https://opentelemetry.io/) client.
+
+## Installation
+
+TBD
+
+## Quick Start
+
+TBD
+
+## Contributing
+
+See [CONTRIBUTING.md](CONTRIBUTING.md)
+
+## Release Schedule
+
+OpenTelemetry C/C++ is under active development.


I could imagine the potential-future C API belongs in its own repository.

Origin/propagators

Missed include (open-telemetry#2143)

Fixes open-telemetry#3849. etw::TracerProvider::GetTracer() was newing up a fresh Tracer per call and returning the only shared_ptr to it. The catch is that etw::Span stores its parent as a raw reference (Tracer &owner_), so the moment a caller drops or reassigns that shared_ptr, every Span that came from that Tracer is left with a dangling owner_. Span::End() then crashes inside owner_.EndSpan(). The snippet from the issue makes it obvious: auto provider = std::make_unique<etw::TracerProvider>(); auto tracer = provider->GetTracer("Foo"); auto spanFoo = tracer->StartSpan("Span-Foo"); tracer = provider->GetTracer("Bar"); // Tracer "Foo" goes away auto spanBar = tracer->StartSpan("Span-Bar"); spanFoo->End(); // crash spanBar->End(); Under MSVC Debug + cdb this lands as an AV in Span::End dereferencing 0xFEEEFEEE -- the debug heap's freed-memory fill -- which is about as unambiguous as UAF gets. The fix is to let TracerProvider keep the Tracers it hands out, keyed by name and guarded by a mutex, and have GetTracer() return the cached entry on repeat calls. That makes the Tracer outlive any Span derived from it, which is the lifetime contract Span's raw owner_ already assumes. It is also the same pattern sdk::trace::TracerProvider has always used, which is why the issue reporter noted the SDK provider does not crash under the same usage. There was an earlier attempt in PR open-telemetry#4070 that switched Span to take a shared_ptr<Tracer> via shared_from_this(). That was turned down because StartSpan() is the ETW exporter's hot path and nobody wants an extra atomic refcount bump on every span just to keep the parent alive. Doing the lifetime work once in GetTracer() keeps owner_ as a plain reference and costs nothing per span. While putting this together I tripped over a second bug that the cache exposes -- worth describing here because the fix lives in the same file. etw_tracer.h and etw_provider.h between them have three independent function-local statics that get tangled at process shutdown: 1. etw::TracerProvider itself (now owning the tracer cache). 2. Tracer::etwProvider() returns a `static ETWProvider instance`. 3. ETWProvider::providers() returns a `static std::map<...> providers` that lives inside that singleton's method, not inside the singleton object, and is lazily created on the first open()/is_registered() call. Before this change, all three were initialized in that order on the very first GetTracer() call: the TracerProvider was already constructed, then the Tracer ctor called etwProvider().open(...), which constructed open-telemetry#2 and open-telemetry#3. Reverse-order destruction therefore took out open-telemetry#3 first, while the TracerProvider (and its cached Tracers, which hold references into open-telemetry#3) was still alive. ~Tracer then ran etwProvider().close(provHandle) against a dangling reference and we got an AV at exit. With the old "fresh Tracer per call, caller drops it immediately" pattern this was hidden because the Tracer (and its provHandle reference) was almost always gone long before shutdown. The cache extends Tracer lifetime to match TracerProvider lifetime, which is exactly what tripped this over locally and on the Windows CI legs. The fix is small: both TracerProvider constructors now do (void)Tracer::etwProvider().is_registered(std::string{}); which forces both ETWProvider singletons to be constructed before the TracerProvider's body finishes. Reverse destruction then guarantees they outlive the cached Tracers. is_registered() is a public, side- effect-free read on ETWProvider that internally touches the providers map, which is exactly the static-init we need. It requires Tracer to befriend TracerProvider so the constructor can call the private etwProvider() accessor. A few things worth calling out: - No API or ABI change. Only private members are added to TracerProvider, and <map>/<memory>/<mutex> are already included by etw_tracer.h. The friend declaration is purely internal. - The cache deliberately does not evict. Span holds its parent by raw reference, so dropping a Tracer that still has live Spans would put the original bug right back. In practice the ETW exporter is built around "one Tracer per provider name, reused for the lifetime of the process", and Windows itself caps registered ETW providers per process well below anything you would worry about for memory. - Keying on name only is consistent with the existing etw::Tracer constructor, which already declares args and schema_url UNREFERENCED_PARAMETER. - etw::LoggerProvider was checked for the same shape and does not currently cache Loggers, so the destruction-order fix is not needed there today. If a similar cache is ever added to LoggerProvider it will want the same one-line force-init in its constructor. - Updated ETWTracer.GlobalSingletonTracer: it previously asserted that two GetTracer() calls with the same provider name produce different trace ids, which only held because of the bug being fixed here. GetTracer(name) is now idempotent per name, so the assertion is flipped to EXPECT_EQ and the stale sample-event comment updated to match. All other ETW tracer tests continue to pass. Repro before the change: AV in Span::End with the freed-fill operand, plus exit-time AV in ~Tracer on ctest runs that exercise a single test in isolation. Same repros after: both run cleanly to completion. Verified locally with the full ctest suite on a Windows abiv2 maintainer-mode build (1104/1104 passing) and on an abiv1 Debug build (43/43 ETW tests passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

initial readme and contributing docs

3d1426b

z1c0 reviewed Oct 2, 2019

View reviewed changes

Comment thread CONTRIBUTING.md Outdated

Comment thread CONTRIBUTING.md Outdated

Comment thread CONTRIBUTING.md Outdated

tigrannajaryan approved these changes Oct 2, 2019

View reviewed changes

address review comments

6ade82e

meastp reviewed Oct 3, 2019

View reviewed changes

jmacd approved these changes Oct 14, 2019

View reviewed changes

g-easy reviewed Oct 14, 2019

View reviewed changes

Comment thread CONTRIBUTING.md Outdated

Comment thread CONTRIBUTING.md Outdated

fix typo and add CLA link

8e354f4

reyang merged commit d79a27c into open-telemetry:master Oct 15, 2019

0x4b pushed a commit to 0x4b/opentelemetry-cpp that referenced this pull request Jul 31, 2020

Merge pull request open-telemetry#3 from Tianlin-Zhao/origin/propagators

a99f620

Origin/propagators

jsuereth mentioned this pull request Dec 1, 2020

Add simple log processor and log exporter interface #403

Merged

ZDzhao mentioned this pull request Feb 18, 2022

crash in ext::http::client::curl::HttpOperation::Send #1220

Closed

VivekSubr mentioned this pull request Jan 6, 2023

Scope should be default constructable #1889

Closed

GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Jun 21, 2024

Merge pull request open-telemetry#3 from open-telemetry/main

579c4d4

Missed include (open-telemetry#2143)

zhugul mentioned this pull request Mar 27, 2025

"malloc(): unaligned fastbin chunk detected" crash in OpenTelemetry C++ SDK 1.16 #3326

Closed

bharat2002 mentioned this pull request May 8, 2025

std::thread crashing when ending Span and or whereever new std::thread is writtern in library(.so) #3394

Open

Sowmya-Iyer mentioned this pull request Jul 21, 2025

AddCallback() for metrics (gauge) causes a pure virtual method called error #3555

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial readme and contributing docs#3

Initial readme and contributing docs#3
reyang merged 3 commits into
open-telemetry:masterfrom
reyang:docs

reyang commented Oct 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tigrannajaryan left a comment

Uh oh!

meastp Oct 3, 2019

Uh oh!

reyang Oct 3, 2019

Uh oh!

jmacd Oct 14, 2019

Uh oh!

reyang Oct 14, 2019

Uh oh!

jmacd Oct 14, 2019

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

reyang commented Oct 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tigrannajaryan left a comment

Choose a reason for hiding this comment

Uh oh!

meastp Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

reyang Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

jmacd Oct 14, 2019

Choose a reason for hiding this comment

Uh oh!

reyang Oct 14, 2019

Choose a reason for hiding this comment

Uh oh!

jmacd Oct 14, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants