Skip to content

feat(appender-tracing): add builder support for custom instrumentation scope#3428

Open
DmitryAstafyev wants to merge 5 commits intoopen-telemetry:mainfrom
DmitryAstafyev:3415-appender-tracing-with-scope
Open

feat(appender-tracing): add builder support for custom instrumentation scope#3428
DmitryAstafyev wants to merge 5 commits intoopen-telemetry:mainfrom
DmitryAstafyev:3415-appender-tracing-with-scope

Conversation

@DmitryAstafyev
Copy link
Copy Markdown

Changes

This PR adds OpenTelemetryTracingBridgeBuilder::with_scope() to allow configuring the OpenTelemetry InstrumentationScope used by the appender logger.

The implementation also updates OpenTelemetryTracingBridgeBuilder to hold the logger provider until build(), so the builder can expose a set of configuration methods:

  • with_scope()
  • with_span_attribute_allowlist()
  • and potentially additional builder options in the future

Design Notes

This change introduces a lifetime parameter on OpenTelemetryTracingBridgeBuilder<'a>. That was done to keep the builder aligned with the existing style, where the bridge is constructed from a provider reference rather than from a pre-created logger.

In return, this keeps the API shape cleaner and gives us a more natural builder surface with related configuration methods grouped together.

Compatibility

Technically, adding the lifetime to the public builder type is a breaking API change.

However, this builder API has not yet shipped in a released version, so this does not affect production users of published crates. In practice, the change is limited to the unreleased API surface.

Testing

Added a unit test (tracing_appender_with_custom_scope) covering with_scope() and verifying that the configured InstrumentationScope is propagated to emitted logs.

Related

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)

@DmitryAstafyev DmitryAstafyev requested a review from a team as a code owner March 18, 2026 20:26
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 18, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.3%. Comparing base (67e7dff) to head (866bae7).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3428   +/-   ##
=====================================
  Coverage   83.2%   83.3%           
=====================================
  Files        128     128           
  Lines      25048   25092   +44     
=====================================
+ Hits       20859   20904   +45     
+ Misses      4189    4188    -1     

☔ View full report in Codecov by Sentry.
📢 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.

@DmitryAstafyev
Copy link
Copy Markdown
Author

Hello @cijothomas, could you please take a look at this PR? It's my first time contributing here, and I'm ready to correct/change if it's required. Many thanks in advance for review.

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Mar 23, 2026

The new scope support looks fine, but this changes the public builder API in a breaking way. OpenTelemetryTracingBridgeBuilder was previously an owned public type, and this change makes it borrow the provider and adds a lifetime parameter. That can break downstream code that names the builder type explicitly, stores it, or returns it from helper functions. For example:

fn make_builder<P, L>(
    provider: &P,
) -> opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridgeBuilder<P, L>
where
    P: opentelemetry::logs::LoggerProvider<Logger = L> + Send + Sync,
    L: opentelemetry::logs::Logger + Send + Sync,
{
    opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge::builder(provider)
}

Can we keep the builder owned while still adding with_scope()?

@DmitryAstafyev DmitryAstafyev force-pushed the 3415-appender-tracing-with-scope branch from aba679c to bf413c0 Compare March 24, 2026 06:35
@DmitryAstafyev
Copy link
Copy Markdown
Author

Thank you @lalitb for the review and for highlighting this.

You are correct: my original proposal introduced a breaking API change by adding a lifetime to the public OpenTelemetryTracingBridgeBuilder type. That is problematic for downstream users who explicitly name, store, or return the builder type.

My goal was to keep configuration cohesive in the builder (with_scope() as a builder method), while still borrowing the provider. In practice, that design required a lifetime on the public builder, which weakens API stability.

I have updated the implementation to remove the lifetime and keep the builder owned, as requested.

The current trade-off is configuration split across two entry points:

  • OpenTelemetryTracingBridge::with_scope(...) for scope configuration
  • builder methods (for example with_span_attribute_allowlist(...)) for additional options

This preserves compatibility, but is less uniform than a fully builder-centric API.

From an API stability perspective, I also see a risk in a public builder whose signature may need to grow over time. If you agree, we could consider a more stable construction surface (for example, keep builder(provider) minimal/stable and expose additional dedicated constructors for optional configuration like scope).

Would you prefer one of these directions?

  1. Keep builder(provider) public and stable, and expose scope via a separate public constructor (for example builder_with_scope(...)) so we avoid growing the builder signature.

  2. Make OpenTelemetryTracingBridge::builder(...) non-public and keep only stable public constructors (new, with_scope, etc.), so internal constructor changes do not affect public API compatibility.

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Mar 25, 2026

Thanks for the update and the well-thought-out options @DmitryAstafyev. I'd go with your Option 1 - keep builder(provider) unchanged and add builder_with_scope(provider, scope) as a new entry point.

The scope isn't a configuration property of the bridge itself - it's a parameter needed alongside the provider to create the underlying logger. Since it must be known at the same time as the provider, a separate builder_with_scope() entry point is the natural fit - it makes that requirement explicit in the API rather than hiding it behind an Option parameter.

also, note that you need to sign CLA before this can be moved further.

@DmitryAstafyev
Copy link
Copy Markdown
Author

Thanks @lalitb, for your response. I've made the requested changes.

  • builder(provider: &P) stay as it was
  • builder_with_scope(...) has beed added
  • with_scope(...) has been added as a constructor

Let me know, please, if I should do something else.

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Mar 26, 2026

Thanks @DmitryAstafyev LGTM. Will approve/merge this once the CLA is signed.

@DmitryAstafyev DmitryAstafyev force-pushed the 3415-appender-tracing-with-scope branch from 6684d49 to 1bef051 Compare March 30, 2026 17:14
Comment thread opentelemetry-appender-tracing/src/layer.rs Outdated
@DmitryAstafyev DmitryAstafyev force-pushed the 3415-appender-tracing-with-scope branch from 1bef051 to 866bae7 Compare April 6, 2026 08:48
@lalitb lalitb enabled auto-merge April 7, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Allow passing InstrumentationScope when creating an instance of OpenTelemetryTracingBridge

3 participants