Skip to content

Conversation

@STRRL
Copy link
Contributor

@STRRL STRRL commented Dec 17, 2025

Moves the logging layer implementation from core/core/src/layers/logging.rs to a separate crate at core/layers/logging. This follows the modular architecture pattern and improves build modularity.

Changes:

  • Created opendal-layer-logging crate with proper manifest
  • Updated imports to use opendal_core prefix
  • Removed end-of-line comments from implementation
  • Added layers-logging feature flag to main Cargo.toml

Which issue does this PR close?

Closes #6939

relates to #6829

Rationale for this change

N/A

What changes are included in this PR?

Are there any user-facing changes?

No I guess :P

AI Usage Statement

This PR was created with assistance from Claude Code (claude-opus-4-5-20251101).

Moves the logging layer implementation from core/core/src/layers/logging.rs
to a separate crate at core/layers/logging. This follows the modular
architecture pattern and improves build modularity.

Changes:
- Created opendal-layer-logging crate with proper manifest
- Updated imports to use opendal_core prefix
- Removed end-of-line comments from implementation
- Added layers-logging feature flag to main Cargo.toml

Co-Authored-By: Claude <noreply@anthropic.com>
@STRRL STRRL requested a review from Xuanwo as a code owner December 17, 2025 06:00
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels Dec 17, 2025
Update dependencies to use the new layers-logging feature flag:
- nodejs binding: add layers-logging feature
- haskell binding: add layers-logging feature
- testkit: add opendal-layer-logging dependency

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@STRRL
Copy link
Contributor Author

STRRL commented Dec 17, 2025

let me ask AI for fixing it

STRRL and others added 5 commits December 16, 2025 22:12
Add services-memory feature to dev-dependencies so doc tests can compile.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Apply taplo and rustfmt formatting.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The retry tests in opendal-core cannot use opendal-layer-logging because
it would create a circular dependency (opendal-core -> opendal-layer-logging -> opendal-core).

Remove the LoggingLayer usage from tests - it was added for debugging purposes only.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The retry layer tests are inline unit tests within opendal-core. These
cannot use opendal-layer-logging because it would create a circular
dependency (opendal-core tests -> opendal-layer-logging -> opendal-core).

The testkit crate resolves this for external/behavior tests, but inline
unit tests within core cannot use external layer crates.

LoggingLayer was only used for debugging purposes in these tests.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@STRRL
Copy link
Contributor Author

STRRL commented Dec 17, 2025

there is still a circular deps issue, I will take a look later

@STRRL STRRL closed this Dec 17, 2025
@koushiro
Copy link
Member

koushiro commented Dec 17, 2025

Hi @STRRL, Thank you for your work on this, but I think we'd better to extract the logging/retry/timeout layer crates in a single PR to avoid conflicts.
If you want to continue, you can consider doing this, or you may also choose other services to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[core split] Layer: logging

2 participants