Skip to content

feat: Add Clone implementation to SdkLogger for consistency with SdkTracer #3059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 7, 2025

This PR resolves an API inconsistency where SdkTracer implements Clone but SdkLogger does not, requiring users to wrap SdkLogger in Arc for cloning scenarios.

Changes Made

  • Added Clone to the derive macro for SdkLogger struct
  • Added a test to verify the clone functionality works correctly

Why This Change is Safe

Both SdkLogger and SdkTracer have identical structure:

  • scope: InstrumentationScope (already implements Clone)
  • provider: SdkLoggerProvider / SdkTracerProvider (both already implement Clone)

Since all fields already implement Clone, adding it to SdkLogger is a straightforward and safe change.

Example Usage

use opentelemetry_sdk::logs::SdkLoggerProvider;
use opentelemetry::logs::LoggerProvider;

let provider = SdkLoggerProvider::builder().build();
let logger = provider.logger("my-service");

// Before: This would fail to compile
// let cloned_logger = logger.clone(); // ❌ 

// After: This now works consistently with SdkTracer
let cloned_logger = logger.clone(); // ✅

This makes the logging and tracing APIs consistent and reduces the need for Arc wrappers when cloning is required.

Fixes #3058.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link

linux-foundation-easycla bot commented Jul 7, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: lalitb / name: Lalit Kumar Bhasin (80057bf)
  • ✅ login: cijothomas / name: Cijo Thomas (24f2c99)

@Copilot Copilot AI changed the title [WIP] inconsistency with Clone implementations for SdkLogger / SdkTracer Add Clone implementation to SdkLogger for consistency with SdkTracer Jul 7, 2025
@Copilot Copilot AI requested a review from lalitb July 7, 2025 18:59
Copilot finished work on behalf of lalitb July 7, 2025 18:59
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.1%. Comparing base (0631070) to head (80057bf).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3059     +/-   ##
=======================================
- Coverage   80.2%   80.1%   -0.1%     
=======================================
  Files        126     126             
  Lines      21949   21949             
=======================================
- Hits       17604   17603      -1     
- Misses      4345    4346      +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.

@cijothomas
Copy link
Member

cijothomas commented Jul 10, 2025

@copilot run cargo fmt and other lint tools

Copy link
Contributor Author

Copilot AI commented Jul 10, 2025

@copilot run cargo fmt and other lint tools

I've successfully run cargo fmt and clippy on the changes. The formatting was applied and I fixed a clippy warning in the test by replacing assert!(true) with a meaningful assertion. All linting passes now. Changes committed in 4c9072f.

@Copilot Copilot AI requested a review from cijothomas July 10, 2025 00:23
Copilot finished work on behalf of cijothomas July 10, 2025 00:23
@davidhewitt davidhewitt mentioned this pull request Jul 16, 2025
4 tasks
@cijothomas cijothomas changed the title Add Clone implementation to SdkLogger for consistency with SdkTracer feat: Add Clone implementation to SdkLogger for consistency with SdkTracer Jul 16, 2025
Copilot finished work on behalf of cijothomas July 16, 2025 15:04
@Copilot Copilot AI requested a review from cijothomas July 16, 2025 15:04
@lalitb lalitb marked this pull request as ready for review July 16, 2025 16:19
@lalitb lalitb requested a review from a team as a code owner July 16, 2025 16:19
@cijothomas
Copy link
Member

Oops! Pushed to wrong branch.

@cijothomas cijothomas closed this Jul 16, 2025
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.

inconsistency with Clone implementations for SdkLogger / SdkTracer
3 participants