Skip to content
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

Add complete error path in log #74

Closed
wants to merge 2 commits into from

Conversation

AKARSHITJOSHI
Copy link

Fixes #65

@akajla09
Copy link
Member

Hi @AKARSHITJOSHI , thanks for submitting a PR! I tried out your changes locally and have a couple of comments:

  • Although this change does indeed add the filename to each log line, it's not that helpful since in most cases the top level caller (likely the router) is logging the errors. So most of the log statements output router.go. For example:
3:26PM INF ../../pkg/service/router.go:133 > ACCESS clientIp=[ duration=0.539666 method=PUT protocol=HTTP/1.1 requestId=cgrj082401g2lho1246g size=119 statusCode=200 uri=/v1/users/0943a997-6609-4d09-9966-894b229b55de userAgent=Go-http-client/1.1
3:26PM ERR ../../pkg/service/router.go:53 > ERROR error="DuplicateRecordError: Duplicate User 5555555555555, A user with the given userId already exists" apiError=DuplicateRecordError requestId=cgrj082401g2lho12470 statusCode=400 uri=/v1/users
3:26PM INF ../../pkg/service/router.go:133 > ACCESS clientIp=[ duration=0.1655 method=POST protocol=HTTP/1.1 requestId=cgrj082401g2lho12470 size=150 statusCode=400 uri=/v1/users userAgent=Go-http-client/1.1
3:26PM INF ../../pkg/service/router.go:133 > ACCESS clientIp=[ duration=0.471625 method=DELETE protocol=HTTP/1.1 requestId=cgrj082401g2lho1247g size=0 statusCode=200 uri=/v1/users/4444444444444 userAgent=Go-http-client/1.1
  • What would instead be great is if the stacktraces generated by any wrapped errors (those created via errors.Wrap()) contain the absolute path (the '/abs/path' in the source fields in this example):

3:34PM ERR ERROR error="error from repo: new error" requestId=cgrj3uq401g33dfv6rp0 stack=[{"func":"SQLiteRepository.GetByRoleId","line":"94","source":"/abs/path/sqlite.go"},{"func":"RoleService.UpdateByRoleId","line":"104","source":"/abs/path/service.go"},{"func":"UpdateHandler","line":"114","source":"/abs/path/handlers.go"},{"func":"RouteHandler[...].ServeHTTP","line":"39","source":"/abs/path/router.go"},{"func":"URLHandler.func1.1","line":"43","source":"/abs/path/hlog.go"},{"func":"HandlerFunc.ServeHTTP","line":"2109","source":"/abs/path/server.go"},{"func":"RequestIDHandler.func1.1","line":"187","source":"/abs/path/hlog.go"},{"func":"HandlerFunc.ServeHTTP","line":"2109","source":"/abs/path/server.go"},{"func":"AccessHandler.func1.1","line":"214","source":"/abs/path/hlog.go"},{"func":"HandlerFunc.ServeHTTP","line":"2109","source":"/abs/path/server.go"},{"func":"NewHandler.func1.1","line":"29","source":"/abs/path/hlog.go"},{"func":"HandlerFunc.ServeHTTP","line":"2109","source":"/abs/path/server.go"},{"func":"(*Router).ServeHTTP","line":"210","source":"/abs/path/mux.go"},{"func":"serverHandler.ServeHTTP","line":"2947","source":"/abs/path/server.go"},{"func":"(*conn).serve","line":"1991","source":"/abs/path/server.go"},{"func":"goexit","line":"1594","source":"asm_amd64.s"}] uri=/v1/roles/test-admin

This logic is handled by zerolog's pkgerrors.MarshalStack which we configure here.

I haven't looked into it in more detail but if there's a way to get the full filename, we'd likely have to implement our own marshaler and add it there. Feel free to take a look into this if you'd like but I'm not sure if it's possible.. in which case we might just want to hold off on this.

@akajla09
Copy link
Member

@AKARSHITJOSHI not sure if you're still looking into this but I'm going to close this PR given the convo above. If you get a chance to do something with MarshalStack, feel free to create a new PR (but like I said, not urgent). Thanks!

@akajla09 akajla09 closed this Apr 21, 2023
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.

Add full file path to error logs that contain a stack
2 participants