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 correlation ids #20

Merged
merged 19 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions api/router/correlation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package router_test

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

"github.com/allinbits/demeris-api-server/api/config"
"github.com/allinbits/demeris-api-server/api/database"
"github.com/allinbits/demeris-api-server/api/router"
"github.com/allinbits/emeris-utils/logging"
"github.com/allinbits/emeris-utils/store"
"github.com/cockroachdb/cockroach-go/v2/testserver"
"github.com/gofrs/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
)

type chainsResponse struct {
0xmuralik marked this conversation as resolved.
Show resolved Hide resolved
Chains []supportedChain `json:"chains"`
}
0xmuralik marked this conversation as resolved.
Show resolved Hide resolved

type supportedChain struct {
0xmuralik marked this conversation as resolved.
Show resolved Hide resolved
ChainName string `json:"chain_name"`
DisplayName string `json:"display_name"`
Logo string `json:"logo"`
}

func TestCorrelationIDMiddleWare(t *testing.T) {
0xmuralik marked this conversation as resolved.
Show resolved Hide resolved
r, cfg, observedLogs, tDown := setup(t)
defer tDown()
require.NotNil(t, r)

go r.Serve(cfg.ListenAddr)
time.Sleep(2 * time.Second)

client := http.Client{
Timeout: 2 * time.Second,
}
req, err := http.NewRequest("GET", fmt.Sprintf("http://%s%s", cfg.ListenAddr, "/chains"), nil)
require.NoError(t, err)

id, _ := uuid.NewV4()
0xmuralik marked this conversation as resolved.
Show resolved Hide resolved

req.Header.Set("X-Correlation-id", fmt.Sprintf("%x", id))

_, err = client.Do(req)
require.NoError(t, err)

require.Eventually(t, func() bool {
count := 0
for _, info := range observedLogs.All() {
if info.ContextMap()[string(logging.IntCorrelationIDName)] != nil {
count++
}
if info.ContextMap()[string(logging.CorrelationIDName)] == fmt.Sprintf("%x", id) {
count++
}
}
return count == 2
}, 5*time.Second, 1*time.Second)
}

func setup(t *testing.T) (router.Router, config.Config, *observer.ObservedLogs, func()) {
tServer, err := testserver.NewTestServer()
require.NoError(t, err)

require.NoError(t, tServer.WaitForInit())

connStr := tServer.PGURL().String()
require.NotNil(t, connStr)

cfg := &config.Config{
DatabaseConnectionURL: connStr,
ListenAddr: "127.0.0.1:9090",
RedisAddr: "127.0.0.1:6379",
KubernetesNamespace: "emeris",
Debug: true,
}

db, err := database.Init(cfg)
require.NoError(t, err)

s, err := store.NewClient(cfg.RedisAddr)
require.NoError(t, err)

observedZapCore, observedLogs := observer.New(zap.InfoLevel)
observedLogger := zap.New(observedZapCore)

return *router.New(db, observedLogger.Sugar(), s, nil, "", nil, cfg.Debug), *cfg, observedLogs, func() { tServer.Stop() }
}
9 changes: 9 additions & 0 deletions api/router/deps/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
kube "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/allinbits/demeris-api-server/api/database"
"github.com/allinbits/emeris-utils/logging"
"github.com/allinbits/emeris-utils/store"
"github.com/gin-gonic/gin"
"go.uber.org/zap"
Expand Down Expand Up @@ -38,9 +39,17 @@ func GetDeps(c *gin.Context) *Deps {

// WriteError lgos and return client-facing errors
func (d *Deps) WriteError(c *gin.Context, err Error, logMessage string, keyAndValues ...interface{}) {

// setting error id
value, ok := c.Request.Context().Value(logging.IntCorrelationIDName).(string)
if !ok {
panic("cant get value int_correlation_id")
}
err.ID = value
_ = c.Error(err)

if keyAndValues != nil {
keyAndValues = append(keyAndValues, "error", err)
d.Logger.Errorw(
logMessage,
keyAndValues...,
Expand Down
21 changes: 0 additions & 21 deletions api/router/deps/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,8 @@ package deps

import (
"fmt"
"strconv"
"sync"

"github.com/sony/sonyflake"
)

var flake *sonyflake.Sonyflake
var once sync.Once

func init() {
once.Do(func() {
flake = sonyflake.NewSonyflake(sonyflake.Settings{})
})
}

type Error struct {
ID string `json:"id"`
Namespace string `json:"namespace"`
Expand All @@ -34,15 +21,7 @@ func (e Error) Unwrap() error {
}

func NewError(namespace string, cause error, statusCode int) Error {
id, err := flake.NextID()
if err != nil {
panic(fmt.Errorf("cannot create sonyflake, %w", err))
}

idstr := strconv.FormatUint(id, 10)

return Error{
ID: idstr,
StatusCode: statusCode,
Namespace: namespace,
LowLevelError: cause,
Expand Down
14 changes: 12 additions & 2 deletions api/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (
"github.com/allinbits/demeris-api-server/api/block"
"github.com/allinbits/demeris-api-server/api/cached"
"github.com/allinbits/demeris-api-server/api/liquidity"
"github.com/allinbits/emeris-utils/logging"
"k8s.io/client-go/informers"

"github.com/allinbits/demeris-api-server/api/relayer"

"github.com/allinbits/emeris-utils/logging"
"github.com/allinbits/emeris-utils/validation"
"github.com/gin-gonic/gin/binding"

Expand Down Expand Up @@ -57,6 +57,7 @@ func New(

engine := gin.New()

engine.Use(logging.CorrelationIDMiddleware(l))
r := &Router{
g: engine,
DB: db,
Expand All @@ -71,10 +72,11 @@ func New(

validation.JSONFields(binding.Validator)

engine.Use(r.catchPanicsFunc)
if debug {
engine.Use(logging.LogRequest(l.Desugar()))
}
engine.Use(r.setLoggerFromContext)
engine.Use(r.catchPanicsFunc)
engine.Use(r.decorateCtxWithDeps)
engine.Use(r.handleErrors)
engine.RedirectTrailingSlash = false
Expand All @@ -89,6 +91,14 @@ func (r *Router) Serve(address string) error {
return r.g.Run(address)
}

func (r *Router) setLoggerFromContext(c *gin.Context) {
l, err := logging.GetLoggerFromContext(c)
if err != nil && r.l == nil {
panic("cant get logger from context")
}
r.l = l
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this safe to do? When multiple API calls arrive aren't them executed concurrently and sharing the same router object?

even it this was safe looks like an anti-pattern to me: request-related stuff should stay inside request's context. What's this r.l used for anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set a modified logger to the context in the CorrelationIDMiddleware function. Intention is to use this logger everywhere. As the api server uses the logger from router, I'm changing the router logger (r.l) to use the logger from context.

Copy link
Contributor

@Pitasi Pitasi Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set a modified logger to the context in the CorrelationIDMiddleware function. Intention is to use this logger everywhere.

that's ok

As the api server uses the logger from router, I'm changing the router logger (r.l) to use the logger from context.

I don't get why we need a router logger (+ I'm still concerned about concurrency safety)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

func (r *Router) catchPanicsFunc(c *gin.Context) {
defer func() {
if rval := recover(); rval != nil {
Expand Down
55 changes: 25 additions & 30 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ require (
github.com/alicebob/miniredis/v2 v2.18.0
github.com/allinbits/demeris-backend-models v1.0.0
github.com/allinbits/emeris-cns-server v0.0.0-20211201093144-fa626186dded
github.com/allinbits/emeris-utils v0.1.0
github.com/allinbits/emeris-utils v1.0.1
github.com/allinbits/sdk-service-meta v0.0.0-20220114142340-a44e073d0b27
github.com/allinbits/starport-operator v0.0.1-alpha.45
github.com/cockroachdb/cockroach-go/v2 v2.2.8
github.com/cosmos/cosmos-sdk v0.42.8
github.com/gin-gonic/gin v1.7.7
github.com/go-playground/validator/v10 v10.10.0
github.com/gofrs/uuid v4.2.0+incompatible
github.com/google/go-cmp v0.5.7
github.com/gravity-devs/liquidity v1.2.9
github.com/jmoiron/sqlx v1.3.3
github.com/lib/pq v1.10.4
github.com/sony/sonyflake v1.0.0
github.com/stretchr/testify v1.7.1-0.20210427113832-6241f9ab9942
github.com/swaggo/swag v1.7.9
github.com/tendermint/tendermint v0.34.11
Expand All @@ -47,12 +47,12 @@ require (
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
github.com/Workiva/go-datastructures v1.0.52 // indirect
github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect
github.com/armon/go-metrics v0.3.8 // indirect
github.com/armon/go-metrics v0.3.10 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/btcsuite/btcd v0.21.0-beta // indirect
github.com/cespare/xxhash v1.1.0 // indirect
github.com/cespare/xxhash/v2 v2.1.1 // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/confio/ics23/go v0.6.6 // indirect
github.com/cosmos/gaia/v5 v5.0.4 // indirect
github.com/cosmos/go-bip39 v1.0.0 // indirect
Expand All @@ -72,7 +72,7 @@ require (
github.com/ethereum/go-ethereum v1.10.15 // indirect
github.com/evanphx/json-patch v4.11.0+incompatible // indirect
github.com/felixge/httpsnoop v1.0.1 // indirect
github.com/fsnotify/fsnotify v1.4.9 // indirect
github.com/fsnotify/fsnotify v1.5.1 // indirect
github.com/ghodss/yaml v1.0.0 // indirect
github.com/gin-contrib/sse v0.1.0 // indirect
github.com/go-kit/kit v0.10.0 // indirect
Expand All @@ -84,13 +84,12 @@ require (
github.com/go-openapi/swag v0.19.15 // indirect
github.com/go-playground/locales v0.14.0 // indirect
github.com/go-playground/universal-translator v0.18.0 // indirect
github.com/go-redis/redis/v8 v8.8.3 // indirect
github.com/go-redis/redis/v8 v8.11.4 // indirect
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
github.com/gofrs/flock v0.8.1 // indirect
github.com/gofrs/uuid v4.0.0+incompatible // indirect
github.com/gogo/gateway v1.1.0 // indirect
github.com/gogo/protobuf v1.3.3 // indirect
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/btree v1.0.0 // indirect
Expand All @@ -107,40 +106,40 @@ require (
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
github.com/gtank/merlin v0.1.1 // indirect
github.com/gtank/ristretto255 v0.1.2 // indirect
github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/iamolegga/enviper v1.2.1 // indirect
github.com/iamolegga/enviper v1.4.0 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/jackc/chunkreader/v2 v2.0.1 // indirect
github.com/jackc/pgconn v1.8.0 // indirect
github.com/jackc/pgconn v1.11.0 // indirect
github.com/jackc/pgio v1.0.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgproto3/v2 v2.0.6 // indirect
github.com/jackc/pgproto3/v2 v2.2.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b // indirect
github.com/jackc/pgtype v1.6.2 // indirect
github.com/jackc/pgx/v4 v4.10.1 // indirect
github.com/jackc/pgtype v1.10.0 // indirect
github.com/jackc/pgx/v4 v4.15.0 // indirect
github.com/jmhodges/levigo v1.0.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.11 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/keybase/go-keychain v0.0.0-20190712205309-48d3d31d256d // indirect
github.com/leodido/go-urn v1.2.1 // indirect
github.com/libp2p/go-buffer-pool v0.0.2 // indirect
github.com/magefile/mage v1.11.0 // indirect
github.com/magiconair/properties v1.8.5 // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/mattn/go-isatty v0.0.12 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643 // indirect
github.com/minio/highwayhash v1.0.1 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.4.1 // indirect
github.com/mitchellh/mapstructure v1.4.3 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.1 // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/mtibben/percent v0.2.1 // indirect
github.com/natefinch/lumberjack v2.0.0+incompatible // indirect
github.com/pelletier/go-toml v1.9.3 // indirect
github.com/pelletier/go-toml v1.9.4 // indirect
github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand All @@ -157,12 +156,12 @@ require (
github.com/sasha-s/go-deadlock v0.2.1-0.20190427202633-1595213edefa // indirect
github.com/shurcooL/sanitized_anchor_name v1.0.0 // indirect
github.com/sirupsen/logrus v1.8.0 // indirect
github.com/spf13/afero v1.5.1 // indirect
github.com/spf13/cast v1.3.1 // indirect
github.com/spf13/afero v1.6.0 // indirect
github.com/spf13/cast v1.4.1 // indirect
github.com/spf13/cobra v1.1.3 // indirect
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/spf13/viper v1.7.1 // indirect
github.com/spf13/viper v1.10.1 // indirect
github.com/stretchr/objx v0.3.0 // indirect
github.com/subosito/gotenv v1.2.0 // indirect
github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 // indirect
Expand All @@ -178,28 +177,24 @@ require (
github.com/yuin/gopher-lua v0.0.0-20200816102855-ee81675732da // indirect
github.com/zondax/hid v0.9.0 // indirect
go.etcd.io/bbolt v1.3.5 // indirect
go.opentelemetry.io/otel v0.20.0 // indirect
go.opentelemetry.io/otel/metric v0.20.0 // indirect
go.opentelemetry.io/otel/trace v0.20.0 // indirect
go.uber.org/atomic v1.7.0 // indirect
go.uber.org/multierr v1.7.0 // indirect
goa.design/goa/v3 v3.5.3 // indirect
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 // indirect
golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect
golang.org/x/mod v0.5.1 // indirect
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f // indirect
golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43 // indirect
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 // indirect
golang.org/x/sys v0.0.0-20211210111614-af8b64212486 // indirect
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect
golang.org/x/tools v0.1.8 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.62.0 // indirect
gopkg.in/ini.v1 v1.66.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
k8s.io/api v0.21.1 // indirect
Expand Down
Loading