Skip to content
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.

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

Differentiate log level depending on "response.status" #178

Open
anar-khalilov opened this issue Nov 3, 2022 · 4 comments
Open

Differentiate log level depending on "response.status" #178

anar-khalilov opened this issue Nov 3, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@anar-khalilov
Copy link
Contributor

anar-khalilov commented Nov 3, 2022

#179

Currently, all requests are being logged as Info (v1.4.3/instrumentation.go:56) and it really results in lots and lots of logs, hence increases costs for our applications.

As per our application's requirements, we don't want to see logs for HTTP200 responses.

What we would like to achieve is to log depending on the response.status value like below.

However, if you come up with a better idea, we are welcome to use it.

func (e *event) finish() {
	e.writeLock.Do(func() {

		var logEntry = log.WithFields(log.Fields{
			"timestamp": e.timestamp.Format(time.RFC3339Nano),
			"duration":  time.Since(e.timestamp).String(),
		}).WithFields(log.Fields(e.fields))

		responseStatusCandidate := e.fields["response.status"]

		if responseStatusCandidate == nil{
			logEntry.Info(e.name)
			return
		}

		responseStatusCode, ok := responseStatusCandidate.(int)

		if !ok{
			logEntry.Info(e.name)
			return
		}

		if (responseStatusCode >= 100 && responseStatusCode <= 199){
			// informational responses
			logEntry.Debug(e.name)
		} else if (responseStatusCode >= 200 && responseStatusCode <= 299){
			// successful responses
			logEntry.Debug(e.name)
		} else if (responseStatusCode >= 300 && responseStatusCode <= 399){
			// redirection messages
			logEntry.Debug(e.name)
		} else if (responseStatusCode >= 400 && responseStatusCode <= 499){
			// client error responses
			logEntry.Error(e.name)
		} else if (responseStatusCode >= 500 && responseStatusCode <= 599){
			// server error responses
			logEntry.Error(e.name)
		} else {
			logEntry.Info(e.name)
		}
	})
}
@anar-khalilov
Copy link
Contributor Author

We would be super happy to receive feedback about this one because this is really important for us.

@pkqk
Copy link
Member

pkqk commented Nov 7, 2022

Hi @anar-khalilov, thanks for your PR.

I can understand your need to not log all downstream requests, we'll have to consider a general solution rather than one that just fits your use case though.

Are you ok running a fork for now while we consider this?

Things to consider:

  • Should this be configurable
  • Log levels, error should be for gateway errors, not downstream responses, so perhaps warning makes more sense
  • We could make the instrumentation middleware customisable

@pkqk pkqk added the enhancement New feature or request label Nov 7, 2022
@pkqk pkqk self-assigned this Nov 7, 2022
@anar-khalilov
Copy link
Contributor Author

Thanks for comprehensive response. We are currently unable to run a fork due to corporate policies. Because of that we are wondering if this issue has been planned/started etc.

@pkqk
Copy link
Member

pkqk commented Dec 4, 2022

Hi @anar-khalilov, it's not on our roadmap yet. If you are unable to run a fork, are you able to put a filter in your log collection that drops the unneeded records?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants