Skip to content

swarm: initial instrumentation #15969

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Feb 23, 2018
Merged

swarm: initial instrumentation #15969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
89dfc96
swarm: initial instrumentation with go-metrics
holisticode Jan 12, 2018
b7516e4
swarm: initialise metrics collection and add ResettingTimer to HTTP r…
nonsense Jan 24, 2018
76dd62c
swarm: update metrics flags names. remove redundant Timer.
nonsense Jan 26, 2018
e05e4ca
swarm: rename method for periodically updating gauges
nonsense Jan 26, 2018
5af18e3
swarm: finalise metrics after feedback
holisticode Jan 30, 2018
661eeb7
swarm/network: always init kad metrics containers
nonsense Feb 21, 2018
13aa42e
swarm/network: off-by-one index in metrics containers
nonsense Feb 21, 2018
e891d75
swarm, metrics: resolved conflicts
nonsense Feb 23, 2018
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
8 changes: 7 additions & 1 deletion cmd/swarm/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/swarm"
bzzapi "github.com/ethereum/go-ethereum/swarm/api"
swarmmetrics "github.com/ethereum/go-ethereum/swarm/metrics"

"gopkg.in/urfave/cli.v1"
)
Expand Down Expand Up @@ -359,9 +360,14 @@ DEPRECATED: use 'swarm db clean'.
DeprecatedEnsAddrFlag,
}
app.Flags = append(app.Flags, debug.Flags...)
app.Flags = append(app.Flags, swarmmetrics.Flags...)
app.Before = func(ctx *cli.Context) error {
runtime.GOMAXPROCS(runtime.NumCPU())
return debug.Setup(ctx)
if err := debug.Setup(ctx); err != nil {
return err
}
swarmmetrics.Setup(ctx)
return nil
}
app.After = func(ctx *cli.Context) error {
debug.Exit()
Expand Down
1 change: 0 additions & 1 deletion metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ func init() {
Enabled = true
}
}
//exp.Exp(DefaultRegistry)
}

// CollectProcessMetrics periodically collects various metrics about the running
Expand Down
53 changes: 53 additions & 0 deletions swarm/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,31 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/swarm/storage"
)

var hashMatcher = regexp.MustCompile("^[0-9A-Fa-f]{64}")

//setup metrics
var (
apiResolveCount = metrics.NewRegisteredCounter("api.resolve.count", nil)
apiResolveFail = metrics.NewRegisteredCounter("api.resolve.fail", nil)
apiPutCount = metrics.NewRegisteredCounter("api.put.count", nil)
apiPutFail = metrics.NewRegisteredCounter("api.put.fail", nil)
apiGetCount = metrics.NewRegisteredCounter("api.get.count", nil)
apiGetNotFound = metrics.NewRegisteredCounter("api.get.notfound", nil)
apiGetHttp300 = metrics.NewRegisteredCounter("api.get.http.300", nil)
apiModifyCount = metrics.NewRegisteredCounter("api.modify.count", nil)
apiModifyFail = metrics.NewRegisteredCounter("api.modify.fail", nil)
apiAddFileCount = metrics.NewRegisteredCounter("api.addfile.count", nil)
apiAddFileFail = metrics.NewRegisteredCounter("api.addfile.fail", nil)
apiRmFileCount = metrics.NewRegisteredCounter("api.removefile.count", nil)
apiRmFileFail = metrics.NewRegisteredCounter("api.removefile.fail", nil)
apiAppendFileCount = metrics.NewRegisteredCounter("api.appendfile.count", nil)
apiAppendFileFail = metrics.NewRegisteredCounter("api.appendfile.fail", nil)
)

type Resolver interface {
Resolve(string) (common.Hash, error)
}
Expand Down Expand Up @@ -155,6 +175,7 @@ type ErrResolve error

// DNS Resolver
func (self *Api) Resolve(uri *URI) (storage.Key, error) {
apiResolveCount.Inc(1)
log.Trace(fmt.Sprintf("Resolving : %v", uri.Addr))

// if the URI is immutable, check if the address is a hash
Expand All @@ -169,6 +190,7 @@ func (self *Api) Resolve(uri *URI) (storage.Key, error) {
// if DNS is not configured, check if the address is a hash
if self.dns == nil {
if !isHash {
apiResolveFail.Inc(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Again is this really a resolve failure? I'd expect the resolve failure metric to tell me how many times Swarm tried to actually resolve a name but failed. Perhaps my expectation is incorrect though?

Copy link
Contributor

Choose a reason for hiding this comment

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

My expectation was to capture how many times a resolve was attempted with the meaning of requested (count) and how many times these attempts failed, answering the question: "How many times did people request a resolution, and how many times this request failed?". Both expectations (mine and yours) can lead to mal-interpretations. We should just settle on one. I believe it makes sense to keep them for now, and then remove such metrics as we go when we realize we are not gaining anything from them. Still, if people think we should remove ones like this one right away, I am ok with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it makes sense to keep them for now, and then remove such metrics as we go when we realize we are not gaining anything from them

I think it should be the other way around. Just add metrics we know are useful, with the freedom to add more useful ones in the future.

In this specific case, if I wanted to know how many resolve failures I'm getting, I'd also want to know what domains are failing too, in which case I'd probably just look for errors in my logs (which have much more context than just a single number).

Copy link
Contributor

Choose a reason for hiding this comment

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

When looking at failures, specifically in a production environment, but also in a staging/test environment, I would structure my workflow in creating alarms and visualizing error situations in a general manner first, like creating alarms when reaching certain thresholds. Only then I would, if the counters/graphs show unacceptable levels or alarms get triggered, I would then start the debugging process of looking at logs for why the alarms were triggered or why there is an unusual/unexpected high amount of errors. I believe this to be a very valid argument to keep this counter. Nevertheless, if it's still maintained that the counter itself is of no value, I will proceed to remove it.

return nil, fmt.Errorf("no DNS to resolve name: %q", uri.Addr)
}
return common.Hex2Bytes(uri.Addr), nil
Expand All @@ -179,23 +201,27 @@ func (self *Api) Resolve(uri *URI) (storage.Key, error) {
if err == nil {
return resolved[:], nil
} else if !isHash {
apiResolveFail.Inc(1)
return nil, err
}
return common.Hex2Bytes(uri.Addr), nil
}

// Put provides singleton manifest creation on top of dpa store
func (self *Api) Put(content, contentType string) (storage.Key, error) {
apiPutCount.Inc(1)
r := strings.NewReader(content)
wg := &sync.WaitGroup{}
key, err := self.dpa.Store(r, int64(len(content)), wg, nil)
if err != nil {
apiPutFail.Inc(1)
return nil, err
}
manifest := fmt.Sprintf(`{"entries":[{"hash":"%v","contentType":"%s"}]}`, key, contentType)
r = strings.NewReader(manifest)
key, err = self.dpa.Store(r, int64(len(manifest)), wg, nil)
if err != nil {
apiPutFail.Inc(1)
return nil, err
}
wg.Wait()
Expand All @@ -206,8 +232,10 @@ func (self *Api) Put(content, contentType string) (storage.Key, error) {
// to resolve basePath to content using dpa retrieve
// it returns a section reader, mimeType, status and an error
func (self *Api) Get(key storage.Key, path string) (reader storage.LazySectionReader, mimeType string, status int, err error) {
apiGetCount.Inc(1)
trie, err := loadManifest(self.dpa, key, nil)
if err != nil {
apiGetNotFound.Inc(1)
status = http.StatusNotFound
log.Warn(fmt.Sprintf("loadManifestTrie error: %v", err))
return
Expand All @@ -221,6 +249,7 @@ func (self *Api) Get(key storage.Key, path string) (reader storage.LazySectionRe
key = common.Hex2Bytes(entry.Hash)
status = entry.Status
if status == http.StatusMultipleChoices {
apiGetHttp300.Inc(1)
return
} else {
mimeType = entry.ContentType
Expand All @@ -229,16 +258,19 @@ func (self *Api) Get(key storage.Key, path string) (reader storage.LazySectionRe
}
} else {
status = http.StatusNotFound
apiGetNotFound.Inc(1)
err = fmt.Errorf("manifest entry for '%s' not found", path)
log.Warn(fmt.Sprintf("%v", err))
}
return
}

func (self *Api) Modify(key storage.Key, path, contentHash, contentType string) (storage.Key, error) {
apiModifyCount.Inc(1)
quitC := make(chan bool)
trie, err := loadManifest(self.dpa, key, quitC)
if err != nil {
apiModifyFail.Inc(1)
return nil, err
}
if contentHash != "" {
Expand All @@ -253,19 +285,23 @@ func (self *Api) Modify(key storage.Key, path, contentHash, contentType string)
}

if err := trie.recalcAndStore(); err != nil {
apiModifyFail.Inc(1)
return nil, err
}
return trie.hash, nil
}

func (self *Api) AddFile(mhash, path, fname string, content []byte, nameresolver bool) (storage.Key, string, error) {
apiAddFileCount.Inc(1)

uri, err := Parse("bzz:/" + mhash)
if err != nil {
apiAddFileFail.Inc(1)
return nil, "", err
}
mkey, err := self.Resolve(uri)
if err != nil {
apiAddFileFail.Inc(1)
return nil, "", err
}

Expand All @@ -284,16 +320,19 @@ func (self *Api) AddFile(mhash, path, fname string, content []byte, nameresolver

mw, err := self.NewManifestWriter(mkey, nil)
if err != nil {
apiAddFileFail.Inc(1)
return nil, "", err
}

fkey, err := mw.AddEntry(bytes.NewReader(content), entry)
if err != nil {
apiAddFileFail.Inc(1)
return nil, "", err
}

newMkey, err := mw.Store()
if err != nil {
apiAddFileFail.Inc(1)
return nil, "", err

}
Expand All @@ -303,13 +342,16 @@ func (self *Api) AddFile(mhash, path, fname string, content []byte, nameresolver
}

func (self *Api) RemoveFile(mhash, path, fname string, nameresolver bool) (string, error) {
apiRmFileCount.Inc(1)

uri, err := Parse("bzz:/" + mhash)
if err != nil {
apiRmFileFail.Inc(1)
return "", err
}
mkey, err := self.Resolve(uri)
if err != nil {
apiRmFileFail.Inc(1)
return "", err
}

Expand All @@ -320,16 +362,19 @@ func (self *Api) RemoveFile(mhash, path, fname string, nameresolver bool) (strin

mw, err := self.NewManifestWriter(mkey, nil)
if err != nil {
apiRmFileFail.Inc(1)
return "", err
}

err = mw.RemoveEntry(filepath.Join(path, fname))
if err != nil {
apiRmFileFail.Inc(1)
return "", err
}

newMkey, err := mw.Store()
if err != nil {
apiRmFileFail.Inc(1)
return "", err

}
Expand All @@ -338,6 +383,7 @@ func (self *Api) RemoveFile(mhash, path, fname string, nameresolver bool) (strin
}

func (self *Api) AppendFile(mhash, path, fname string, existingSize int64, content []byte, oldKey storage.Key, offset int64, addSize int64, nameresolver bool) (storage.Key, string, error) {
apiAppendFileCount.Inc(1)

buffSize := offset + addSize
if buffSize < existingSize {
Expand Down Expand Up @@ -366,10 +412,12 @@ func (self *Api) AppendFile(mhash, path, fname string, existingSize int64, conte

uri, err := Parse("bzz:/" + mhash)
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err
}
mkey, err := self.Resolve(uri)
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err
}

Expand All @@ -380,11 +428,13 @@ func (self *Api) AppendFile(mhash, path, fname string, existingSize int64, conte

mw, err := self.NewManifestWriter(mkey, nil)
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err
}

err = mw.RemoveEntry(filepath.Join(path, fname))
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err
}

Expand All @@ -398,11 +448,13 @@ func (self *Api) AppendFile(mhash, path, fname string, existingSize int64, conte

fkey, err := mw.AddEntry(io.Reader(combinedReader), entry)
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err
}

newMkey, err := mw.Store()
if err != nil {
apiAppendFileFail.Inc(1)
return nil, "", err

}
Expand All @@ -412,6 +464,7 @@ func (self *Api) AppendFile(mhash, path, fname string, existingSize int64, conte
}

func (self *Api) BuildDirectoryTree(mhash string, nameresolver bool) (key storage.Key, manifestEntryMap map[string]*manifestTrieEntry, err error) {

uri, err := Parse("bzz:/" + mhash)
if err != nil {
return nil, nil, err
Expand Down
9 changes: 9 additions & 0 deletions swarm/api/http/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,19 @@ import (
"time"

"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/swarm/api"
)

//templateMap holds a mapping of an HTTP error code to a template
var templateMap map[int]*template.Template

//metrics variables
var (
htmlCounter = metrics.NewRegisteredCounter("api.http.errorpage.html.count", nil)
jsonCounter = metrics.NewRegisteredCounter("api.http.errorpage.json.count", nil)
)

//parameters needed for formatting the correct HTML page
type ErrorParams struct {
Msg string
Expand Down Expand Up @@ -132,6 +139,7 @@ func respond(w http.ResponseWriter, r *http.Request, params *ErrorParams) {

//return a HTML page
func respondHtml(w http.ResponseWriter, params *ErrorParams) {
htmlCounter.Inc(1)
err := params.template.Execute(w, params)
if err != nil {
log.Error(err.Error())
Expand All @@ -140,6 +148,7 @@ func respondHtml(w http.ResponseWriter, params *ErrorParams) {

//return JSON
func respondJson(w http.ResponseWriter, params *ErrorParams) {
jsonCounter.Inc(1)
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(params)
}
Expand Down
Loading