Skip to content

Commit

Permalink
Merge pull request #4 from lithictech/v2
Browse files Browse the repository at this point in the history
Version 2 release: remove logrus and outdated modules
  • Loading branch information
rgalanakis authored Jul 31, 2024
2 parents 3a1cda5 + 23d0bb9 commit 7b9f0d7
Show file tree
Hide file tree
Showing 47 changed files with 628 additions and 1,459 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ jobs:
uses: actions/checkout@v2
with:
ref: ${{ github.head_ref }}
- name: Set up Go 1.16.x
- name: Set up Go 1.22.x
uses: actions/setup-go@v1
with:
go-version: 1.16.x
go-version: 1.22.x
- uses: actions/cache@v1
with:
path: ~/go/pkg/mod
Expand Down
4 changes: 2 additions & 2 deletions api/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package api

import (
"context"
"github.com/labstack/echo"
"github.com/lithictech/go-aperitif/logctx"
"github.com/labstack/echo/v4"
"github.com/lithictech/go-aperitif/v2/logctx"
)

// StdContext returns a standard context from an echo context.
Expand Down
31 changes: 16 additions & 15 deletions api/api.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
/*
Package api is a standalone API package/pattern built on echo and logrus.
Package api is a standalone API package/pattern built on echo.
It sets up /statusz and /healthz endpoints,
and sets up logging middleware that takes care of the following important,
and fundamentally (in Go) interconnected tasks:
- Extract (or add) a trace ID header to the request and response.
- The trace ID can be retrieved through api.TraceID(context) of the echo.Context for the request.
- Use that trace ID header as context for the logrus logger.
- Handle request logging (metadata about the request and response,
and log at the level appropriate for the status code).
- The request logger can be retrieved api.Logger(echo.Context).
- Recover from panics.
- Coerce all errors into api.Error types, and marshal them.
- Override echo's HTTPErrorHandler to pass through api.Error types.
- Extract (or add) a trace ID header to the request and response.
- The trace ID can be retrieved through api.TraceID(context) of the echo.Context for the request.
- Use that trace ID header as context for the logger.
- Handle request logging (metadata about the request and response,
and log at the level appropriate for the status code).
- The request logger can be retrieved api.Logger(echo.Context).
- Recover from panics.
- Coerce all errors into api.Error types, and marshal them.
- Override echo's HTTPErrorHandler to pass through api.Error types.
*/
package api

import (
"github.com/labstack/echo"
"github.com/labstack/echo/middleware"
"github.com/sirupsen/logrus"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
"github.com/lithictech/go-aperitif/v2/logctx"
"log/slog"
"net/http"
"os"
)

type Config struct {
// If not provided, create an echo.New.
App *echo.Echo
Logger *logrus.Entry
Logger *slog.Logger
LoggingMiddlwareConfig LoggingMiddlwareConfig
// Origins for echo's CORS middleware.
// If it and CorsConfig are empty, do not add the middleware.
Expand Down Expand Up @@ -57,7 +58,7 @@ type Config struct {

func New(cfg Config) *echo.Echo {
if cfg.Logger == nil {
cfg.Logger = unconfiguredLogger()
cfg.Logger = logctx.UnconfiguredLogger()
}
if cfg.HealthHandler == nil {
if cfg.HealthResponse == nil {
Expand Down
122 changes: 58 additions & 64 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ package api_test

import (
"errors"
"github.com/labstack/echo"
"github.com/lithictech/go-aperitif/api"
"github.com/lithictech/go-aperitif/api/apiparams"
. "github.com/lithictech/go-aperitif/api/echoapitest"
. "github.com/lithictech/go-aperitif/apitest"
"github.com/lithictech/go-aperitif/logctx"
"github.com/labstack/echo/v4"
"github.com/lithictech/go-aperitif/v2/api"
"github.com/lithictech/go-aperitif/v2/api/apiparams"
. "github.com/lithictech/go-aperitif/v2/api/echoapitest"
. "github.com/lithictech/go-aperitif/v2/apitest"
"github.com/lithictech/go-aperitif/v2/logctx"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/rgalanakis/golangal"
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -25,15 +24,14 @@ func TestAPI(t *testing.T) {

var _ = Describe("API", func() {
var e *echo.Echo
var logger *logrus.Logger
var logHook *test.Hook
var logEntry *logrus.Entry

var logger *slog.Logger
var logHook *logctx.Hook

BeforeEach(func() {
logger, logHook = test.NewNullLogger()
logEntry = logger.WithFields(nil)
logger, logHook = logctx.NewNullLogger()
e = api.New(api.Config{
Logger: logEntry,
Logger: logger,
HealthResponse: map[string]interface{}{"o": "k"},
StatusResponse: map[string]interface{}{"it": "me"},
})
Expand All @@ -48,7 +46,7 @@ var _ = Describe("API", func() {

It("can use custom health and status fields", func() {
e = api.New(api.Config{
Logger: logEntry,
Logger: logger,
HealthHandler: func(c echo.Context) error {
return c.String(200, "yo")
},
Expand Down Expand Up @@ -117,58 +115,56 @@ var _ = Describe("API", func() {
Describe("logging", func() {
It("does not corrupt the input logger (by reassigning the closure)", func() {
e.GET("/before-first-call", func(c echo.Context) error {
Expect(api.Logger(c).Data).ToNot(HaveKey("request_status"))
Expect(api.Logger(c).Handler().(*logctx.Hook).AttrMap()).ToNot(HaveKey("request_status"))
return c.String(401, "ok")
})
e.GET("/after-first-call", func(c echo.Context) error {
Expect(api.Logger(c).Data).ToNot(HaveKey("request_status"))
Expect(api.Logger(c).Handler().(*logctx.Hook).AttrMap()).ToNot(HaveKey("request_status"))
return c.String(403, "ok")
})
Expect(Serve(e, GetRequest("/before-first-call"))).To(HaveResponseCode(401))
Expect(Serve(e, GetRequest("/after-first-call"))).To(HaveResponseCode(403))
Expect(logHook.Entries).To(HaveLen(2))
Expect(logHook.Records()).To(HaveLen(2))
})
It("logs normal requests at info", func() {
e.GET("/", func(c echo.Context) error {
return c.String(200, "ok")
})
Expect(Serve(e, GetRequest("/"))).To(HaveResponseCode(200))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Level).To(Equal(logrus.InfoLevel))
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].Record.Level).To(Equal(slog.LevelInfo))
})
It("logs 500+ at error", func() {
e.GET("/", func(c echo.Context) error {
return c.String(500, "oh")
})
Expect(Serve(e, GetRequest("/"))).To(HaveResponseCode(500))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Level).To(Equal(logrus.ErrorLevel))
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].Record.Level).To(Equal(slog.LevelError))
})
It("logs 400 to 499 as warn", func() {
e.GET("/", func(c echo.Context) error {
return c.String(400, "client err")
})
Expect(Serve(e, GetRequest("/"))).To(HaveResponseCode(400))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Level).To(Equal(logrus.WarnLevel))
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].Record.Level).To(Equal(slog.LevelWarn))
})
It("logs status and health as debug", func() {
logger.SetLevel(logrus.DebugLevel)
Expect(Serve(e, GetRequest("/healthz"))).To(HaveResponseCode(200))
Expect(Serve(e, GetRequest("/statusz"))).To(HaveResponseCode(200))
Expect(logHook.Entries).To(HaveLen(2))
Expect(logHook.Entries[0].Level).To(Equal(logrus.DebugLevel))
Expect(logHook.Entries[1].Level).To(Equal(logrus.DebugLevel))
Expect(logHook.Records()).To(HaveLen(2))
Expect(logHook.Records()[0].Record.Level).To(Equal(slog.LevelDebug))
Expect(logHook.Records()[1].Record.Level).To(Equal(slog.LevelDebug))
})
It("logs options as debug", func() {
logger.SetLevel(logrus.DebugLevel)
Expect(Serve(e, NewRequest("OPTIONS", "/foo", nil))).To(HaveResponseCode(404))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Level).To(Equal(logrus.DebugLevel))
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].Record.Level).To(Equal(slog.LevelDebug))
})
It("can log request and response headers", func() {
e = api.New(api.Config{
Logger: logEntry,
Logger: logger,
LoggingMiddlwareConfig: api.LoggingMiddlwareConfig{
RequestHeaders: true,
ResponseHeaders: true,
Expand All @@ -179,24 +175,24 @@ var _ = Describe("API", func() {
return c.String(200, "ok")
})
Expect(Serve(e, GetRequest("/", SetReqHeader("ReqHead", "ReqHeadVal")))).To(HaveResponseCode(200))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Data).To(And(
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].AttrMap()).To(And(
HaveKeyWithValue("request_header.Reqhead", "ReqHeadVal"),
HaveKeyWithValue("response_header.Reshead", "ResHeadVal"),
))
})
It("can use custom DoLog, BeforeRequest, and AfterRequest hooks", func() {
doLogCalled := false
e = api.New(api.Config{
Logger: logEntry,
Logger: logger,
LoggingMiddlwareConfig: api.LoggingMiddlwareConfig{
BeforeRequest: func(_ echo.Context, e *logrus.Entry) *logrus.Entry {
return e.WithField("before", 1)
BeforeRequest: func(_ echo.Context, e *slog.Logger) *slog.Logger {
return e.With("before", 1)
},
AfterRequest: func(_ echo.Context, e *logrus.Entry) *logrus.Entry {
return e.WithField("after", 2)
AfterRequest: func(_ echo.Context, e *slog.Logger) *slog.Logger {
return e.With("after", 2)
},
DoLog: func(c echo.Context, e *logrus.Entry) {
DoLog: func(c echo.Context, e *slog.Logger) {
doLogCalled = true
api.LoggingMiddlewareDefaultDoLog(c, e)
},
Expand All @@ -207,9 +203,9 @@ var _ = Describe("API", func() {
})
Expect(Serve(e, GetRequest("/"))).To(HaveResponseCode(400))
Expect(doLogCalled).To(BeTrue())
Expect(logHook.Entries[len(logHook.Entries)-1].Data).To(And(
HaveKeyWithValue("before", 1),
HaveKeyWithValue("after", 2),
Expect(logHook.LastRecord().AttrMap()).To(And(
HaveKeyWithValue("before", BeEquivalentTo(1)),
HaveKeyWithValue("after", BeEquivalentTo(2)),
))
})
})
Expand Down Expand Up @@ -284,16 +280,17 @@ var _ = Describe("API", func() {
r, err := http.NewRequest("GET", "", nil)
Expect(err).ToNot(HaveOccurred())
ctx := e.NewContext(r, httptest.NewRecorder())
logger := logrus.New().WithField("a", 2)
logger, _ := logctx.NewNullLogger()
logger = logger.With("a", 2)
api.SetLogger(ctx, logger)
tid := api.TraceId(ctx)

c := api.StdContext(ctx)
tkey, tval := logctx.ActiveTraceId(c)
Expect(tkey).To(Equal(logctx.RequestTraceIdKey))
Expect(tval).To(Equal(tid))
Expect(logctx.Logger(c).Data).To(And(
HaveKeyWithValue("a", 2),
Expect(logctx.Logger(c).Handler().(*logctx.Hook).AttrMap()).To(And(
HaveKeyWithValue("a", BeEquivalentTo(2)),
HaveKeyWithValue(BeEquivalentTo(logctx.RequestTraceIdKey), tid),
))
})
Expand All @@ -320,27 +317,24 @@ var _ = Describe("API", func() {
})

Describe("DebugMiddleware", func() {
BeforeEach(func() {
logger.SetLevel(logrus.DebugLevel)
})
It("noops if not enabled", func() {
e.Use(api.DebugMiddleware(api.DebugMiddlewareConfig{Enabled: false, DumpResponseBody: true}))
e.GET("/foo", func(c echo.Context) error {
return c.String(200, "ok")
})
Serve(e, NewRequest("POST", "/endpoint", nil))
Expect(logHook.Entries).To(HaveLen(1))
Expect(logHook.Entries[0].Message).To(Equal("request_finished"))
Expect(logHook.Records()).To(HaveLen(1))
Expect(logHook.Records()[0].Record.Message).To(Equal("request_finished"))
})
It("dumps what is enabled", func() {
e.Use(api.DebugMiddleware(api.DebugMiddlewareConfig{Enabled: true, DumpResponseBody: true, DumpResponseHeaders: true}))
e.GET("/endpoint", func(c echo.Context) error {
return c.String(200, "ok")
})
Serve(e, NewRequest("GET", "/endpoint", nil))
Expect(logHook.Entries).To(HaveLen(2))
Expect(logHook.Entries[0].Message).To(Equal("request_debug"))
Expect(logHook.Entries[0].Data).To(And(
Expect(logHook.Records()).To(HaveLen(2))
Expect(logHook.Records()[0].Record.Message).To(Equal("request_debug"))
Expect(logHook.Records()[0].AttrMap()).To(And(
HaveKeyWithValue("debug_response_headers", HaveKey("Content-Type")),
HaveKeyWithValue("debug_response_body", ContainSubstring("ok")),
))
Expand All @@ -351,9 +345,9 @@ var _ = Describe("API", func() {
return c.String(200, "ok")
})
Serve(e, NewRequest("GET", "/endpoint", nil, SetReqHeader("Foo", "x")))
Expect(logHook.Entries).To(HaveLen(2))
Expect(logHook.Entries[0].Message).To(Equal("request_debug"))
Expect(logHook.Entries[0].Data).To(And(
Expect(logHook.Records()).To(HaveLen(2))
Expect(logHook.Records()[0].Record.Message).To(Equal("request_debug"))
Expect(logHook.Records()[0].AttrMap()).To(And(
HaveKeyWithValue("debug_request_headers", HaveKey("Foo")),
HaveKeyWithValue("debug_response_headers", HaveKey("Content-Type")),
HaveKeyWithValue("debug_request_body", ""),
Expand All @@ -367,13 +361,13 @@ var _ = Describe("API", func() {
})
Serve(e, NewRequest("GET", "/endpoint", nil, SetReqHeader("Foo", "x")))
Serve(e, NewRequest("GET", "/endpoint", nil, SetReqHeader("Foo", "x")))
Expect(logHook.Entries).To(HaveLen(4))
Expect(logHook.Entries[0].Message).To(Equal("request_debug"))
Expect(logHook.Entries[0].Data).ToNot(HaveKey("memory_sys"))
Expect(logHook.Entries[1].Message).To(Equal("request_finished"))
Expect(logHook.Entries[2].Message).To(Equal("request_debug"))
Expect(logHook.Entries[2].Data).To(HaveKey("memory_sys"))
Expect(logHook.Entries[3].Message).To(Equal("request_finished"))
Expect(logHook.Records()).To(HaveLen(4))
Expect(logHook.Records()[0].Record.Message).To(Equal("request_debug"))
Expect(logHook.Records()[0].AttrMap()).ToNot(HaveKey("memory_sys"))
Expect(logHook.Records()[1].Record.Message).To(Equal("request_finished"))
Expect(logHook.Records()[2].Record.Message).To(Equal("request_debug"))
Expect(logHook.Records()[2].AttrMap()).To(HaveKey("memory_sys"))
Expect(logHook.Records()[3].Record.Message).To(Equal("request_finished"))
})
})
})
2 changes: 1 addition & 1 deletion api/apiparams/apiparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package apiparams

import (
"fmt"
"github.com/lithictech/go-aperitif/validator"
"github.com/lithictech/go-aperitif/v2/validator"
"net/http"
"reflect"
"time"
Expand Down
8 changes: 4 additions & 4 deletions api/apiparams/apiparams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package apiparams_test

import (
"fmt"
"github.com/labstack/echo"
"github.com/lithictech/go-aperitif/api/apiparams"
. "github.com/lithictech/go-aperitif/api/echoapitest"
. "github.com/lithictech/go-aperitif/apitest"
"github.com/labstack/echo/v4"
"github.com/lithictech/go-aperitif/v2/api/apiparams"
. "github.com/lithictech/go-aperitif/v2/api/echoapitest"
. "github.com/lithictech/go-aperitif/v2/apitest"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/rgalanakis/golangal"
Expand Down
4 changes: 2 additions & 2 deletions api/apiparams/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package apiparams_test

import (
"encoding/json"
"github.com/lithictech/go-aperitif/api/apiparams"
"github.com/lithictech/go-aperitif/convext"
"github.com/lithictech/go-aperitif/v2/api/apiparams"
"github.com/lithictech/go-aperitif/v2/convext"
"net/http"
"strings"
"testing"
Expand Down
Loading

0 comments on commit 7b9f0d7

Please sign in to comment.