Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Use Datadog reserved Status field, send error messages if no added fields #19

Merged
merged 5 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/analytics/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var (
)

var datadogLogLevel = "_logLevel"
var datadogStatus = "status"
Copy link
Contributor

Choose a reason for hiding this comment

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

So how does status differ from _logLevel? From the docs, it seems like `status is the correct one to use so why send both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
image

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we switch our usage of _loglevel to status? I don't think we really care about the levels it doesn't support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


func NewClient(properties map[string]interface{}) (*Client, error) {
result, err := getTrackingFileContents(analyticsFile)
Expand Down Expand Up @@ -78,16 +79,20 @@ func (t *Client) Debug(event string) {

func (t *Client) Warn(event string) {
t.Properties[datadogLogLevel] = Warn
t.Properties[datadogStatus] = Warn
Copy link
Contributor

Choose a reason for hiding this comment

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

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

t.track(event)
}

func (t *Client) Error(event string) {
t.Properties[datadogLogLevel] = Error
t.Properties[datadogStatus] = Error
t.track(event)
}

func (t *Client) Panic(event string) {
t.Properties[datadogLogLevel] = Panic
// Using error since datadog does not support panic forr the reserved status field
t.Properties[datadogStatus] = Error
t.track(event)
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/analytics/fields_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,14 @@ func (fl *fieldListener) Write(entry zapcore.Entry, fields []zapcore.Field) erro
allFields = append(allFields, fl.fields...)
allFields = append(allFields, fields...)

safeLogsStr := logging.SanitizeFields(allFields, fl.client.Hash)
var safeLogsStr string

if len(allFields) != 0 {
safeLogsStr = logging.SanitizeFields(allFields, fl.client.Hash)
} else {
safeLogsStr = entry.Message
}

switch entry.Level {
case zapcore.DebugLevel:
fl.client.Debug(safeLogsStr)
Expand Down