-
Notifications
You must be signed in to change notification settings - Fork 452
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
Telemetry: Record when a mount succesfully starts #437
Conversation
@@ -141,7 +141,8 @@ public void Mount(EventLevel verbosity, Keywords keywords) | |||
// Use TracingConstants.MessageKey.InfoMessage rather than TracingConstants.MessageKey.CriticalMessage | |||
// as this message should not appear as an error | |||
{ TracingConstants.MessageKey.InfoMessage, "Virtual repo is ready" }, | |||
}); | |||
}, | |||
Keywords.Telemetry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this still be EventLevel.Critical
, or can we make this Info level (see line 137)?
CC: @sanoursa. Saeed, do you remember if we perviously got push back for Critical + Telemetry events?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed this with @sanoursa offline and he agreed this should no longer be Critical
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @wilbaker , I updated as suggested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update event so that it's no longer Critical
(and confirm that it's still being recorded properly, even at Info level).
13b72ec
to
76ee853
Compare
Add a telemetry marker so we can tell if a mount successfully came up