Skip to content

Commit

Permalink
Moved to zerolog (#9)
Browse files Browse the repository at this point in the history
- Key changes:
  - Added some tests;
  - Moved to Go 1.18, Alpine 3.15.1;
  - Moved to zerolog:
    - Pretty formatting by default, JSON is also an option (env: `LOG_FORMAT`: `pretty`, `json`);
    - Optional access log (env `LOG_REQUESTS`: `true`);
    - NOTE: Logging format is subject to change.
  • Loading branch information
weisdd authored Mar 21, 2022
1 parent ba75d2f commit 995ca01
Show file tree
Hide file tree
Showing 19 changed files with 525 additions and 106 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:

- uses: actions/setup-go@v2
with:
go-version: "1.17"
go-version: "1.18"

- name: golangci-lint
uses: golangci/golangci-lint-action@v3
Expand Down
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ linters:

linters-settings:
gosimple:
go: "1.17"
go: "1.18"
gocyclo:
min-complexity: 10
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# CHANGELOG

## 0.7.0

- Key changes:
- Added some tests;
- Moved to Go 1.18, Alpine 3.15.1;
- Moved to zerolog:
- Pretty formatting by default, JSON is also an option (env: `LOG_FORMAT`: `pretty`, `json`);
- Optional access log (env `LOG_REQUESTS`: `true`);
- NOTE: Logging format is subject to change.

## 0.6.0

- Key changes:
Expand Down
7 changes: 2 additions & 5 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
ARG ALPINE_VERSION=3.15.0
ARG GOLANG_VERSION=1.17.7-alpine3.15

FROM golang:${GOLANG_VERSION} as builder
FROM golang:1.18.0-alpine3.15 as builder

ARG VERSION

Expand All @@ -22,7 +19,7 @@ RUN go install \
" \
./...

FROM alpine:${ALPINE_VERSION} as runtime
FROM alpine:3.15.1 as runtime

RUN set -x \
&& apk add --update --no-cache ca-certificates tzdata \
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ OIDC roles are expected to be present in `roles` within a jwt token.
| | | | |
| **Logging** | | | |
| | `DEBUG` | `false` | Whether to print out debug log messages. |
| | `LOG_FORMAT` | `pretty` | Log format (`pretty`, `json`) |
| | `LOG_NO_COLOR` | `false` | Whether to disable colors for `pretty` format |
| | `LOG_REQUESTS` | `false` | Whether to log HTTP requests |
| | | | |
| **HTTP Server** | | | |
| | `PORT` | `8080` | Port the web server will listen on. |
Expand Down
5 changes: 3 additions & 2 deletions cmd/lfgw/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (app *application) loadACL() (ACLMap, error) {

err = yaml.Unmarshal(yamlFile, &aclYaml)
if err != nil {
app.errorLog.Fatal(err)
return aclMap, err
}

for role, ns := range aclYaml {
Expand All @@ -121,7 +121,8 @@ func (app *application) loadACL() (ACLMap, error) {
acl.LabelFilter = lf
acl.RawACL = ns
aclMap[role] = acl
app.infoLog.Printf("Loaded role definition for %s: %q (converted to %s)", role, ns, acl.LabelFilter.AppendString(nil))
app.logger.Info().Caller().
Msgf("Loaded role definition for %s: %q (converted to %s)", role, ns, acl.LabelFilter.AppendString(nil))
}

return aclMap, nil
Expand Down
38 changes: 1 addition & 37 deletions cmd/lfgw/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestACL_ToSlice(t *testing.T) {
got, err := acl.toSlice(tt.ns)
if tt.fail {
if err == nil {
t.Errorf("Expected a non-nil error, though got %s", err)
t.Error("Expected a non-nil error, though got a nil one")
}
} else {
if reflect.DeepEqual(got, tt.want) {
Expand Down Expand Up @@ -127,39 +127,3 @@ func TestACL_PrepareLF(t *testing.T) {
}

// TODO: test loadACL
// // PrepareLF Returns a label filter based on rule definitions (non-regexp for one namespace, regexp - for many)
// func (a ACL) PrepareLF(ns string) (metricsql.LabelFilter, error) {
// var lf = metricsql.LabelFilter{
// Label: "namespace",
// IsNegative: false,
// }

// if ns == ".*" {
// lf.Value = ns
// lf.IsRegexp = true
// }

// buffer, err := a.toSlice(ns)
// if err != nil {
// return metricsql.LabelFilter{}, err
// }

// if len(buffer) == 1 {
// lf.Value = buffer[0]
// if strings.ContainsAny(buffer[0], `.+*?^$()[]{}|\`) {
// lf.IsRegexp = true
// }
// } else {
// lf.Value = fmt.Sprintf("^(%s)$", strings.Join(buffer, "|"))
// lf.IsRegexp = true
// }

// if lf.IsRegexp {
// _, err := regexp.Compile(lf.Value)
// if err != nil {
// return metricsql.LabelFilter{}, fmt.Errorf("%s in %q (converted from %q)", err, lf.Value, ns)
// }
// }

// return lf, nil
// }
54 changes: 52 additions & 2 deletions cmd/lfgw/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ package main
import (
"fmt"
"net/http"
"strconv"
"strings"

"github.com/rs/zerolog"
"github.com/rs/zerolog/hlog"
)

// serverError sends a generic 500 Internal Server Error response to the user.
func (app *application) serverError(w http.ResponseWriter, err error) {
app.errorLog.Println(err)
func (app *application) serverError(w http.ResponseWriter, r *http.Request, err error) {
hlog.FromRequest(r).Error().Caller(1).
Err(err).Msg("")
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
}

Expand Down Expand Up @@ -41,3 +46,48 @@ func (app *application) getRawAccessToken(r *http.Request) (string, error) {

return "", fmt.Errorf("no bearer token found")
}

// lshortfile implements Lshortfile equivalent for zerolog's CallerMarshalFunc
func (app *application) lshortfile(file string, line int) string {
// Copied from the standard library: https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/log/log.go;drc=926994fd7cf65b2703552686965fb05569699897;l=134
short := file
for i := len(file) - 1; i > 0; i-- {
if file[i] == '/' {
short = file[i+1:]
break
}
}
file = short
return file + ":" + strconv.Itoa(line)
}

// enrichLogContext adds a custom field and a value to zerolog context
func (app *application) enrichLogContext(r *http.Request, field string, value string) {
if field != "" && value != "" {
log := zerolog.Ctx(r.Context())
log.UpdateContext(func(c zerolog.Context) zerolog.Context {
return c.Str(field, value)
})
}
}

// enrichDebugLogContext adds a custom field and a value to zerolog context if logging level is set to Debug
func (app *application) enrichDebugLogContext(r *http.Request, field string, value string) {
if app.Debug {
if field != "" && value != "" {
log := zerolog.Ctx(r.Context())
log.UpdateContext(func(c zerolog.Context) zerolog.Context {
return c.Str(field, value)
})
}
}
}

func (app *application) isNotAPIRequest(path string) bool {
return !strings.Contains(path, "/api/") && !strings.Contains(path, "/federate")
}

func (app *application) isUnsafePath(path string) bool {
// TODO: move to regexp?
return strings.Contains(path, "/admin/tsdb") || strings.Contains(path, "/api/v1/write")
}
151 changes: 151 additions & 0 deletions cmd/lfgw/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package main

import (
"fmt"
"net/http"
"testing"

"github.com/rs/zerolog"
)

func TestGetRawAccessToken(t *testing.T) {
logger := zerolog.New(nil)
app := &application{
logger: &logger,
}

tests := []struct {
name string
header string
fail bool
want string
}{
{
name: "X-Forwarded-Access-Token",
header: "X-Forwarded-Access-Token",
fail: false,
want: "FAKE_TOKEN",
},
{
name: "X-Auth-Request-Access-Token",
header: "X-Auth-Request-Access-Token",
fail: false,
want: "FAKE_TOKEN",
},
{
name: "Authorization",
header: "Authorization",
fail: false,
want: "FAKE_TOKEN",
},
{
name: "Random header",
header: "Random-Header",
fail: true,
want: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r, err := http.NewRequest(http.MethodGet, "/", nil)
if err != nil {
t.Fatal(err)
}

switch tt.header {
case "Authorization":
r.Header.Set(tt.header, fmt.Sprintf("Bearer %s", tt.want))
default:
r.Header.Set(tt.header, tt.want)
}

got, err := app.getRawAccessToken(r)
if tt.fail {
if err == nil {
t.Error("Expected a non-nil error, though got a nil one")
}
} else {
if got != tt.want {
t.Errorf("want %s; got %s", tt.want, got)
}
}
})
}
}

func TestIsUnsafePath(t *testing.T) {
logger := zerolog.New(nil)
app := &application{
logger: &logger,
}

tests := []struct {
name string
path string
want bool
}{
{
name: "tsdb",
path: "/admin/tsdb/1",
want: true,
},
{
name: "write",
path: "/api/v1/write",
want: true,
},
{
name: "random endpoint",
path: "/api/v1/random",
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := app.isUnsafePath(tt.path)
if got != tt.want {
t.Errorf("want %t; got %t", tt.want, got)
}
})
}
}

func TestIsNotAPIRequest(t *testing.T) {
logger := zerolog.New(nil)
app := &application{
logger: &logger,
}

tests := []struct {
name string
path string
want bool
}{
{
name: "api",
path: "/api/v1/query",
want: false,
},
{
name: "federate",
path: "/federate",
want: false,
},
{
name: "random endpoint",
path: "/metrics",
want: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := app.isNotAPIRequest(tt.path)
if got != tt.want {
t.Errorf("want %t; got %t", tt.want, got)
}
})
}
}
12 changes: 8 additions & 4 deletions cmd/lfgw/lf.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ func (app *application) modifyMetricExpr(expr metricsql.Expr, newFilter metricsq
// Update label filters
metricsql.VisitAll(newExpr, modifyLabelFilter)

app.debugLog.Printf("Rewrote query %s to query %s", expr.AppendString(nil), newExpr.AppendString(nil))
// TODO: log somehow?
// app.logger.Debug().Caller().
// Msgf("Rewrote query %s to query %s", expr.AppendString(nil), newExpr.AppendString(nil))

return newExpr
}
Expand All @@ -79,9 +81,11 @@ func (app *application) modifyMetricExpr(expr metricsql.Expr, newFilter metricsq
func (app *application) optimizeMetricExpr(expr metricsql.Expr) metricsql.Expr {
newExpr := metricsql.Optimize(expr)

if !app.equalExpr(expr, newExpr) {
app.debugLog.Printf("Optimized query %s to query %s", expr.AppendString(nil), newExpr.AppendString(nil))
}
// TODO: log somehow?
// if !app.equalExpr(expr, newExpr) {
// app.logger.Debug().Caller().
// Msgf("Optimized query %s to query %s", expr.AppendString(nil), newExpr.AppendString(nil))
// }

return newExpr
}
Expand Down
8 changes: 3 additions & 5 deletions cmd/lfgw/lf_test.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
package main

import (
"io"
"log"
"testing"

"github.com/VictoriaMetrics/metricsql"
"github.com/rs/zerolog"
)

func TestApplication_modifyMetricExpr(t *testing.T) {
logger := zerolog.New(nil)
app := &application{
errorLog: log.New(io.Discard, "", 0),
infoLog: log.New(io.Discard, "", 0),
debugLog: log.New(io.Discard, "", 0),
logger: &logger,
}

newFilterPlain := metricsql.LabelFilter{
Expand Down
Loading

0 comments on commit 995ca01

Please sign in to comment.