Skip to content

Add possibility to send telemetry events by data collectors#4622

Merged
jakubch1 merged 13 commits into
microsoft:mainfrom
jakubch1:dev/jachocho/telemetry2
Jul 31, 2023
Merged

Add possibility to send telemetry events by data collectors#4622
jakubch1 merged 13 commits into
microsoft:mainfrom
jakubch1:dev/jachocho/telemetry2

Conversation

@jakubch1

Copy link
Copy Markdown
Member

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.

  • I have ensured that there is a previously discussed and approved issue.
@jakubch1 jakubch1 changed the title [draft] telemetry 2 Add possibility to send telemetry events by data collectors Jul 28, 2023
@jakubch1 jakubch1 marked this pull request as ready for review July 28, 2023 13:07

@cvpoienaru cvpoienaru left a comment

Copy link
Copy Markdown
Member

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);

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member Author

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)

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Member Author

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

@MarcoRossignoli MarcoRossignoli left a comment

Copy link
Copy Markdown
Contributor

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

This was referenced Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants