Skip to content

Add prometheus stats#16

Merged
CoderCookE merged 15 commits into
masterfrom
ec/stats
Feb 27, 2020
Merged

Add prometheus stats#16
CoderCookE merged 15 commits into
masterfrom
ec/stats

Conversation

@CoderCookE

Copy link
Copy Markdown
Owner

No description provided.

Comment thread internal/stats/main.go
addr = flag.String("prometheus-listen-address", ":8080", "The address to listen on for HTTP requests.")
)

var (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NIT: merge these 2 var sections.

Comment thread internal/stats/main.go Outdated
)

var (
addr = flag.String("prometheus-listen-address", ":8080", "The address to listen on for HTTP requests.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this come from a config or environment var, instead of the commandline?

@CoderCookE CoderCookE Feb 26, 2020

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I'd think a config, but this was the path of least resistance for this project since at the moment its all cli/flag based

Comment thread go.sum
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/OneOfOne/xxhash v1.2.7 h1:fzrmmkskv067ZQbd9wERNGuxckWw67dyzoMG62p7LMo=
github.com/OneOfOne/xxhash v1.2.7/go.mod h1:eZbhyaAYD41SGSSsnmcpxVoRiQ/MPUTjUdIIOT9Um7Q=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WOW! This brings in a lot of dependencies!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NIT: Also, consider running go mod tidy to clean up some of these. Ex, go-spew is still listed in this file.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

this actually is the output of go mod tidy. not sure where go-spew is coming from.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yikes! Something else may be using it. it is pulling in go-kit and grpc. eithe one of those could have that as a dependency. go mod graph | grep go-spew would tell you what's using it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

something seems to be using testify and logrus which have go-spew as dependencies

* origin/master:
  remove useless waitgroups in test
* origin/master:
  not every request is going to be a hit, so just check that some of them hit the cache
@CoderCookE CoderCookE mentioned this pull request Feb 26, 2020
7 tasks
@CoderCookE CoderCookE merged commit a5853c2 into master Feb 27, 2020
@CoderCookE CoderCookE deleted the ec/stats branch February 27, 2020 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants