Conversation
felipemadero
requested review from
cwend-ava,
jasonatran,
learyce,
meaghanfitzgerald and
sukantoraymond
| if telemetryToken == "" && !utils.IsE2E() { | ||
| app.Log.Warn("no token is configured for sending metrics") | ||
| } |
There was a problem hiding this comment.
do we need to users to see this?
There was a problem hiding this comment.
this is only going into avalanche.log, not stdout. it is useful for debugging. not expected to be printed on releases.
| client, err := posthog.NewWithConfig(telemetryToken, posthog.Config{Endpoint: telemetryInstance}) | ||
| if err != nil { | ||
| app.Log.Warn(fmt.Sprintf("failure creating metrics client: %s", err)) | ||
| } |
There was a problem hiding this comment.
same here, why not just print nothing to logs?
There was a problem hiding this comment.
this is only going into avalanche.log, not stdout. we are seeing that not all events seem to be sent, so we need to have some logs on this, so we can see if something is happening and maybe try again based on the error
| if err := client.Enqueue(event); err != nil { | ||
| app.Log.Warn(fmt.Sprintf("failure sending metrics %#v: %s", telemetryProperties, err)) | ||
| } | ||
| if err := client.Close(); err != nil { | ||
| app.Log.Warn(fmt.Sprintf("failure closing metrics client %#v: %s", telemetryProperties, err)) |
There was a problem hiding this comment.
this is only going into avalanche.log, not stdout. we are seeing that not all events seem to be sent, so we need to have some logs on this, so we can see if something is happening and maybe try again based on the error
| flags map[string]string, | ||
| err error, | ||
| ) { | ||
| if sent { |
There was a problem hiding this comment.
if user calls command 1, sent will be set to true after tracking info is sent. What about when user calls command 2, since sent would have been set to true here, will the tracking info be sent?
There was a problem hiding this comment.
I guess best way to test this is see if we can see the second command on the dashboard after first command is called
There was a problem hiding this comment.
that variable is set per execution. a new execution of CLI with starts with the value set to false.
3 participants
Why this should be merged
Current CLI metrics don't collect information on failed commands. This PR add err message collection
in a new field, error.
How this works
How this was tested
How is this documented