feat: add context event observer API

[yannham]

Motivation

OTEP: Thread Context: Sharing Thread-Level Information with external readers is on the verge of being merged as an official OTEP. This proposal defines a protocol for an instrumented application to publish thread-level context somewhere that is discoverable by an external, potentially out-of-process reader (typically the eBPF profiler).

In order to properly implement this, any telemetry crate building upon opentelemetry-rust needs to be notified about context switches. This PR adds such a capability/API in a very simple way, currently gated behind an experimental feature.

Changes

Add an API for downstream crates to subscribe to context switch events through the ContextObserver trait. The initial implementation is minimal and has only two events, on_context_enter and on_context_exit. We currently only allow one global observer.

Additionally, for any implementation of the OTEP mentioned above to be remotely performant, we'll also need to store alongside opentelemetry-rust's Context the ThreadContextRecord view of this context defined in the OTEP, which bears similar information but in a different shape. This PR adds a generic free-form field to Context, the observer's context view, which represents an immutable, set-once view of the associated context for the observer.

The view is behind a OnceLock so that:

• there's an obvious Default implementation
• the lifecycle design questions are entirely offloaded to the observer. Whether to initialize the view immediately or at first on_context_enter, etc.

An alternative would be to have an Option<_>, but that would need to be initialized at context creation, which would need an additional context event (e.g. on_context_created or make_view). Another possibility would be to require Default on ObserverContextView, but that would make it not object-safe which is out of the question.

All in all the lock is simple, minimize changes to this repo, doesn't prevent future optimization (having more events for optimized view creations, like when deriving a context from an existing one). To only price is the lock's additional size, (I suppose at most a usize).

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: yannham / name: Yann Hamdaoui (47182eb, 5453016, 62fc736, 8665cb3, fe6986f)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.3%. Comparing base (95c1d4b) to head (0395006).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #3585    +/-   ##
======================================
  Coverage   83.2%   83.3%            
======================================
  Files        130     130            
  Lines      28232   28393   +161     
======================================
+ Hits       23491   23652   +161     
  Misses      4741    4741            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[scottgerring]

This s a clever way of doing this! I was imagining something with lazily-invoked callbacks but this feels nicer.

[scottgerring]

Because we have quite a low MSRV (1.75), I suspect that users can't rely on trait upcasting to do things with their view. Because its methodless, they'll be able to store a view but not recover its concrete type. I think we should be able to add a helper to facilitate this, and probably we should add a test to validate and show the general usage shape too

[scottgerring]

We take a mutable borrow on the context here ....

[scottgerring]

... and invoke the observer here. I think this means that Bad Things will happen if the observer does things like calling Context::current(), tries to attach another context, or emits telemetry probably even - as this'd lead to trying to take another mutable borrow on the RefCell?

I think a typical observer shouldnt need to do these things but perhaps we should make that explicit in the API docs?

[yannham]

Ah, very good point, that would panic 🤔 we should definitively warn about it. I wonder if there's a way to prevent that but this is going to be difficult, since Context::current() is basically a freely accessible global, I don't see an easy and ergonomic way to prevent on_context_enter specifically to access it (without making the life of everyone else harder).

I wonder if the simplest might not be to move the call to on_context_enter out of the stack borrow entirely. One possibility is after the push: we can look at the return value of push and if it's not MAX, then we did entered the context. But since it's owned by the stack,we will need to borrow the stack to provide a reference to the context still, even if only immutably. Which sounds back to square one.

If we do it before, it should work though. We can check that there's space in the stack, then call on_context_enter, and then do the push. Wdyt? The cost is one additional load/store to the RefCell's flag I suppose, but it's touched anyway by the borrow_mut so should be in cache

[yannham]

(since it's thread-local and there's no .await point nothing else can borrow in the meantime I assume)

[scottgerring]

That sounds reasonable - want to try? The cost sounds reasonable to me especially considering with expect good cache locality; it would also be trivial to regress with the benchmark suite to quantify that in ns overhead.

Alternatively, how gross is just documenting it? I mean - if we say its forbidden, and someone writes code that does this, they will find out very quickly that it is verboten - its not going to hide some heisenbug.

It is probably illustrative to think about whether or not someone might reasonably want to do something that either produces telemetry or needs to poke the context directly in a hook. On the telemetry front I could imagine logging, at least in debug scenarios, might come up

[yannham]

Alternatively, how gross is just documenting it? I mean - if we say its forbidden, and someone writes code that does this, they will find out very quickly that it is verboten - its not going to hide some heisenbug.

Yes, at least it panics and isn't just silently wrong/corrupted/UB. Still, I'm not sure how easy it is to relate the error (that will say multiple borrows at some random context usage site, unrelated to the observer) to the the observer.

It is probably illustrative to think about whether or not someone might reasonably want to do something that either produces telemetry or needs to poke the context directly in a hook. On the telemetry front I could imagine logging, at least in debug scenarios, might come up

I agree this is the important question to answer. Technically, both directions are sound/acceptable but maybe taking multiple borrows just shouldn't happen in an observer? At least that was my initial assumption, but as you mention logging might actually be use-case. Though I expect observers to be really "low-level" and maybe not log through the same pipeline as the instrumented application.

Since the observer implementation is almost part of opentelemetry-rust (to me, it is to be thought of as an extension mechanism, and maybe at some point the thread-level context publication code will actually be moved from downstream libraries to here), I still feel like it should not affect the contexts. I'll update the documentation for now and keep the code identical. If someone comes to complain about a legitimate use-case through an issue, we can always switch to the non-locking implementation.