Skip to content
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

Send assembly references to Telemetry #493

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

yongyan-gh
Copy link
Collaborator

Read assembly references from managed assemblies and send to telemetry by chunks.

@yongyan-gh yongyan-gh marked this pull request as ready for review September 29, 2021 00:11
private const string SummaryEventName = "AnalysisSummary";

private readonly int ChunkSize = 8192;
Copy link
Contributor

@eddynaka eddynaka Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8192

u can remove this, since u are using the value from the constructor #Closed

private const string CompilerEventName = "CompilerInformation";
private const string CommandLineEventName = "CommandLineInformation";
private const string AssemblyReferencesEventName = "AssemblyReferencesInformation";
private const string SummaryEventName = "AnalysisSummary";
Copy link
Contributor

@eddynaka eddynaka Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SummaryEventName

move this to the first, so we will have ordered by length #Closed

{
Initialize(appInsightsKey);
this.appInsightsRegistered = true;

Copy link
Contributor

@eddynaka eddynaka Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can u remove this new line? #Closed

s_sessionId = TelemetryEnabled ? Guid.NewGuid().ToString() : null;
ChunkSize = chunkSize;

if (!TelemetryEnabled)
Copy link
Collaborator

@shaopeng-gh shaopeng-gh Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!TelemetryEnabled)

question, why we add this new condition #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code in below block is to initialize a TelemetryClient instance to be used to send telemetry data.
if caller already passed in the TelemetryClient instance in constructor, we don't need to initialize a new instance.
Added the TelemetryClient / Configuration as constructor parameter so that we can overwrite the built-in instances. The unit tests can pass in mock client/configuration for testing purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, since we are creating the compilerdatalogger for each object, we dont need to keep creating an instance of the telemetry.

with that, once it is initialized, it won't create again

Copy link
Collaborator

@shaopeng-gh shaopeng-gh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@eddynaka eddynaka merged commit 034bd5c into main Sep 29, 2021
@eddynaka eddynaka deleted the users/yongyan-gh/sendAssemblyReferences branch September 29, 2021 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants