Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/// <see cref="ITelemetryInitializer"/> implementation that stamps ASP.NET Core environment name
/// on telemetries.
/// </summary>
internal class AspNetCoreEnvironmentTelemetryInitializer: ITelemetryInitializer
public class AspNetCoreEnvironmentTelemetryInitializer: ITelemetryInitializer
Copy link
Copy Markdown
Member

@pharring pharring Feb 28, 2018

Choose a reason for hiding this comment

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

(This comment applies to all the newly public classes.)
Should these types be sealed? It's usually a good idea with public types that were not designed to be extended by users - especially if they have any virtual methods.
You can always unseal a public type later, but you can't go the other way 'round.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree it can be made sealed. However, similar ones in Web repo are not sealed.

https://github.com/Microsoft/ApplicationInsights-dotnet-server/blob/develop/Src/Web/Web.Shared.Net/SessionTelemetryInitializer.cs#L12

I'll limit my change to just making classes public for now. When we address (#612) this can be revisited.

{
private const string AspNetCoreEnvironmentPropertyName = "AspNetCoreEnvironment";
private readonly IHostingEnvironment environment;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/// <summary>
/// A telemetry initializer that will gather Azure Web App Role Environment context information.
/// </summary>
internal class AzureWebAppRoleEnvironmentTelemetryInitializer : TelemetryInitializerBase
public class AzureWebAppRoleEnvironmentTelemetryInitializer : TelemetryInitializerBase
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.

My comment about sealing applies more-so to those derived from TelemetryInitializerBase

{
/// <summary>Azure Web App name corresponding to the resource name.</summary>
private const string WebAppNameEnvironmentVariable = "WEBSITE_SITE_NAME";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/// <summary>
/// This telemetry initializer extracts client IP address and populates telemetry.Context.Location.Ip property.
/// </summary>
internal class ClientIpHeaderTelemetryInitializer : TelemetryInitializerBase
public class ClientIpHeaderTelemetryInitializer : TelemetryInitializerBase
{
private const string HeaderNameDefault = "X-Forwarded-For";
private readonly char[] headerValuesSeparatorDefault = { ',' };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/// <summary>
/// A telemetry initializer that populates cloud context role instance.
/// </summary>
internal class DomainNameRoleInstanceTelemetryInitializer : TelemetryInitializerBase
public class DomainNameRoleInstanceTelemetryInitializer : TelemetryInitializerBase
{
private string roleInstanceName;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.AspNetCore.Http;

internal class OperationNameTelemetryInitializer : TelemetryInitializerBase
public class OperationNameTelemetryInitializer : TelemetryInitializerBase
{
public OperationNameTelemetryInitializer(IHttpContextAccessor httpContextAccessor) : base(httpContextAccessor)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/// <summary>
/// This will allow to mark synthetic traffic from availability tests
/// </summary>
internal class SyntheticTelemetryInitializer : TelemetryInitializerBase
public class SyntheticTelemetryInitializer : TelemetryInitializerBase
{
private const string SyntheticTestRunId = "SyntheticTest-RunId";
private const string SyntheticTestLocation = "SyntheticTest-Location";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;

internal abstract class TelemetryInitializerBase : ITelemetryInitializer
public abstract class TelemetryInitializerBase : ITelemetryInitializer
{
private IHttpContextAccessor httpContextAccessor;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.AspNetCore.Http;

internal class WebSessionTelemetryInitializer : TelemetryInitializerBase
public class WebSessionTelemetryInitializer : TelemetryInitializerBase
{
private const string WebSessionCookieName = "ai_session";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.AspNetCore.Http;

internal class WebUserTelemetryInitializer : TelemetryInitializerBase
public class WebUserTelemetryInitializer : TelemetryInitializerBase
{
private const string WebUserCookieName = "ai_user";

Expand Down