Skip to content

Add logrus logs integration #1036

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

Merged
merged 11 commits into from
Jun 23, 2025
Merged

Add logrus logs integration #1036

merged 11 commits into from
Jun 23, 2025

Conversation

giortzisg
Copy link
Contributor

@giortzisg giortzisg commented Jun 13, 2025

Description

Closes #1028.
Addinglogrus integration, by introducing logHook. Refactored and deprecated existing logic ofNew and NewFromClient functionality, separating them into two different constructors for events and logs. These are NewEventHook and NewLogHook methods for creating hooks with a new Sentry client, and corresponding FromClient methods for using an existing client. These replace the deprecated New and NewFromClient methods. Example of logrus hook usage:

// example for creating two logrus hooks for sending info messages as logs and 
// logs hook
levels := []logrus.Level{logrus.InfoLevel}
	logHook, err := NewLogHook(levels, sentry.ClientOptions{
		Dsn:         "http://whatever@example.com/1337",
		Environment: "test",
	})

// event hook
levels := []logrus.Level{logrus.ErrorLevel}
	eventHook, err := NewEventHook(levels, sentry.ClientOptions{
		Dsn:         "http://whatever@example.com/1337",
		Environment: "test",
	})

logger := logrus.New()
logger.AddHook(logHook)
logger.AddHook(eventHook)

@giortzisg giortzisg requested a review from cleptric June 13, 2025 08:49
@giortzisg giortzisg self-assigned this Jun 13, 2025
@giortzisg giortzisg added the Feature Issue type label Jun 13, 2025
Copy link

codecov bot commented Jun 13, 2025

Codecov Report

Attention: Patch coverage is 90.30303% with 16 lines in your changes missing coverage. Please review.

Project coverage is 86.26%. Comparing base (2837952) to head (82fd2bb).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
logrus/logrusentry.go 90.30% 15 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1036      +/-   ##
==========================================
+ Coverage   85.63%   86.26%   +0.63%     
==========================================
  Files          55       55              
  Lines        5526     5664     +138     
==========================================
+ Hits         4732     4886     +154     
+ Misses        651      637      -14     
+ Partials      143      141       -2     

☔ 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 requested a review from Copilot June 13, 2025 09:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Logrus logging integration with Sentry by refactoring the existing hook constructors and introducing a new logHook alongside the existing eventHook.

  • Introduces separate constructors (NewEventHook and NewLogHook) and corresponding FromClient methods.
  • Refactors and deprecates the old New and NewFromClient methods.
  • Adds a new logHook implementation to handle logs with Sentry's logger integration.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
logrus/logrusentry.go Refactored hooks into eventHook and introduced logHook for logs.
logrus/go.mod Added new test and indirect dependencies.
log.go Updated sentryLogger to add a default "sentry.origin" attribute.
Comments suppressed due to low confidence (2)

log.go:147

  • There is an inconsistency between the default 'sentry.origin' attribute values in log.go ('auto.logger.log') and in logrusentry.go (set as 'auto.logger.logrus'). Consider reconciling these values to ensure consistent tagging across logging outputs.
if _, ok := attrs["sentry.origin"]; !ok {

logrus/logrusentry.go:237

  • A new logHook implementation has been introduced. Please ensure that comprehensive unit tests are added to cover its behavior, including log level handling and data type conversions.
type logHook struct {

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Can we add a test that shows that both log and event hooks can coexist together?

We also need to update the README.

// Handle request field
key := h.key(FieldRequest)
if req, ok := entry.Data[key].(*http.Request); ok {
h.logger.SetAttributes(attribute.String("http.method", req.Method))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
h.logger.SetAttributes(attribute.String("http.method", req.Method))
h.logger.SetAttributes(attribute.String("http.request.method", req.Method))

key := h.key(FieldRequest)
if req, ok := entry.Data[key].(*http.Request); ok {
h.logger.SetAttributes(attribute.String("http.method", req.Method))
h.logger.SetAttributes(attribute.String("http.url", req.URL.String()))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
h.logger.SetAttributes(attribute.String("http.url", req.URL.String()))
h.logger.SetAttributes(attribute.String("url.full", req.URL.String()))

if err != nil {
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

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

We should short-circuit this if EnableLogs is set to false.

@giortzisg
Copy link
Contributor Author

added test & README examples

@giortzisg giortzisg requested a review from AbhiPrasad June 17, 2025 13:02
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

nice, great work @giortzisg.

@giortzisg giortzisg merged commit f43c181 into master Jun 23, 2025
17 checks passed
@giortzisg giortzisg deleted the logrus-integration branch June 23, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Issue type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Structured logging support for logrus
2 participants