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

unable to skip log based on the response status #2649

Open
3 tasks done
shortcuts opened this issue Jun 21, 2024 · 2 comments
Open
3 tasks done

unable to skip log based on the response status #2649

shortcuts opened this issue Jun 21, 2024 · 2 comments

Comments

@shortcuts
Copy link

shortcuts commented Jun 21, 2024

Issue Description

I've tried to skip logs for 404 and 429 but was unable to by using a simple Skipper because it is evaluated before next has been called and therefore the status is always 200.

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

Being able to skip based on the status

Actual behaviour

Skipper is called before the status has been set

Steps to reproduce

  1. Trigger a 404
  2. See the log being outputted
  3. Add a print in the middleware, see status 200 on 404 requests

Working code to debug

package main

func main() {
	e := echo.New()

	e.Use(
		middleware.LoggerWithConfig(middleware.LoggerConfig{
			Skipper: func(c echo.Context) bool {
				return c.Response().Status == http.StatusNotFound || c.Response().Status == http.StatusTooManyRequests
			},
		}),
	)

	e.Start(":1313")

}

See https://github.com/shortcuts/codes/pull/9/files also if you want to see the full file

Version/commit

Latest

@aldas
Copy link
Contributor

aldas commented Jun 22, 2024

This is working as intended. Skipper is called always before next middleware/handler is called. At this time there is no response (code) yet.

You can use RequestLogger middleware to ignore some log lines. See

	logger := slog.New(slog.NewJSONHandler(os.Stdout, nil))
	e.Use(middleware.RequestLoggerWithConfig(middleware.RequestLoggerConfig{
		LogStatus:   true,
		LogURI:      true,
		LogError:    true,
		HandleError: true, // forwards error to the global error handler, so it can decide appropriate status code
		LogValuesFunc: func(c echo.Context, v middleware.RequestLoggerValues) error {
			if v.Status == http.StatusNotFound || v.Status == http.StatusTooManyRequests {
				return nil
			}
			
			if v.Error == nil {
				logger.LogAttrs(context.Background(), slog.LevelInfo, "REQUEST",
					slog.String("uri", v.URI),
					slog.Int("status", v.Status),
				)
			} else {
				logger.LogAttrs(context.Background(), slog.LevelError, "REQUEST_ERROR",
					slog.String("uri", v.URI),
					slog.Int("status", v.Status),
					slog.String("err", v.Error.Error()),
				)
			}
			return nil
		},
	}))

@shortcuts
Copy link
Author

Thanks for the fast answer @aldas !

Naively I thought that the skipper (in the log context) scope was to skip logs for any situation and the request logger reserved to 3rd party integrations and/or customization.

I think we can close this issue then, I'll go with your suggested solution, thanks!

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

No branches or pull requests

2 participants