Skip to content

Commit 3c002e7

Browse files
authored
Merge pull request #801 from n2h9/logger-caller-hook-use-function-name-to-filter-out
[logger] use function in caller hook to filter out meshkit/logger and logrus package
2 parents dba3cce + bb82312 commit 3c002e7

File tree

2 files changed

+35
-34
lines changed

2 files changed

+35
-34
lines changed

logger/caller_hook.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ type CallerHook struct {
1515
skippedPaths []string
1616
}
1717

18-
// defaultCallerSkippedPaths contains the default path patterns to skip when finding the real caller
18+
// defaultCallerSkippedPaths contains the default function name patterns to skip when finding the real caller
1919
var (
2020
defaultCallerSkippedPaths = []string{
21-
"meshkit/logger",
22-
"sirupsen/logrus",
21+
"github.com/meshery/meshkit/logger",
22+
"github.com/sirupsen/logrus",
2323
}
2424
defaultCallerSkippedPathsMu sync.RWMutex
2525
)
@@ -36,10 +36,16 @@ func (hook *CallerHook) Levels() []logrus.Level {
3636
return logrus.AllLevels
3737
}
3838

39-
// shouldSkipFrame checks if a file path should be skipped
40-
func (hook *CallerHook) shouldSkipFrame(file string) bool {
39+
// shouldSkipFrame checks if a function should be skipped based on function name
40+
func (hook *CallerHook) shouldSkipFrame(pc uintptr) bool {
41+
fn := runtime.FuncForPC(pc)
42+
if fn == nil {
43+
return false
44+
}
45+
46+
funcName := fn.Name()
4147
for _, path := range hook.skippedPaths {
42-
if strings.Contains(file, path) {
48+
if strings.Contains(funcName, path) {
4349
return true
4450
}
4551
}
@@ -55,7 +61,7 @@ func (hook *CallerHook) Fire(entry *logrus.Entry) error {
5561
break
5662
}
5763

58-
if !hook.shouldSkipFrame(file) {
64+
if !hook.shouldSkipFrame(pc) {
5965
fn := runtime.FuncForPC(pc)
6066
funcName := "unknown"
6167
if fn != nil {

logger/caller_hook_test.go

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,18 @@ package logger
33
import (
44
"bytes"
55
"encoding/json"
6+
"runtime"
67
"strings"
78
"testing"
89

910
"github.com/sirupsen/logrus"
1011
)
1112

12-
// testSkippedPaths contains the paths we want to skip during tests
13-
// (including caller_hook.go but excluding the logger.go and test files)
13+
// testSkippedPaths contains the function name patterns we want to skip during tests
1414
var testSkippedPaths = []string{
15-
"meshkit/logger/caller_hook.go",
16-
"meshkit/logger/logger.go",
17-
"sirupsen/logrus",
15+
"github.com/meshery/meshkit/logger.(*CallerHook).Fire",
16+
"github.com/meshery/meshkit/logger.(*Logger)",
17+
"github.com/sirupsen/logrus",
1818
}
1919

2020
func TestCallerHook_WithMeshkitLogger(t *testing.T) {
@@ -245,30 +245,25 @@ func TestCallerHook_DifferentLogLevels(t *testing.T) {
245245
}
246246

247247
func TestShouldSkipFrame(t *testing.T) {
248-
hook := &CallerHook{skippedPaths: []string{"meshkit/logger", "sirupsen/logrus"}}
249-
250-
testCases := []struct {
251-
name string
252-
filepath string
253-
expected bool
254-
}{
255-
{"should skip meshkit/logger path", "/some/path/meshkit/logger/logger.go", true},
256-
{"should skip meshkit/logger path", "/some/path/meshkit/logger/caller_hook.go", true},
257-
{"should skip sirupsen/logrus path", "/some/path/sirupsen/logrus/entry.go", true},
258-
{"should skip meshkit/logger test files", "/some/path/meshkit/logger/caller_hook_test.go", true}, // This will be skipped with default paths
259-
{"should not skip regular application path", "/some/path/myapp/main.go", false},
260-
{"should not skip empty path", "", false},
261-
{"should not skip partial match", "/some/path/logger-test/file.go", false},
248+
// Use the test-specific skipped paths
249+
hook := &CallerHook{skippedPaths: testSkippedPaths}
250+
251+
// Test with the current function (this test function should not be skipped)
252+
pc, _, _, _ := runtime.Caller(0)
253+
result := hook.shouldSkipFrame(pc)
254+
255+
// This test function should NOT be skipped since it doesn't match our specific patterns
256+
if result {
257+
currentFunc := runtime.FuncForPC(pc)
258+
funcName := "unknown"
259+
if currentFunc != nil {
260+
funcName = currentFunc.Name()
261+
}
262+
t.Errorf("Test function %s should not be skipped, but shouldSkipFrame returned true", funcName)
262263
}
263264

264-
for _, tc := range testCases {
265-
t.Run(tc.name, func(t *testing.T) {
266-
result := hook.shouldSkipFrame(tc.filepath)
267-
if result != tc.expected {
268-
t.Errorf("shouldSkipFrame(%q) = %v, expected %v", tc.filepath, result, tc.expected)
269-
}
270-
})
271-
}
265+
// Test that the method works correctly
266+
t.Logf("shouldSkipFrame works correctly for test functions")
272267
}
273268

274269
func TestCallerHook_Levels(t *testing.T) {

0 commit comments

Comments
 (0)