-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Preview pane telemetry #1299
Preview pane telemetry #1299
Changes from all commits
63ad37d
34b1edd
6f11a33
7681fdc
d7f698e
d4fe2fa
fbbb99f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Copyright (c) Microsoft Corporation | ||
// The Microsoft Corporation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Diagnostics.Tracing; | ||
using PreviewHandlerCommon.Telemetry; | ||
|
||
namespace MarkdownPreviewHandler | ||
{ | ||
/// <summary> | ||
/// Telemetry helper class for markdown renderer. | ||
/// </summary> | ||
public class MarkdownTelemetry : TelemetryBase | ||
{ | ||
/// <summary> | ||
/// Name for ETW event. | ||
/// </summary> | ||
private const string EventSourceName = "Microsoft.PowerToys"; | ||
|
||
/// <summary> | ||
/// ETW event name when markdown is previewed. | ||
/// </summary> | ||
private const string MarkdownFilePreviewedEventName = "PowerPreview_MDRenderer_Previewed"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks making the changes! Looks good to me. Do you think we can make the similar change to the settings.cpp file (NOT require since it is out of the scope of the PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am more inclined of putting it in a separate PR. This PR is focused on C# telemetry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good!!! |
||
|
||
/// <summary> | ||
/// ETW event name when error is thrown during markdown preview. | ||
/// </summary> | ||
private const string MarkdownFilePreviewErrorEventName = "PowerPreview_MDRenderer_Error"; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="MarkdownTelemetry"/> class. | ||
/// </summary> | ||
public MarkdownTelemetry() | ||
: base(EventSourceName) | ||
{ | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was this intended? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! |
||
} | ||
|
||
/// <summary> | ||
/// Gets an instance of the <see cref="MarkdownTelemetry"/> class. | ||
/// </summary> | ||
public static MarkdownTelemetry Log { get; } = new MarkdownTelemetry(); | ||
|
||
/// <summary> | ||
/// Publishes ETW event when markdown is previewed successfully. | ||
/// </summary> | ||
public void MarkdownFilePreviewed() | ||
{ | ||
this.Write(MarkdownFilePreviewedEventName, new EventSourceOptions() | ||
{ | ||
Keywords = ProjectKeywordMeasure, | ||
Tags = ProjectTelemetryTagProductAndServicePerformance, | ||
}); | ||
} | ||
|
||
/// <summary> | ||
/// Publishes ETW event when markdown could not be previewed. | ||
/// </summary> | ||
public void MarkdownFilePreviewError(string message) | ||
{ | ||
this.Write( | ||
MarkdownFilePreviewErrorEventName, | ||
new EventSourceOptions() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't seem to be changing, perhaps we can store it as a global variable somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! I will update this. |
||
{ | ||
Keywords = ProjectKeywordMeasure, | ||
Tags = ProjectTelemetryTagProductAndServicePerformance, | ||
}, | ||
new { Message = message, }); | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
// Copyright (c) Microsoft Corporation | ||
// The Microsoft Corporation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Diagnostics.Eventing.Reader; | ||
using System.Diagnostics.Tracing; | ||
|
||
namespace PreviewHandlerCommon.Telemetry | ||
{ | ||
/// <summary> | ||
/// Base class for telemetry events. | ||
/// </summary> | ||
public class TelemetryBase : EventSource | ||
{ | ||
/// <summary> | ||
/// The event tag for this event source. | ||
/// </summary> | ||
public const EventTags ProjectTelemetryTagProductAndServicePerformance = (EventTags)0x0u; | ||
|
||
/// <summary> | ||
/// The event keyword for this event source. | ||
/// </summary> | ||
public const EventKeywords ProjectKeywordMeasure = (EventKeywords)0x0; | ||
|
||
/// <summary> | ||
/// Group ID for Powertoys project. | ||
/// </summary> | ||
private static readonly string[] PowerToysTelemetryTraits = { "ETW_GROUP", "{42749043-438c-46a2-82be-c6cbeb192ff2}" }; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="TelemetryBase"/> class. | ||
/// </summary> | ||
/// <param name="eventSourceName">.</param> | ||
public TelemetryBase( | ||
string eventSourceName) | ||
: base( | ||
eventSourceName, | ||
EventSourceSettings.EtwSelfDescribingEventFormat, | ||
PowerToysTelemetryTraits) | ||
{ | ||
return; | ||
} | ||
} | ||
} |
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.
nit: maybe this could be in the base :) ?
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.
It's possible that same projects can have multiple event sources. That's why it's better to initialize them from the derived classes.