movio / bramble

A federated GraphQL API gateway
https://movio.github.io/bramble/
MIT License
497 stars 53 forks source link

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

Open anar-khalilov opened 1 year ago

anar-khalilov commented 1 year ago

https://github.com/movio/bramble/pull/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 commented 1 year ago

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

pkqk commented 1 year ago

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:

anar-khalilov commented 1 year ago

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 commented 1 year ago

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?