Skip to content

Commit

Permalink
Merge branch 'improve-error-logging' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
anbsky committed Nov 5, 2024
2 parents 17f1173 + 6654703 commit 99e2e2c
Show file tree
Hide file tree
Showing 18 changed files with 173 additions and 152 deletions.
14 changes: 7 additions & 7 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,24 @@ on:
permissions:
contents: read
# Optional: allow read access to pull request. Use with `only-new-issues` option.
# pull-requests: read
pull-requests: read

jobs:
golangci:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-go@v4
- uses: actions/setup-go@v5
with:
go-version: '1.22.x'
go-version: '1.23'
cache: false
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v6
with:
version: 'v1.56.2'
# only-new-issues: true
version: 'v1.60'
only-new-issues: true
- id: govulncheck
uses: golang/govulncheck-action@v1
28 changes: 0 additions & 28 deletions .github/workflows/stale.yml

This file was deleted.

12 changes: 6 additions & 6 deletions .github/workflows/test-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ jobs:
name: test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: FedericoCarboni/setup-ffmpeg@v2
- uses: actions/setup-go@v4
- uses: actions/setup-go@v5
with:
go-version: '1.22.x'
go-version: '1.23'
id: go

- name: Retrieve release details
Expand Down Expand Up @@ -78,12 +78,12 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-go@v4
- uses: actions/setup-go@v5
with:
go-version: '1.22.x'
go-version: '1.23'
id: go

- run: git branch
Expand Down
9 changes: 0 additions & 9 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,6 @@ prepare_test:
$(go) install $($(go) list -tags tools -f '{{range $_, $p := .Imports}}{{$p}} {{end}}')
$(go) run . db_migrate_up

.PHONY: test_circleci
test_circleci:
scripts/wait_for_wallet.sh
cd tools &&\
$(go) install $($(go) list -tags tools -f '{{range $_, $p := .Imports}}{{$p}} {{end}}')
$(go) run . db_migrate_up
$(go) test -covermode=count -coverprofile=coverage.out ./...
goveralls -coverprofile=coverage.out -service=circle-ci -ignore=models/ -repotoken $(COVERALLS_TOKEN)

.PHONY: clean
clean:
rm -rf ./dist
Expand Down
2 changes: 1 addition & 1 deletion app/arweave/arweave_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestReplaceAssetUrl(t *testing.T) {
}

func TestGetClaimUrl(t *testing.T) {
// t.Skip("skipping this in automated mode as it requires extra setup on arfleet")
t.Skip("skipping this in automated mode as it requires extra setup on arfleet")

require := require.New(t)
assert := assert.New(t)
Expand Down
23 changes: 15 additions & 8 deletions app/asynquery/asynquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/OdyseeTeam/odysee-api/app/geopublish/metrics"
"github.com/OdyseeTeam/odysee-api/app/query"
"github.com/OdyseeTeam/odysee-api/internal/monitor"
"github.com/OdyseeTeam/odysee-api/internal/tasks"
"github.com/OdyseeTeam/odysee-api/models"
"github.com/OdyseeTeam/odysee-api/pkg/logging"
Expand Down Expand Up @@ -232,8 +233,9 @@ func (m *CallManager) robustCall(ctx context.Context, aq *models.Asynquery, patc
log.Info("error getting sdk address for user")
return asynq.SkipRetry
}
sdkAddress := u.R.LbrynetServer.Address

caller := query.NewCaller(u.R.LbrynetServer.Address, aq.UserID)
caller := query.NewCaller(sdkAddress, aq.UserID)

t := time.Now()

Expand Down Expand Up @@ -261,18 +263,23 @@ func (m *CallManager) robustCall(ctx context.Context, aq *models.Asynquery, patc
}
delete(pp, "file_path")

res, err := caller.Call(context.TODO(), request)
ctx = query.AttachOrigin(ctx, "asynquery")
res, err := caller.Call(ctx, request)
metrics.ProcessingTime.WithLabelValues(metrics.LabelProcessingQuery).Observe(float64(time.Since(t)))

QueriesSent.Inc()

if err != nil {
QueriesFailed.Inc()
log.Warn(sdkNetError.Error(), "err", err)
resErrMsg := err.Error()
log.Warn("asynquery failed", "err", resErrMsg)
m.finalizeQueryRecord(ctx, aq.ID, nil, err.Error())
log.Warn("asynquery request failed", "err", err)
monitor.ErrorToSentry(fmt.Errorf("asynquery request failed: %w", err), map[string]string{
"user_id": fmt.Sprintf("%d", u.ID),
"endpoint": sdkAddress,
"method": request.Method,
})

err := m.finalizeQueryRecord(ctx, aq.ID, nil, err.Error())
if err != nil {
log.Warn("failed to finalize query record", "err", err)
log.Warn("failed to finalize asynquery record", "err", err)
}
return sdkNetError
}
Expand Down
2 changes: 1 addition & 1 deletion app/geopublish/geopublish.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func getCaller(sdkAddress, filename string, userID int, qCache *cache.Cache) *qu
c := query.NewCaller(sdkAddress, userID)
c.Cache = qCache
c.AddPreflightHook(query.AllMethodsHook, func(_ *query.Caller, ctx context.Context) (*jsonrpc.RPCResponse, error) {
q := query.GetFromContext(ctx)
q := query.QueryFromContext(ctx)
params := q.ParamsAsMap()
params[fileNameParam] = filename
q.Request.Params = params
Expand Down
4 changes: 0 additions & 4 deletions app/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,7 @@ func Handle(w http.ResponseWriter, r *http.Request) {

c.Cache = qCache

metrics.ProxyCallCounter.WithLabelValues(rpcReq.Method, c.Endpoint(), origin).Inc()
rpcRes, err := c.Call(r.Context(), rpcReq)
metrics.ProxyCallDurations.WithLabelValues(rpcReq.Method, c.Endpoint(), origin).Observe(c.Duration)

if err != nil {
writeResponse(w, rpcerrors.ToJSON(err))
Expand All @@ -154,8 +152,6 @@ func Handle(w http.ResponseWriter, r *http.Request) {
}
monitor.ErrorToSentry(err, map[string]string{"request": fmt.Sprintf("%+v", rpcReq), "response": fmt.Sprintf("%+v", rpcRes)})
observeFailure(metrics.GetDuration(r), rpcReq.Method, metrics.FailureKindNet)
metrics.ProxyCallFailedDurations.WithLabelValues(rpcReq.Method, c.Endpoint(), origin, metrics.FailureKindNet).Observe(c.Duration)
metrics.ProxyCallFailedCounter.WithLabelValues(rpcReq.Method, c.Endpoint(), origin, metrics.FailureKindNet).Inc()
logger.Log().Errorf("error calling lbrynet: %v, request: %+v", err, rpcReq)
return
}
Expand Down
2 changes: 1 addition & 1 deletion app/publish/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func getCaller(sdkAddress, filename string, userID int, qCache *cache.Cache) *qu
c := query.NewCaller(sdkAddress, userID)
c.Cache = qCache
c.AddPreflightHook(query.AllMethodsHook, func(_ *query.Caller, ctx context.Context) (*jsonrpc.RPCResponse, error) {
q := query.GetFromContext(ctx)
q := query.QueryFromContext(ctx)
params := q.ParamsAsMap()
params[fileNameParam] = filename
q.Request.Params = params
Expand Down
63 changes: 23 additions & 40 deletions app/query/caller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ const (
AllMethodsHook = ""
)

type contextKey string

var (
contextKeyQuery = contextKey("query")
contextKeyResponse = contextKey("response")
contextKeyLogEntry = contextKey("log-entry")
)

type HTTPRequester interface {
Do(req *http.Request) (res *http.Response, err error)
}
Expand All @@ -50,38 +42,13 @@ type HTTPRequester interface {
// Hooks can modify both query and response, as well as perform additional queries via supplied Caller.
// If nil is returned instead of *jsonrpc.RPCResponse, original response is returned.
type Hook func(*Caller, context.Context) (*jsonrpc.RPCResponse, error)

type hookEntry struct {
method string
function Hook
name string
}

func AttachToContext(ctx context.Context, query *Query) context.Context {
return context.WithValue(ctx, contextKeyQuery, query)
}

func GetFromContext(ctx context.Context) *Query {
return ctx.Value(contextKeyQuery).(*Query)
}

func WithResponse(ctx context.Context, response *jsonrpc.RPCResponse) context.Context {
return context.WithValue(ctx, contextKeyResponse, response)
}

func GetResponse(ctx context.Context) *jsonrpc.RPCResponse {
return ctx.Value(contextKeyResponse).(*jsonrpc.RPCResponse)
}

func WithLogEntry(ctx context.Context, entry *logrus.Entry) context.Context {
return context.WithValue(ctx, contextKeyLogEntry, entry)
}

// WithLogField injects additional data into default post-query log entry
func WithLogField(ctx context.Context, key string, value interface{}) {
e := ctx.Value(contextKeyLogEntry).(*logrus.Entry)
e.Data[key] = value
}

// Caller patches through JSON-RPC requests from clients, doing pre/post-processing,
// account processing and validation.
type Caller struct {
Expand Down Expand Up @@ -159,8 +126,10 @@ func (c *Caller) addDefaultHooks() {
c.AddPreflightHook(MethodGet, preflightHookGet, builtinHookName)
c.AddPreflightHook(MethodClaimSearch, preflightHookClaimSearch, builtinHookName)

c.AddPostflightHook(MethodClaimSearch, postClaimSearchArfleetThumbs, builtinHookName)
c.AddPostflightHook(MethodResolve, postResolveArfleetThumbs, builtinHookName)
if config.GetArfleetEnabled() {
c.AddPostflightHook(MethodClaimSearch, postClaimSearchArfleetThumbs, builtinHookName)
c.AddPostflightHook(MethodResolve, postResolveArfleetThumbs, builtinHookName)
}
}

func (c *Caller) CloneWithoutHook(endpoint, method, name string) *Caller {
Expand All @@ -187,6 +156,20 @@ func (c *Caller) Endpoint() string {
// Call method forwards a JSON-RPC request to the lbrynet server.
// It returns a response that is ready to be sent back to the JSON-RPC client as is.
func (c *Caller) Call(ctx context.Context, req *jsonrpc.RPCRequest) (*jsonrpc.RPCResponse, error) {
origin := OriginFromContext(ctx)
metrics.ProxyCallCounter.WithLabelValues(req.Method, c.Endpoint(), origin).Inc()
res, err := c.call(ctx, req)
metrics.ProxyCallDurations.WithLabelValues(req.Method, c.Endpoint(), origin).Observe(c.Duration)
if err != nil {
metrics.ProxyCallFailedDurations.WithLabelValues(req.Method, c.Endpoint(), origin, metrics.FailureKindNet).Observe(c.Duration)
metrics.ProxyCallFailedCounter.WithLabelValues(req.Method, c.Endpoint(), origin, metrics.FailureKindNet).Inc()
}
return res, err
}

// Call method forwards a JSON-RPC request to the lbrynet server.
// It returns a response that is ready to be sent back to the JSON-RPC client as is.
func (c *Caller) call(ctx context.Context, req *jsonrpc.RPCRequest) (*jsonrpc.RPCResponse, error) {
if c.endpoint == "" {
return nil, errors.Err("cannot call blank endpoint")
}
Expand All @@ -203,7 +186,7 @@ func (c *Caller) Call(ctx context.Context, req *jsonrpc.RPCRequest) (*jsonrpc.RP

// Applying preflight hooks
var res *jsonrpc.RPCResponse
ctx = AttachToContext(ctx, q)
ctx = AttachQuery(ctx, q)
for _, hook := range c.preflightHooks {
if isMatchingHook(q.Method(), hook) {
res, err = hook.function(c, ctx)
Expand Down Expand Up @@ -251,7 +234,7 @@ func (c *Caller) SendQuery(ctx context.Context, q *Query) (*jsonrpc.RPCResponse,
start := time.Now()
client := c.getRPCClient(q.Method())
r, err = client.CallRaw(q.Request)
c.Duration = time.Since(start).Seconds()
c.Duration += time.Since(start).Seconds()
logger.Log().Debugf("sent request: %s %+v (%.2fs)", q.Method(), q.Params(), c.Duration)

// Generally a HTTP transport failure (connect error etc)
Expand Down Expand Up @@ -303,8 +286,8 @@ func (c *Caller) SendQuery(ctx context.Context, q *Query) (*jsonrpc.RPCResponse,

// Applying postflight hooks
var hookResp *jsonrpc.RPCResponse
ctx = WithLogEntry(ctx, logEntry)
ctx = WithResponse(ctx, r)
ctx = AttachLogEntry(ctx, logEntry)
ctx = AttachResponse(ctx, r)
for _, hook := range c.postflightHooks {
if isMatchingHook(q.Method(), hook) {
hookResp, err = hook.function(c, ctx)
Expand Down
Loading

0 comments on commit 99e2e2c

Please sign in to comment.