Skip to content

Conversation

giortzisg
Copy link
Contributor

Description

Create a debuglog package to easily access the sentry.DebugLogger without depending on the root level. This change aims to remove any cyclic imports when internal packages are used on the top level, but also want to access the debug logger.

Issues

@giortzisg giortzisg requested a review from cleptric October 6, 2025 09:23
@giortzisg giortzisg self-assigned this Oct 6, 2025
Copy link

linear bot commented Oct 6, 2025

cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 64.70588% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.92%. Comparing base (4edee8a) to head (8df1e66).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
transport.go 57.89% 8 Missing ⚠️
tracing.go 58.33% 5 Missing ⚠️
util.go 33.33% 4 Missing ⚠️
client.go 75.00% 2 Missing ⚠️
fasthttp/sentryfasthttp.go 0.00% 1 Missing ⚠️
fiber/sentryfiber.go 0.00% 1 Missing ⚠️
integrations.go 66.66% 1 Missing ⚠️
logrus/logrusentry.go 0.00% 1 Missing ⚠️
slog/converter.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
- Coverage   86.98%   86.92%   -0.06%     
==========================================
  Files          54       56       +2     
  Lines        6068     6080      +12     
==========================================
+ Hits         5278     5285       +7     
- Misses        645      649       +4     
- Partials      145      146       +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.

@giortzisg giortzisg changed the title chore: create debuglog package ref: create debuglog package Oct 6, 2025
@lcian lcian self-requested a review October 6, 2025 12:36
Copy link
Member

@lcian lcian left a comment

Choose a reason for hiding this comment

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

LGTM in principle and I understand the problem this solves.
I think it's a bit unfortunate that we have to lock on every message, and I wonder if there's a different way to do it without locking.
Anyways, I would assume that customers don't enable Debug: true in production, so it shouldn't be a big deal in practice.

@giortzisg
Copy link
Contributor Author

@lcian We can actually remove the mutex locks by removing the SetLogger function. I guess SetOutput would suffice.

Copy link
Member

@lcian lcian left a comment

Choose a reason for hiding this comment

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

Nice!

@giortzisg giortzisg merged commit c24b748 into master Oct 6, 2025
17 of 18 checks passed
@giortzisg giortzisg deleted the chore/debug-logger branch October 6, 2025 13:31
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.

Create a package for debug logger and move it internally

2 participants