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.

Sign up for GitHub

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

Jump to bottom

Minor perf improvements.. #1170

Merged
merged 5 commits into from
Jul 16, 2019
Merged

Minor perf improvements.. #1170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1acabf1
Minor perf improvements..
cijothomas Jul 15, 2019
0ff4692
correct link
cijothomas Jul 15, 2019
23e918b
Merge branch 'develop' into cithomas/minor1
cijothomas Jul 15, 2019
eb79750
move ikey logic closer to actual usage
cijothomas Jul 15, 2019
8a65bc1
Merge branch 'cithomas/minor1' of https://github.com/microsoft/Applic…
cijothomas Jul 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This changelog will be used to generate documentation on [release notes page](ht

## VNext
- [Fix: Emit warning if user sets both Sampling IncludedTypes and ExcludedTypes. Excluded will take precedence.](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1166)
- [Minor perf improvement by reading Actity.Tag only if required.](https://github.com/microsoft/ApplicationInsights-dotnet/pull/1170)

## Version 2.11.0-beta1
- [Performance fixes: Support Head Sampling; Remove NewGuid(); Sampling Flags; etc... ](https://github.com/microsoft/ApplicationInsights-dotnet/pull/1158)
Expand Down
10 changes: 6 additions & 4 deletions src/Microsoft.ApplicationInsights/Extensibility/OperationCorrelationTelemetryInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ public void Initialize(ITelemetry telemetryItem)
}
}

string operationName = currentActivity.GetOperationName();

if (string.IsNullOrEmpty(itemContext.Name) && !string.IsNullOrEmpty(operationName))
if (string.IsNullOrEmpty(itemContext.Name))
{
itemContext.Name = operationName;
string operationName = currentActivity.GetOperationName();
if (!string.IsNullOrEmpty(operationName))
{
itemContext.Name = operationName;
}
}
}
});
Expand Down
14 changes: 7 additions & 7 deletions src/Microsoft.ApplicationInsights/TelemetryClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,13 +478,6 @@ public void InitializeInstrumentationKey(ITelemetry telemetry)
[EditorBrowsable(EditorBrowsableState.Never)]
public void Initialize(ITelemetry telemetry)
{
string instrumentationKey = this.Context.InstrumentationKey;

if (string.IsNullOrEmpty(instrumentationKey))
{
instrumentationKey = this.configuration.InstrumentationKey;
}

ISupportAdvancedSampling telemetryWithSampling = telemetry as ISupportAdvancedSampling;

// Telemetry can be already sampled out if that decision was made before calling Track()
Expand Down Expand Up @@ -520,6 +513,13 @@ public void Initialize(ITelemetry telemetry)
Utils.CopyDictionary(this.Context.GlobalProperties, telemetry.Context.GlobalProperties);
}

string instrumentationKey = this.Context.InstrumentationKey;

if (string.IsNullOrEmpty(instrumentationKey))
{
instrumentationKey = this.configuration.InstrumentationKey;
Copy link
Member

Choose a reason for hiding this comment

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

I remember keeping this assignment outside to make sure that proactively sampled out item won't get "Invalid Ikey" or won't get dropped before it arrives to sampling for gain up calculation in Sampling Telemetry Processor. If we are sure that items make it all the way without IKey - this change is OK. There was a case where setting empty IKey prevented all telemetry from leaving SDK (I may have false memory here, though).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lines i moved into the if block will not have helped with the issue. It just assigns ikey to a local variable, which is ONLY used inside the if block. I just moved the assignment closer to actual usage, in the same if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/microsoft/ApplicationInsights-aspnetcore/blob/develop/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs#L513 This initialization is probably what you are referring to. That assignment is outside of any Sampling If-Else statements, and is correctly done so,

}

telemetry.Context.Initialize(this.Context, instrumentationKey);

for (int index = 0; index < this.configuration.TelemetryInitializers.Count; index++)
Expand Down