sentry-go icon indicating copy to clipboard operation
sentry-go copied to clipboard

Add Slog integration

Open ribice opened this issue 1 year ago • 4 comments

Resolves #527

ribice avatar Jul 28 '24 10:07 ribice

Codecov Report

Attention: Patch coverage is 86.68731% with 43 lines in your changes missing coverage. Please review.

Project coverage is 83.21%. Comparing base (eb74cc1) to head (08340ae). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
slog/common.go 89.13% 16 Missing and 4 partials :warning:
slog/sentryslog.go 66.66% 14 Missing and 1 partial :warning:
slog/converter.go 91.48% 7 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #865      +/-   ##
==========================================
+ Coverage   83.01%   83.21%   +0.20%     
==========================================
  Files          51       54       +3     
  Lines        5433     5756     +323     
==========================================
+ Hits         4510     4790     +280     
- Misses        767      804      +37     
- Partials      156      162       +6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 28 '24 10:07 codecov[bot]

@cleptric Just realised we can't use 1.18 with exp/slog (requires at least 1.19): https://github.com/golang/go/issues/57113

ribice avatar Jul 29 '24 09:07 ribice

Will this be merged?

assembly-winston avatar Sep 16 '24 18:09 assembly-winston

Yes. We're holding off merging new features for a week or so to give us time to fix potential issues in 0.29.0. Afterwards, we'll get this out.

cleptric avatar Sep 16 '24 18:09 cleptric

Let's make this it's own package. You can update https://github.com/getsentry/sentry-go/blob/master/.craft.yml and I'll add it to our release registry.

I updated .craft.yml and made slog integration a module. Is there anything else that needs to be updated?

ribice avatar Nov 05 '24 13:11 ribice

Just CI needs to pass and then we can get this merged 🙂

cleptric avatar Nov 05 '24 13:11 cleptric

@cleptric Shall I update the vet script in Makefile to skip if no go files are found (which is the case for slog in Go <1.21 due to build constraints)?

ribice avatar Nov 05 '24 19:11 ribice

Waiting for #898 to be merged before I merge this.

ribice avatar Nov 08 '24 17:11 ribice