-
Notifications
You must be signed in to change notification settings - Fork 323
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
Add possibility to send telemetry events by data collectors #4622
Conversation
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.
LGTM
/// <summary> | ||
/// Initializes telemetry reporter | ||
/// </summary> | ||
void Initialize(ITelemetryReporter telemetryReporter); |
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 wonder if we should have an option object for future extensibility but maybe we can add to the ITelemetryReporter
self if needed
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 think extending API for extensions is not issue anymore.
/// </summary> | ||
/// <param name="name">Telemetry event name</param> | ||
/// <param name="properties">Telemetry event properties</param> | ||
public TelemetryEvent(string name, IDictionary<string, object> properties) |
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.
a bit generic but underneath we don't have more than that
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.
this is API used by VS 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.
some comment but I don't know if we can do better
Description
Please add a meaningful description for this change.
Ensure the PR has required unit tests.
Related issue
Kindly link any related issues. E.g. Fixes #xyz.