Support multiple otlp providers

[ScriptSmith]

No description provided.

[greptile-apps]

Greptile Summary

This PR extends the OTLP usage sink from a single optional backend to an arbitrary number of backends, enabling fan-out of usage data (tokens, costs, latency) to multiple OpenTelemetry-compatible destinations simultaneously (e.g. Grafana Cloud + Datadog).

Key changes:

• UsageConfig.otlp is widened from Option<UsageOtlpConfig> → Vec<UsageOtlpConfig>, and the TOML config syntax changes from [observability.usage.otlp] to [[observability.usage.otlp]] — this is a breaking change for anyone already using OTLP and there is no migration shim or changelog entry.
• A new name field on UsageOtlpConfig gives each sink a human-readable label for log output; it falls back to the endpoint URL, then to "otlp".
• UsageSink::name() is correctly widened from &'static str to &str to accommodate the owned String stored in OtlpSink.
• The server startup loop now iterates over all configured sinks, initialising each one independently and pushing it into the composite sink.

Confidence Score: 4/5

• Safe to merge after confirming the breaking config-format change is documented and existing operators are aware.
• The implementation logic is correct — the loop, name resolution, and lifetime widening are all sound. The one concrete concern is the undocumented breaking TOML syntax change ([otlp] → [[otlp]]) that will cause a hard parse failure at startup for any deployment currently using OTLP.
• src/config/observability.rs — the breaking config schema change needs a migration note or backward-compat shim.

Important Files Changed

Filename	Overview
src/config/observability.rs	Changes otlp field from Option<UsageOtlpConfig> to Vec<UsageOtlpConfig> and adds a name field; the TOML syntax change from [...] to [[...]] is a breaking config migration with no backward-compatibility shim.
src/usage_sink.rs	Adds name: String to OtlpSink, resolves it from config name → endpoint → "otlp", and widens name() return type from &'static str to &str across all trait implementations. Clean and correct.
src/cli/server.rs	Replaces single-sink if let Some branch with a loop over all configured sinks; logic is correct but the mid-function trait use import is an unusual style choice.

Sequence Diagram

sequenceDiagram
    participant S as server.rs startup
    participant C as UsageConfig otlp list
    participant O as OtlpSink new
    participant CS as CompositeSink
    participant BW as Buffer Worker

    S->>C: iterate otlp configs
    loop for each enabled OtlpConfig
        S->>O: new(otlp_config, tracing_config)
        O-->>S: Ok(OtlpSink with name+logger)
        S->>CS: sinks.push(Arc::new(otlp_sink))
    end
    S->>CS: CompositeSink::new(sinks)
    S->>BW: buffer.start_worker(composite_sink)
    Note over BW,CS: On each flush, CompositeSink fans out write_batch to all OtlpSinks

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/config/observability.rs
Line: 782

Comment:
**Breaking config change — no migration path**

Changing `otlp` from `Option<UsageOtlpConfig>` to `Vec<UsageOtlpConfig>` alters the expected TOML syntax. Any existing deployment that has:

```toml
[observability.usage.otlp]
enabled = true
endpoint = "http://localhost:4317"
```

will now fail to deserialize at startup with a type mismatch, because a TOML inline table (`[section]`) cannot be coerced into a `Vec` — the new syntax requires:

```toml
[[observability.usage.otlp]]
enabled = true
endpoint = "http://localhost:4317"
```

Since `serde` will return an error rather than silently ignoring the old key, this is a hard startup failure for anyone who already has OTLP configured. Consider adding a note in the changelog/PR description, and ideally a custom deserializer or a serde `alias` / migration note so operators aren't surprised.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/server.rs
Line: 300-301

Comment:
**Trait import placed mid-function**

Placing a `use` statement inside the body of a function (rather than at the top of the module or at the start of the enclosing block) is unusual in Rust and can be surprising to readers. The trait import is needed so that `otlp_sink.name()` resolves, but a cleaner place would be at the top of the module alongside the other `#[cfg(feature = "otlp")]` imports (e.g. near where `crate::config::UsageOtlpConfig` and `TracingConfig` are imported in `usage_sink.rs`), or at the very start of the `if let Some(buffer)` block.

```suggestion
        #[cfg(feature = "otlp")]
        {
            use usage_sink::UsageSink as _;
            for otlp_config in &config.observability.usage.otlp {
```

Alternatively, expose `name()` as an inherent method on `OtlpSink` (in addition to the trait impl) so no trait import is needed at the call site.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "Review fix"}

[ScriptSmith]

@greptile-apps

[greptile-apps]

Breaking config change — no migration path

Changing otlp from Option<UsageOtlpConfig> to Vec<UsageOtlpConfig> alters the expected TOML syntax. Any existing deployment that has:

[observability.usage.otlp]
enabled = true
endpoint = "http://localhost:4317"

will now fail to deserialize at startup with a type mismatch, because a TOML inline table ([section]) cannot be coerced into a Vec — the new syntax requires:

[[observability.usage.otlp]]
enabled = true
endpoint = "http://localhost:4317"

Since serde will return an error rather than silently ignoring the old key, this is a hard startup failure for anyone who already has OTLP configured. Consider adding a note in the changelog/PR description, and ideally a custom deserializer or a serde alias / migration note so operators aren't surprised.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/config/observability.rs
Line: 782

Comment:
**Breaking config change — no migration path**

Changing `otlp` from `Option<UsageOtlpConfig>` to `Vec<UsageOtlpConfig>` alters the expected TOML syntax. Any existing deployment that has:

```toml
[observability.usage.otlp]
enabled = true
endpoint = "http://localhost:4317"
```

will now fail to deserialize at startup with a type mismatch, because a TOML inline table (`[section]`) cannot be coerced into a `Vec` — the new syntax requires:

```toml
[[observability.usage.otlp]]
enabled = true
endpoint = "http://localhost:4317"
```

Since `serde` will return an error rather than silently ignoring the old key, this is a hard startup failure for anyone who already has OTLP configured. Consider adding a note in the changelog/PR description, and ideally a custom deserializer or a serde `alias` / migration note so operators aren't surprised.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Trait import placed mid-function

Placing a use statement inside the body of a function (rather than at the top of the module or at the start of the enclosing block) is unusual in Rust and can be surprising to readers. The trait import is needed so that otlp_sink.name() resolves, but a cleaner place would be at the top of the module alongside the other #[cfg(feature = "otlp")] imports (e.g. near where crate::config::UsageOtlpConfig and TracingConfig are imported in usage_sink.rs), or at the very start of the if let Some(buffer) block.

Suggested change

	#[cfg(feature = "otlp")]
	if let Some(otlp_config) = &config.observability.usage.otlp
	&& otlp_config.enabled
	{
	use usage_sink::UsageSink as _;
	#[cfg(feature = "otlp")]
	{
	use usage_sink::UsageSink as _;
	for otlp_config in &config.observability.usage.otlp {

Alternatively, expose name() as an inherent method on OtlpSink (in addition to the trait impl) so no trait import is needed at the call site.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cli/server.rs
Line: 300-301

Comment:
**Trait import placed mid-function**

Placing a `use` statement inside the body of a function (rather than at the top of the module or at the start of the enclosing block) is unusual in Rust and can be surprising to readers. The trait import is needed so that `otlp_sink.name()` resolves, but a cleaner place would be at the top of the module alongside the other `#[cfg(feature = "otlp")]` imports (e.g. near where `crate::config::UsageOtlpConfig` and `TracingConfig` are imported in `usage_sink.rs`), or at the very start of the `if let Some(buffer)` block.

```suggestion
        #[cfg(feature = "otlp")]
        {
            use usage_sink::UsageSink as _;
            for otlp_config in &config.observability.usage.otlp {
```

Alternatively, expose `name()` as an inherent method on `OtlpSink` (in addition to the trait impl) so no trait import is needed at the call site.

How can I resolve this? If you propose a fix, please make it concise.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}