Skip to content

feat(error): enhance error logging with tracing integration#186

Merged
m4tx merged 1 commit intocot-rs:masterfrom
Soroushsrd:feature/tracing-error-logs
Feb 23, 2025
Merged

feat(error): enhance error logging with tracing integration#186
m4tx merged 1 commit intocot-rs:masterfrom
Soroushsrd:feature/tracing-error-logs

Conversation

@Soroushsrd
Copy link
Contributor

This PR enhances error logging in Cot by implementing structured logging with the tracing crate. The goal is to improve observability and enable better integration with OpenTelemetry for monitoring errors in production services.

Changes:

  • Implement structured error logging with tracing spans
  • Add request context and metadata to error logs
  • Include backtrace information in error spans
  • Add comprehensive test coverage for logging functionality
  • Maintain backwards compatibility with existing error pages

Added new tests that verify:

  • Error logging with request context
  • Error logging without request context
  • Panic logging
  • Not found logging
  • Integration with error page handlers

related Issues
Implements better error monitoring as discussed in issue #160

  • Tests added for new functionality (some might be redundant)
  • Code follows project style guidelines
  • All tests passing
  • No new warnings

@Soroushsrd
Copy link
Contributor Author

Soroushsrd commented Feb 22, 2025

The CI test failure appears to be due to a dependency (hybrid-array-0.3.0) requiring edition2024, which isn't stable in Cargo 1.84.1. This is unrelated to my changes for error logging.

Error message:

error: failed to parse manifest at `/Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hybrid-array-0.3.0/Cargo.toml`
Caused by:
  feature `edition2024` is required

The CI test failure is actually related to this transitive dependency chain:
hmac -> digest -> crypto-common -> hybrid-array

We could:

  1. Pin hmac to a version that uses older versions of these dependencies
  2. Pin digest to a version before it started using crypto-common 0.2.0-rc.1
  3. Update the project's MSRV to a version that supports edition2024

Ill be happy to do any of them. Please let me know. @m4tx

@m4tx
Copy link
Member

m4tx commented Feb 22, 2025

Updating our MSRV (so option 3) to 1.85 is fine; we'll want to upgrade to edition 2024 soon anyways, and maintaining a stable MSRV isn't super important at this stage of the project development anyway. Thanks!

@codecov
Copy link

codecov bot commented Feb 23, 2025

Codecov Report

Attention: Patch coverage is 95.31250% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cot/src/error_page.rs 95.31% 2 Missing and 4 partials ⚠️
Flag Coverage Δ
rust 78.55% <95.31%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cot/src/error_page.rs 89.42% <95.31%> (+2.80%) ⬆️

Copy link
Member

@m4tx m4tx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of fixing the dependency issue, since it was causing problems elsewhere, too (see #187).

The change looks good to me; please ensure the macro invocations are consistently formatted (see my review comments), and please fix the cargo fmt issues, and then it'll be ready to merge!

@Soroushsrd
Copy link
Contributor Author

Thank you Ill keep that in mind for future contributions.

@Soroushsrd Soroushsrd force-pushed the feature/tracing-error-logs branch from c0cd3ea to 36ce724 Compare February 23, 2025 13:05
Add structured logging for errors and panics using the tracing crate to
improve observability. This change enables better integration with
OpenTelemetry for monitoring errors in production services.
- Add structured error logging with metadata
- Include request context in error logs
- Add backtrace information to error spans
- Add comprehensive test coverage for logging(although some match the
older ones)
- Maintain existing error page functionality

The logging implementation includes:
- Error type and message
- Stack traces- Request information (method, path)
- Proper span contexts for OpenTelemetry
Fixes cot-rs#160
@Soroushsrd Soroushsrd force-pushed the feature/tracing-error-logs branch from 36ce724 to 7a4b4c8 Compare February 23, 2025 13:24
Copy link
Member

@m4tx m4tx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot for the contribution!

@m4tx m4tx merged commit 6f9e4fb into cot-rs:master Feb 23, 2025
20 checks passed
This was referenced Feb 23, 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.

2 participants