Skip to content

Commit

Permalink
Merge pull request #56 from lbryio/feature/benchmarking
Browse files Browse the repository at this point in the history
Feature/benchmarking
  • Loading branch information
anbsky committed Aug 22, 2019
2 parents fc73536 + cb613fd commit 4f2638b
Show file tree
Hide file tree
Showing 15 changed files with 199 additions and 155 deletions.
58 changes: 0 additions & 58 deletions .github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,58 +0,0 @@
<!--
Thanks for reporting an issue and helping us improve!
To make it possible for us to help you, please fill out below information carefully.
-->


## The Issue

In order to <achieve some value>,
as a <type of user>,
I want <some functionality>.


### Steps to reproduce
1.
2.
3.

### Expected behaviour
Tell us what should happen

### Actual behaviour
Tell us what happens instead


## System Configuration

- Browser version:
- Operating system:


## Anything Else
<!-- Include anything else that does not fit into the above sections -->


## Screenshots
<!-- If a screenshot would help explain the bug, please include one or two here -->


## Internal Use

### Acceptance Criteria
1.
2.
3.

### Definition of Done
- [ ] Tested against acceptance criteria
- [ ] Tested against the assumptions of user story
- [ ] The project builds without errors
- [ ] Unit tests are written and passing
- [ ] Tests on devices/OS listed in the issue have passed
- [ ] QA performed & issues resolved
- [ ] Refactoring completed
- [ ] Any configuration or build changes documented
- [ ] Documentation updated
- [ ] Peer Code Review performed
38 changes: 0 additions & 38 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,38 +0,0 @@
## PR Checklist
Please check all that apply to this PR using "x":

- [ ] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
- [ ] I have checked that this PR does not introduce a breaking change
- [ ] This PR introduces breaking changes and I have provided a detailed explanation below


## PR Type
What kind of change does this PR introduce?

Why is this change necessary?

<!-- Please check all that apply to this PR using "x". -->

- [ ] Bugfix
- [ ] Feature
- [ ] Breaking changes (bugfix or feature that introduces breaking changes)
- [ ] Code style update (formatting)
- [ ] Refactoring (no functional changes)
- [ ] Documentation changes
- [ ] Other - Please describe:

## Fixes

Issue Number: N/A


## What is the current behavior?


## What is the new behavior?


## Other information


<!-- If this PR contains a breaking change, please describe the impact and solution strategy for existing applications below. -->
5 changes: 5 additions & 0 deletions api/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,25 @@ import (
"net/http"

"github.com/lbryio/lbrytv/app/player"
"github.com/lbryio/lbrytv/internal/metrics"
"github.com/lbryio/lbrytv/internal/monitor"

"github.com/gorilla/mux"
)

var logger = monitor.NewModuleLogger("api")

var Collector = metrics.NewCollector()

// Index serves a blank home page
func Index(w http.ResponseWriter, req *http.Request) {
w.WriteHeader(http.StatusOK)
}

func stream(uri string, w http.ResponseWriter, req *http.Request) {
Collector.MetricsIncrement("player_instances_count", metrics.One)
err := player.PlayURI(uri, w, req)
Collector.MetricsDecrement("player_instances_count", metrics.One)
// Only output error if player has not pushed anything to the client yet
if err != nil {
if err.Error() == "paid stream" {
Expand Down
2 changes: 1 addition & 1 deletion app/proxy/query_filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var forbiddenMethods = []string{
"sync_apply",
}

const forbiddenParam = "account_id"
const forbiddenParam = paramAccountID

func MethodRequiresAccountID(method string) bool {
return methodInList(method, accountSpecificMethods)
Expand Down
32 changes: 22 additions & 10 deletions app/proxy/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
// Service generates Caller objects and keeps execution time metrics
// for all calls proxied through those objects.
type Service struct {
*metrics.ExecTimeMetrics
*metrics.Collector
TargetEndpoint string
logger monitor.QueryMonitor
}
Expand All @@ -43,9 +43,9 @@ type Query struct {
// Normally only one instance of Service should be created per running server.
func NewService(targetEndpoint string) *Service {
s := Service{
ExecTimeMetrics: metrics.NewMetrics(),
TargetEndpoint: targetEndpoint,
logger: monitor.NewProxyLogger(),
Collector: metrics.NewCollector(),
TargetEndpoint: targetEndpoint,
logger: monitor.NewProxyLogger(),
}
return &s
}
Expand Down Expand Up @@ -97,6 +97,16 @@ func (q *Query) ParamsAsMap() map[string]interface{} {
return nil
}

func (q *Query) paramsForLog() map[string]interface{} {
params := q.ParamsAsMap()
for k := range params {
if k == paramAccountID {
params[k] = monitor.ValueMask
}
}
return params
}

// cacheHit returns true if we got a resolve query with more than `cacheResolveLongerThan` urls in it.
func (q *Query) isCacheable() bool {
if q.Method() == methodResolve && q.Params() != nil {
Expand Down Expand Up @@ -129,7 +139,7 @@ func (q *Query) attachAccountID(id string) {
p[paramAccountID] = id
q.Request.Params = p
} else {
q.Request.Params = map[string]string{"account_id": id}
q.Request.Params = map[string]interface{}{"account_id": id}
}
}
}
Expand Down Expand Up @@ -232,16 +242,16 @@ func (c *Caller) call(rawQuery []byte) (*jsonrpc.RPCResponse, CallError) {
queryStartTime := time.Now()
r, err := c.sendQuery(q)
if err != nil {
return nil, NewInternalError(err)
return r, NewInternalError(err)
}
execTime := time.Now().Sub(queryStartTime).Seconds()

c.service.LogExecTime(q.Method(), execTime, q.Params())
c.service.SetMetricsValue(q.Method(), execTime, q.Params())

if r.Error == nil {
c.service.logger.LogSuccessfulQuery(q.Method(), execTime, q.Params())
if r.Error != nil {
c.service.logger.LogFailedQuery(q.Method(), q.paramsForLog(), r.Error)
} else {
c.service.logger.LogFailedQuery(q.Method(), q.Params(), r.Error)
c.service.logger.LogSuccessfulQuery(q.Method(), execTime, q.paramsForLog())
}

r, err = processResponse(q.Request, r)
Expand All @@ -257,10 +267,12 @@ func (c *Caller) call(rawQuery []byte) (*jsonrpc.RPCResponse, CallError) {
func (c *Caller) Call(rawQuery []byte) []byte {
r, err := c.call(rawQuery)
if err != nil {
monitor.CaptureException(err, map[string]string{"query": string(rawQuery), "response": fmt.Sprintf("%v", r)})
return c.marshalError(err)
}
serialized, err := c.marshal(r)
if err != nil {
monitor.CaptureException(err)
return c.marshalError(err)
}
return serialized
Expand Down
51 changes: 48 additions & 3 deletions app/proxy/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestCallerMetrics(t *testing.T) {
service: svc,
}
c.Call([]byte(newRawRequest(t, "resolve", map[string]string{"urls": "what"})))
assert.Equal(t, 0.25, math.Round(svc.GetExecTimeMetrics("resolve").ExecTime*100)/100)
assert.Equal(t, 0.25, math.Round(svc.GetMetricsValue("resolve").Value*100)/100)
}

func TestCallResolve(t *testing.T) {
Expand All @@ -116,7 +116,29 @@ func TestCallResolve(t *testing.T) {
rawCallReponse := c.Call(request)
parseRawResponse(t, rawCallReponse, &resolveResponse)
assert.Equal(t, resolvedClaimID, resolveResponse[resolvedURL].ClaimID)
assert.True(t, svc.GetExecTimeMetrics("resolve").ExecTime > 0)
assert.True(t, svc.GetMetricsValue("resolve").Value > 0)
}

func TestCallAccountBalance(t *testing.T) {
// TODO: Add actual account balance response check after 0.39 support is added to lbry.go
// var accountBalanceResponse ljsonrpc.AccountBalanceResponse

rand.Seed(time.Now().UnixNano())
dummyAccountID := rand.Int()

acc, _ := lbrynet.CreateAccount(dummyAccountID)
defer lbrynet.RemoveAccount(dummyAccountID)

svc := NewService(config.GetLbrynet())
c := svc.NewCaller()
c.SetAccountID(acc.ID)

request := newRawRequest(t, "account_balance", nil)
hook := logrus_test.NewLocal(svc.logger.Logger())
c.Call(request)

assert.Equal(t, map[string]interface{}{"account_id": "****"}, hook.LastEntry().Data["params"])
assert.Equal(t, "account_balance", hook.LastEntry().Data["method"])
}

func TestCallAccountList(t *testing.T) {
Expand All @@ -136,7 +158,7 @@ func TestCallAccountList(t *testing.T) {
rawCallReponse := c.Call(request)
parseRawResponse(t, rawCallReponse, &accResponse)
assert.Equal(t, acc.ID, accResponse.ID)
assert.True(t, svc.GetExecTimeMetrics("account_list").ExecTime > 0)
assert.True(t, svc.GetMetricsValue("account_list").Value > 0)
}

func TestCallSDKError(t *testing.T) {
Expand Down Expand Up @@ -204,3 +226,26 @@ func TestCallClientJSONError(t *testing.T) {
assert.Equal(t, "unexpected end of JSON input", rpcResponse.Error.Message)
assert.Equal(t, "malformed JSON from client: unexpected end of JSON input", hook.LastEntry().Message)
}

func TestParamsAsMap(t *testing.T) {
var q *Query

q, _ = NewQuery(newRawRequest(t, "version", nil))
assert.Nil(t, q.ParamsAsMap())

q, _ = NewQuery(newRawRequest(t, "resolve", map[string]string{"urls": "what"}))
assert.Equal(t, map[string]interface{}{"urls": "what"}, q.ParamsAsMap())

q, _ = NewQuery(newRawRequest(t, "account_balance", nil))
q.attachAccountID("123")
assert.Equal(t, map[string]interface{}{"account_id": "123"}, q.ParamsAsMap())

searchParams := map[string]interface{}{
"any_tags": []interface{}{
"art", "automotive", "blockchain", "comedy", "economics", "education",
"gaming", "music", "news", "science", "sports", "technology",
},
}
q, _ = NewQuery(newRawRequest(t, "claim_search", searchParams))
assert.Equal(t, searchParams, q.ParamsAsMap())
}
2 changes: 1 addition & 1 deletion deployments/docker-dev/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ services:
POSTGRES_USER: lbrytv
POSTGRES_PASSWORD: lbrytv
lbrynet:
image: lbry/lbrynet-tv:latest-rc
image: lbry/lbrynet-tv:latest
ports:
- "5579:5279"
volumes:
Expand Down
2 changes: 1 addition & 1 deletion deployments/docker/app/config/lbrytv.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Database:
DBName: lbrytv
Options: sslmode=disable

AccountsEnabled: 0
AccountsEnabled: 1

MetricsAddress: :2112
MetricsPath: /metrics
6 changes: 0 additions & 6 deletions deployments/docker/prometheus/prometheus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ global:
monitor: 'codelab-monitor'

scrape_configs:
# The job name is added as a label `job=<job_name>` to any timeseries scraped from this config.
- job_name: 'prometheus'
scrape_interval: 5s
static_configs:
- targets: ['localhost:9090']

- job_name: 'app'
scrape_interval: 1s
static_configs:
Expand Down
Loading

0 comments on commit 4f2638b

Please sign in to comment.