docs: add extension system architecture document

[gouslu]

Change Summary

PR to discuss extension system architecture in relation to implementation in PR #2141

How are these changes tested?

Are there any user-facing changes?

Yes, adds extension section to configuration in the proposes design.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.46%. Comparing base (94df6e2) to head (93667d1).
⚠️ Report is 101 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2230      +/-   ##
==========================================
- Coverage   87.46%   87.46%   -0.01%     
==========================================
  Files         558      560       +2     
  Lines      185736   186850    +1114     
==========================================
+ Hits       162456   163425     +969     
- Misses      22754    22899     +145     
  Partials      526      526              

Components	Coverage Δ
otap-dataflow	89.65% <ø> (-0.03%)	⬇️
query_abstraction	80.61% <ø> (ø)
query_engine	90.30% <ø> (-0.01%)	⬇️
syslog_cef_receivers	∅ <ø> (∅)
otel-arrow-go	52.44% <ø> (ø)
quiver	91.91% <ø> (+0.08%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[lquerel]

I like the trait-based direction in this PR. It gives the engine a cleaner abstraction boundary than passing around concrete service handles, and it feels like the right long-term shape if we want extensions to expose reusable capabilities rather than just implementation details.

I do think the naming can be improved to make the model easier to understand, and also to future-proof it if we later broaden the scope beyond per-pipeline extensions.

Right now there are two different concepts mixed under the word extension:

• the long-lived runtime unit with lifecycle (start, shutdown ordering, control channel, ...)
• the capability contract that consumers look up and use

That is why names like ExtensionTrait and TraitRegistration feel a bit hard to parse when reading the code for the first time.

I would suggest keeping Extension for the lifecycle/runtime concept, and renaming the trait-related pieces toward capability terminology. For example:

• ExtensionTrait -> ExtensionCapability
• TraitRegistration -> CapabilityRegistration
• ExtensionRegistry -> CapabilityRegistry
• extension_traits! -> extension_capabilities!

The reason I think this matters is that the consumer is not really asking for "an extension"; it is asking for a capability exposed by an extension. Making that explicit in the naming should make the design easier to follow.

One additional point: I do not think extension capabilities should be limited to receivers and exporters. Conceptually, this mechanism looks useful for any node type. Processors seem like an obvious next consumer, and I would prefer the API and naming to reflect that from the start. Even if this PR only wires the registry into receivers/exporters for now, I think the design should stay clearly node-agnostic rather than implying that extensions are specifically an ingress/egress feature.

I also think this naming would age better if we later introduce broader scopes for extensions. Today the model is per-pipeline, but I can see value in eventually supporting:

• pipeline-scoped extensions
• group-scoped extensions
• engine-scoped extensions

If we go in that direction (in future PRs), I would strongly prefer to preserve the current performance properties and keep the hot path as cheap as possible.

In particular, I think broader scopes should not imply that the hot path starts talking to a shared global service on every item. My preference would be:

• capability lookup remains a startup-time operation
• nodes cache the looked-up capability once during startup
• broader-scoped extensions push state to consumers rather than being synchronously queried on the hot path
• shared state is propagated via watch-style subscriptions, immutable snapshots, atomics, or similarly cheap read-side mechanisms
• avoid putting mutexes / blocking coordination / request-response hops on the per-message path

In other words, I think the right mental model is that broader-scoped extensions should still behave like a control-plane mechanism, not a data-plane mechanism.

If we ever support multiple scopes, I think the engine should probably build a merged capability view for each pipeline, with a clear precedence rule such as:

pipeline > group > engine

That would let pipelines override broader-scoped capabilities without changing the consumer model.

So overall: I am supportive of the trait-based design here. My main suggestion is to rename the trait-facing pieces around the idea of "capabilities", because I think that makes the current design clearer and gives us a better vocabulary if we later want to extend the mechanism to group-level or engine-level scopes while keeping the hot path fast, and while making the mechanism available uniformly to all node types rather than only receivers/exporters.

@jmacd @utpilla for visibility

[lquerel]

Retry logic is not part of the exporters, we already have a retry processor.

[gouslu]

maybe, but an exporter might have an http client with a configurable retry, that uses sth like 'HttpRetryExtension', that can be configured? I don't know, maybe not a great example.

[lquerel]

Not sure I understand what we are referring to here. Health reporting is already in place and is a core function, not really an extension-based system. Similarly, I'm not sure what is meant by periodic cleanup.

[gouslu]

Right, not a good example.

[lquerel]

Why limiting extensions to receivers and exporters?

[gouslu]

No reason, we can add it to processors, I just didn't see a start method for processors and I needed to think a little longer how to hook processors up with it.. I was expecting this feedback, thanks.

[lquerel]

Saying that the registry is stateless while it contains trait objects seems strange to me.

[gouslu]

Right, not that kind of state, but I get your point :D

[lquerel]

Is the start method the best place to pass the ExtensionRegistry?

[gouslu]

I am not sure of that. Probably factory function(?). I will think of alternatives. I had other ideas too, but let me see what I can come up with as alternatives.

[lquerel]

In these design decisions, it is not explicitly stated that the lifecycle of extensions is directly tied to the lifecycle of pipeline instances. It is represented in the diagram, but I would like it to be clear to the reader that an extension instance is not shared between pipelines.

[lquerel]

Why do we have a TimerTick variant in the ExtensionControlMsg enum if extensions manage their own timers?

[utpilla]

This seems problematic. While both approaches #2113 and #2141 agree on the fundamentals: extensions are PData-free, start first, shut down last, and expose capabilities via a registry, they differ in how these capabilities flow from extension to consumer.

In this PR (#2141), the extension struct is the capability. The extension_traits! macro clones self into the registry. Consumers get a clone of the extension. Correctness requires all mutable state to be Arc-wrapped so clones stay in sync.

In #2113, the factory builds two separate objects: the extension (lifecycle) and a handle (capability).
The extension doesn't need Clone. No macro needed.

This encourages holding internal state behind Arc

The "remember to Arc" problem:

In the clone-self mode (this PR), the engine creates three independent copies of the extension struct during pipeline build:

1. The original created by the factory, lives inside ExtensionWrapper
2. The macro clone extension_traits!() calls self.clone() and stores the copy in the ExtensionRegistry
3. The consumer clone, registry.get() clones from Minor fixes from (#212) #2 and hands it to the receiver/exporter

After registration, start() consumes #1 (the original). The exporter/receiver works with #3 (a clone of a clone). These are different objects and they only share state for fields wrapped in Arc. Any non-Arc mutable field silently diverges.

This compiles without warning:

#[derive(Clone)]
struct MyExtension {
    token_tx: Arc<watch::Sender<Token>>,  // ✅ shared across all clones
    retry_count: u32,                      // ❌ each clone gets its own copy
}

start() runs on copy #1 and increments retry_count. The exporter holds copy #3 and sees retry_count stuck at 0. The framework relies on a convention: "wrap everything mutable in Arc" which the compiler cannot enforce.

In #2113, this bug is structurally impossible. The handle and extension are different types constructed by the factory. There's nothing to diverge because they were never copies of the same object. No "remember to Arc everything" convention is needed. I think that's a better contract to offer the extension authors.

[lquerel]

I admit that detail escaped me, and it does seem error-prone. Separating extension and handle does seem like a good idea.

@gouslu @utpilla How does that integrate with the other idea of allowing multiple capabilities to be exposed by the same extension?

[utpilla]

How does that integrate with the other idea of allowing multiple capabilities to be exposed by the same extension?

In #2113, ExtensionHandles supports registering multiple handles of different types from the same extension factory:

let mut handles = ExtensionHandles::new();
handles.register(ClientAuthenticatorHandle::new(auth));
handles.register(HealthCheckHandle::new(health));
// register as many distinct handle types as needed

Each register() call adds a (TypeId, ErasedHandle) entry. Consumers look them up by extension name + concrete type:

let auth = registry.get::<ClientAuthenticatorHandle>("azure-auth")?;
let health = registry.get::<HealthCheckHandle>("azure-auth")?;

The difference from this PR (or #2141) is what gets registered: concrete handle types vs trait objects. In this design, extension_traits!(BearerTokenProvider, HealthCheck) registers dyn Trait objects (clones of the extension itself). In my approach, the factory creates purpose-built handle structs and registers them independently. The 1-to-N relationship is the same; the unit of registration differs.

[gouslu]

Fair criticism @utpilla, but this is by design. Extensions will require clone to be implemented, that would be the signal that extension developer needs for shared state. However, it is not explicit. I thought, together with documentation, it could be fine. This design trade-off comes from multiple decisions that lead to this:

• simplify extension traits, easy to add and maintain.
• simplify extension development.
• avoid complexity of mutex or any other more complex ways of dealing with memory barriers within the framework.
• wrap all mutable fields with Arc<> is a very basic concept to grasp, lowers barrier of entry and complexity of implementation.
• reduce boilerplate and hide behind a simple macro that is owned by the engine.

In PR #2113, it still seems that if your start method had to regularly update the token, it would have to find a way to share it with the extension instance, which would require the author to have an Arc<> struct field, in your case it is Arc<Mutex<>> since you made the handles Sync, which doesn't have to be required if the job is to provide a tonic interceptor. In #2113 registry still requires Clone + Send + 'static which can (in theory) lead to same error proneness at some place, if not by the extension implementer. In addition to all the plumbing required to connect handle to trait.

Edit: I understand the handle based design prevents the mistake, and I think that's the main benefit of it. I don't like the boilerplate of handle based design, as well as the mutex requirement to satisfy Sync. If we change this design to be similar to your handle based design, but drop the Sync requirement and maybe find ways to get rid of the boilerplate, I could be happier. I'll look into it a little bit and maybe some other alternatives.

[utpilla]

On simplicity of usage and plumbing: In #2113, the "plumbing" is three lines in the factory: ExtensionHandles::new(), .register(handle), and passing handles to ExtensionWrapper. The extension author implements a plain trait — no #[derive(Clone)], no macro, no Arc wrapping. That's not adding any significant complexity, it's just more explicit wiring.

On Arc<Mutex<>> and Sync: The Arc<Mutex<>> is inside the handles. It's framework code, not extension-author code. The extension author just implements a simple trait for example ClientAuthenticator: Send (a plain trait, no Sync). Neither the extension author nor the consumer ever writes Arc or Mutex. As for whether Sync is needed for tonic interceptors, the handle is what enters tonic services, and the framework makes it Sync automatically. The extension author doesn't think about it.

On "same error proneness": The Clone + Send + 'static requirement on handles is fundamentally different from Clone on the extension struct. A handle is a read endpoint — a small, purpose-built struct whose only job is to let consumers read state that the extension produces. There's no lifecycle logic, no credentials, no retry state. It's just a thin bridge between the extensions and its consumers. The extension struct is the whole machine. Requiring Clone on that means every field the author adds — config, credentials, senders, counters — must be individually assessed for Arc-worthiness. A handle doesn't have that problem because it only contains what consumers need to read, not what the extension needs to run.

On sharing tokens between start() and the handle: The extension author doesn't use Arc for this. The factory creates two separate objects — the extension (which produces state) and the handle (which lets consumers read it). They're connected internally, but neither the extension author nor the consumer has to think about shared ownership. In #2141, the extension struct is the handle, so every field that start() mutates must be manually wrapped in Arc to stay in sync with the cloned copies consumers receive.

[gouslu]

This is the example extension based on what I propose in PR #2141 -> https://github.com/gouslu/otel-arrow/blob/97a702c1090c9df1a701bc5af7cf590547534603/rust/otap-dataflow/crates/contrib-nodes/src/extensions/azure_identity_auth_extension/extension.rs

[gouslu]

I have found out that there are multiple popular crates out there that takes the same approach to shared state as mine. Axum, built by the tokio team (tokio-rs/axum), uses the same Clone + Arc convention for shared state — documented, not enforced at compile time. You can see the documentation here for shared mutable state in this section:

This approach that I have also ended up with seems to be adopted by multiple widely used libraries, including axum with 1.4k forks and 25.3k stars.

What I have come up with the extension system I have developed is basically a service locator pattern while avoiding Sync boundaries entirely, and one of the main differences with axum seems to be that axum uses generics for state, such as Extension<T> where T is the state. Our design wants to avoid a defined state as a generic type parameter because each extension implementer can have their own shared state.

All these being said, inspired by axum, I am trying to figure out if I can actually group all shared state into a struct and make it available in trait implementations, somehow. I'll keep working on it, but I am not sure if it will get anywhere without a significant trade-off in the design.

Edit: The same state-sharing pattern (author is responsible for Arc<> wrapping due to Clone requirement) can be found in tonic, tower, and warp — all by the tokio team all of them rely on documentation and tutorials for this.

cc: @lquerel, @jmacd, @utpilla

[gouslu]

What I think I would document for the extension authors to make shared state management easy for them is to follow a convention where they group up their internal shared state into a struct and wrap the whole struct in Arc<MySharedState> and only access it via self.state or something like that. Which is still not enforced by the compiler but it is a convention to make shared state management easier for extension authors.

Like I mentioned, I will try to find a way to enforce this pattern, but I am not very hopeful that it'll come as an "all-win" situation.

[utpilla]

I prefer correctness by construction — where the framework's types and API make incorrect usage impossible — over correctness by convention, where the author has to remember to do the right thing (e.g., Arc-wrap shared fields).

That said, I'll admit this does make the clone-self approach in #2141 more acceptable than I initially thought. If Axum has normalized this pattern, we could too. But why settle for "acceptable" when we can make the problem impossible in the first place?

[gouslu]

The claim about the handle based system not having the same issue is incorrect and I have demonstrated it here.

Maybe you can demonstrate how it is made impossible with an example.

Imagine a scenario where both lifecycle and the extension instances need to write to the same data. It could, for example, be a ratelimiter, where lifecycle resets the quota based on time or information accessible to the extension from the engine via start method, and the extension has a "try_acquire" method to get quota.