Use consisent logging instead of printf everywhere.

[krasi-georgiev]

The codebase has printf in a lot of places which is easy to start with, but when parsing or filtering logs it brings up many problems later on with monitoring and tracing.

a logger should be initialized in the main package and then passed to all package constructors.

This is a very good logging package
github.com/go-kit/kit/log

for an example how to implement look at the thanos or prometheus projects.

all commented-out printf-s can be replaced with debug logs so that these are displayed only when debugging is enabled.
level.Debug(logger).Log("msg", "debug")

Important! Check the project coding guidelines
https://github.com/tellor-io/telliot/blob/master/docs/coding-style-guide.md

[JGcarv]

I'll take this one today!

[krasi-georgiev]

An update: this is partially done, but there are still few places with printf and logrus.
TL-DR need to use github.com/go-kit/kit/log everywhere. It needs to be passed as an argument when initializing the components.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 0.1 ETH (114.26 USD @ $1142.63/ETH) attached to it as part of the Tellor-io fund.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin's Discord
• $1,033,707.71 more funded OSS Work available on the Gitcoin Issue Explorer

[GabrielNicolasAvellaneda]

@krasi-georgiev I would like to take on this but I after reading your suggestions on Gitcoin and reading the Telliot code I would say that instead of changing all commented out fmt.Printf to level.Debug(logger).Log(....) we should decide on each situation what makes sense more sense from the user perspective.
I would use log debug when the output should be useful for inspecting a problem, more from a developer point of view, but situations where the user expects output to the console, should be INFO level. Like for example
https://github.com/tellor-io/telliot/blob/master/pkg/ops/disputeOps.go#L249-L255

	fmt.Printf("Dispute %s (%s):\n", dispute.DisputeId.String(), descString)
	fmt.Printf("    Accused Party: %s\n", reportedAddr.Hex())
	fmt.Printf("    Disputed by: %s\n", reportingMiner.Hex())
	fmt.Printf("    Created on:  %s\n", createdTime.Format("3:04 PM January 02, 2006 MST"))
	fmt.Printf("    Fee: %s TRB\n", util.FormatERC20Balance(uintVars[8]))
	fmt.Printf("    \n")
	fmt.Printf("    Value disputed for requestID %d:\n", dispute.RequestId.Uint64())

Please correct me if I'm wrong. Thanks!

[krasi-georgiev]

yep very true! Also another rule is that of a log happens once at start up it is fine to be info, but if it happens many times in a loop unless it is extremely important in most case this is too much noise and it should be debug.

[GabrielNicolasAvellaneda]

@krasi-georgiev Yes, makes sense to not overwhelm the user, but I think that in situations where you want to analyze the logs with a tool, changing behavior can be confusing as that's the only feedback that you have for your analysis, but anyway we will get the right balance I think. I'm going to do some changes here and as I learn more about this project we will find what works for a good user experience.

[krasi-georgiev]

once this is done also should add in the same PR some more linting to the makefile like

go-lint: check-git deps $(GOLANGCI_LINT) $(FAILLINT)
	$(call require_clean_work_tree,'detected not clean master before running lint, previous job changed something?')
	@echo ">> verifying modules being imported"
	@$(FAILLINT) -paths "errors=github.com/pkg/errors" ./...
	@$(FAILLINT) -paths "fmt.{Print,Printf,Println,Sprint}" -ignore-tests ./...
	@echo ">> linting all of the Go files GOGC=${GOGC}"
	@$(GOLANGCI_LINT) run
	@echo ">> ensuring Copyright headers"
	@go run ./scripts/copyright
	$(call require_clean_work_tree,'detected files without copyright, run make lint and commit changes')

the 2 etra lines are:

	@$(FAILLINT) -paths "errors=github.com/pkg/errors" ./...
	@$(FAILLINT) -paths "fmt.{Print,Printf,Println,Sprint}" -ignore-tests ./...

This will enforce that there are no print statements anywhere and also that pkg/errors is used everwhere

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 265 years, 6 months from now.
Please review their action plans below:

1) g33kidd has applied to start work (Funders only: approve worker | reject worker).

I will take a stab at this, it seemed like an interesting thing to do.
2) voanhcung has applied to start work (Funders only: approve worker | reject worker).

Ok thaiquang0602@gmail.com.......................
3) therocketcat has been approved to start work.

I looked at the code base and the previously mentioned example projects you mentioned.

From the examples i've gathered that if the functions using printf dont have a constructor then the logger should be dependency injected into the function itself. If this isnt the case, then i'd like clarification there.

And in the main package (telliot/cmd/telliot/main.go) they will be setup using the SetupLogger function from util and passed to either the constructor or injected directly into function (as first parameter).

Also looking at the codebase i cant find any commented out printfs, so im assuming you mean printfs in general?

Open to discuss this further if i've misunderstood the task RocketCat#3507
4) mendesfabio has applied to start work (Funders only: approve worker | reject worker).

I've just checked the repo and it seems some printf-s were replaced (and it helped me to understand how to do it). We still have 10 files with printf-s and I will change that.
5) rodrigoadriandiaz has applied to start work (Funders only: approve worker | reject worker).

i could do this, replacing all calls to printf for an implementation of the above mentioned logging package, i could start working on this tomorrow
6) raulcorreia7 has applied to start work (Funders only: approve worker | reject worker).

Hi,
This would be a no brainer,
DM me.
7) coder4520 has applied to start work (Funders only: approve worker | reject worker).

I guess this would be really good first project to start with.
8) adiprerepa has applied to start work (Funders only: approve worker | reject worker).

This would be very easy - just find and replace printf to logging and then double checking.
9) xf3rno has applied to start work (Funders only: approve worker | reject worker).

I would perform the requested task; replace all current logging with google's golang logger library (quite lightweight: https://github.com/google/logger), and scope each message depending on the context (debug, warning, etc).

Learn more on the Gitcoin Issue Details page.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 0.1 ETH (167.47 USD @ $1674.72/ETH) has been submitted by:

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin's Discord
• $1,072,039.97 more funded OSS Work available on the Gitcoin Issue Explorer