Use Datadog reserved Status field, send error messages if no added fields #19
Conversation
| @@ -78,16 +79,20 @@ func (t *Client) Debug(event string) { | |||
|
|
|||
| func (t *Client) Warn(event string) { | |||
| t.Properties[datadogLogLevel] = Warn | |||
| t.Properties[datadogStatus] = Warn | |||
There was a problem hiding this comment.
were we always sending the logs, just classifying them as Info? I thought we send the "klotho compiling failed" as info, but also with the error logs and thats what was also missing
pkg/logging/fields.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func SendEntryMessage() zap.Field { |
There was a problem hiding this comment.
Can you add some godoc for this explaining what it does / when to use it?
Also, this could be simplified I think to just
var SendEntryMessage = zap.Object("entryMessage", struct{}{})(and checking f == SendEntryMessage in the listener instead of the Key)
| @@ -34,6 +34,7 @@ var ( | |||
| ) | |||
|
|
|||
| var datadogLogLevel = "_logLevel" | |||
| var datadogStatus = "status" | |||
There was a problem hiding this comment.
So how does status differ from _logLevel? From the docs, it seems like `status is the correct one to use so why send both?
There was a problem hiding this comment.
yeh I was confused by this in their docs initially also. So _loglevel allows you to specify whatever level you want and you can filter on it from the DD console. The status one though only allows the pre-defined (info, warn, error) and we can't modify that, but that causes the colorized fields in DD
So it's not entirely necessary since we filter on the _loglevel, but it gives you a quicker at a glance view since it will color the actual log entries instead of treating everything as info and blue by default.
There was a problem hiding this comment.
Can we switch our usage of _loglevel to status? I don't think we really care about the levels it doesn't support.
There was a problem hiding this comment.
we could, but it doesn't have things like debug/panic which currently i set to the closest relevant so debug -> info and panic -> error. If we're ok with losing granularity I can change that to just use status, but it didn't seem like we needed to consolidate and afaik it doesnt change costs in any way either so i left both.
|
fixed some minor go linting errors
This is for #6
Theres 2 changes here
info, warn, errorso anything aboveerrorwe just useerrorWe might want to chat about the second point. My memory is kind of fuzzy on this but I believe yuval did a pass on error messages and added the sanitization process for things that touch user code because we didn't want to leak anything via error messages. This change would mean that if we forget to add the zap field to sanitize user code info in a plugin or something, we would just send the error message.
Standard checks