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

Replace consts with static readonly #428

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion src/CodeGenerators/SettingsGen.Example/Settings.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<Settings xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns="http://schemas.microsoft.com/2011/01/fabric">
<Settings xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://schemas.microsoft.com/2011/01/fabric">
<Section Name="NewSection">
<Parameter Name="Whatever" Value="" />
<Parameter Name="TestingThis" Value="Hmmmm" />
Expand Down
44 changes: 22 additions & 22 deletions src/Extensions/Abstractions/Activities/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ public static class ActivityExtensions
public static Guid? GetRootIdAsGuid(this Activity activity) =>
Guid.TryParse(activity.RootId, out Guid result)
? result
: (Guid?)null;
: null;

/// <summary>
/// Get user hash from activity
/// </summary>
public static string GetUserHash(this Activity activity) =>
activity.GetBaggageItem(UserHashKey) ?? string.Empty;
activity.GetBaggageItem(s_userHashKey) ?? string.Empty;

/// <summary>
/// Returns true if activity is transaction
/// </summary>
public static bool IsHealthCheck(this Activity activity) =>
string.Equals(
activity.GetBaggageItem(HealthCheckMarkerKey),
HealthCheckMarkerValue,
activity.GetBaggageItem(s_healthCheckMarkerKey),
s_healthCheckMarkerValue,
StringComparison.OrdinalIgnoreCase);

/// <summary>
Expand All @@ -45,16 +45,16 @@ public static bool IsHealthCheck(this Activity activity) =>
/// <remarks>Currently added to the BaggageItems (via Kestrel) using header value Correlation-Context: "PerformanceTestMarker=true"</remarks>
public static bool IsPerformanceTest(this Activity activity) =>
string.Equals(
activity.GetBaggageItem(PerformanceMarkerKey),
PerformanceMarkerValue,
activity.GetBaggageItem(s_performanceMarkerKey),
s_performanceMarkerValue,
StringComparison.OrdinalIgnoreCase);

/// <summary>
/// Set user hash for the activity
/// </summary>
/// <remarks>This property would be transfered to child activity and via web requests</remarks>
public static Activity SetUserHash(this Activity activity, string userHash) =>
activity.SetBaggage(UserHashKey, userHash);
activity.SetBaggage(s_userHashKey, userHash);

/// <summary>
/// Mark as health check activity
Expand All @@ -63,7 +63,7 @@ public static Activity SetUserHash(this Activity activity, string userHash) =>
public static Activity MarkAsHealthCheck(this Activity activity) =>
activity.IsHealthCheck()
? activity
: activity.SetBaggage(HealthCheckMarkerKey, HealthCheckMarkerValue);
: activity.SetBaggage(s_healthCheckMarkerKey, s_healthCheckMarkerValue);

/// <summary>
/// Set result
Expand Down Expand Up @@ -109,42 +109,42 @@ public static Activity SetMetadata(this Activity activity, string metadata) =>
/// </summary>
[Obsolete(CorrelationIdObsoleteMessage, false)]
public static Guid? GetObsoleteCorrelationId(this Activity activity) =>
Guid.TryParse(activity.GetBaggageItem(ObsoleteCorrelationId), out Guid correlation)
Guid.TryParse(activity.GetBaggageItem(s_obsoleteCorrelationId), out Guid correlation)
? correlation
: (Guid?)null;
: null;

/// <summary>
/// Set correlation guid that is used by old Omex services
/// </summary>
/// <remarks>This property would be transfered to child activity and via web requests</remarks>
[Obsolete(CorrelationIdObsoleteMessage, false)]
public static Activity SetObsoleteCorrelationId(this Activity activity, Guid correlation) =>
activity.SetBaggage(ObsoleteCorrelationId, correlation.ToString());
activity.SetBaggage(s_obsoleteCorrelationId, correlation.ToString());

/// <summary>
/// Get transaction id that is used by old Omex services
/// </summary>
[Obsolete(TransactionIdObsoleteMessage, false)]
public static uint? GetObsoleteTransactionId(this Activity activity) =>
uint.TryParse(activity.GetBaggageItem(ObsoleteTransactionId), out uint transactionId)
uint.TryParse(activity.GetBaggageItem(s_obsoleteTransactionId), out uint transactionId)
? transactionId
: (uint?)null;
: null;

/// <summary>
/// Set transaction id that is used by old Omex services
/// </summary>
/// <remarks>This property would be transfered to child activity and via web requests</remarks>
[Obsolete(TransactionIdObsoleteMessage, false)]
public static Activity SetObsoleteTransactionId(this Activity activity, uint transactionId) =>
activity.SetBaggage(ObsoleteTransactionId, transactionId.ToString(CultureInfo.InvariantCulture));

private const string UserHashKey = "UserHash";
private const string HealthCheckMarkerKey = "HealthCheckMarker";
private const string HealthCheckMarkerValue = "true";
private const string PerformanceMarkerKey = "PerformanceTestMarker";
private const string PerformanceMarkerValue = "true";
private const string ObsoleteCorrelationId = "ObsoleteCorrelationId";
private const string ObsoleteTransactionId = "ObsoleteTransactionId";
activity.SetBaggage(s_obsoleteTransactionId, transactionId.ToString(CultureInfo.InvariantCulture));

private static readonly string s_userHashKey = "UserHash";
private static readonly string s_healthCheckMarkerKey = "HealthCheckMarker";
private static readonly string s_healthCheckMarkerValue = "true";
private static readonly string s_performanceMarkerKey = "PerformanceTestMarker";
private static readonly string s_performanceMarkerValue = "true";
private static readonly string s_obsoleteCorrelationId = "ObsoleteCorrelationId";
private static readonly string s_obsoleteTransactionId = "ObsoleteTransactionId";
private const string CorrelationIdObsoleteMessage = "Please use Activity.Id or Activity.GetRootIdAsGuid() for new services instead";
private const string TransactionIdObsoleteMessage = "Please use Activity.IsHealthCheck() for new services instead";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ namespace Microsoft.Omex.Extensions.Abstractions.ExecutionContext
public class BaseExecutionContext : IExecutionContext
{
// Defined by Azure https://whatazurewebsiteenvironmentvariablesareavailable.azurewebsites.net/
internal const string RegionNameVariableName = "REGION_NAME";
internal static readonly string RegionNameVariableName = "REGION_NAME";

// We define them
internal const string ClusterNameVariableName = "CLUSTER_NAME";
internal const string SliceNameVariableName = "SLICE_NAME";
internal const string AspNetCoreEnviromentVariableName = "ASPNETCORE_ENVIRONMENT";
internal const string DotNetEnviromentVariableName = "DOTNET_ENVIRONMENT"; // getting environment directly only if we don't have IHostEnvironment ex. InitializationLogger
internal static readonly string ClusterNameVariableName = "CLUSTER_NAME";
internal static readonly string SliceNameVariableName = "SLICE_NAME";
internal static readonly string AspNetCoreEnviromentVariableName = "ASPNETCORE_ENVIRONMENT";
internal static readonly string DotNetEnviromentVariableName = "DOTNET_ENVIRONMENT"; // getting environment directly only if we don't have IHostEnvironment ex. InitializationLogger

// defined by Service Fabric https://docs.microsoft.com/en-us/azure/service-fabric/service-fabric-environment-variables-reference
internal const string ServiceNameVariableName = "Fabric_ServiceName";
internal const string ApplicationNameVariableName = "Fabric_ApplicationName";
internal const string NodeNameVariableName = "Fabric_NodeName";
internal const string NodeIPOrFQDNVariableName = "Fabric_NodeIPOrFQDN";
internal static readonly string ServiceNameVariableName = "Fabric_ServiceName";
internal static readonly string ApplicationNameVariableName = "Fabric_ApplicationName";
internal static readonly string NodeNameVariableName = "Fabric_NodeName";
internal static readonly string NodeIPOrFQDNVariableName = "Fabric_NodeIPOrFQDN";

/// <summary>
/// Create instance of execution context
Expand Down
4 changes: 2 additions & 2 deletions src/Extensions/Abstractions/SfConfigurationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ namespace Microsoft.Omex.Extensions.Abstractions
public class SfConfigurationProvider
{
/// SF environment variables taken from https://docs.microsoft.com/en-us/azure/service-fabric/service-fabric-environment-variables-reference
internal const string PublishAddressEvnVariableName = "Fabric_NodeIPOrFQDN";
internal static readonly string PublishAddressEvnVariableName = "Fabric_NodeIPOrFQDN";

internal const string EndpointPortEvnVariableSuffix = "Fabric_Endpoint_";
internal static readonly string EndpointPortEvnVariableSuffix = "Fabric_Endpoint_";

/// <summary>
/// Get Service Fabric Variable from environment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ public void SendActivityMetric(Activity activity)
string userHash = activity.GetUserHash();
bool isHealthCheck = activity.IsHealthCheck();

string subtype = NullPlaceholder;
string metadata = NullPlaceholder;
string resultAsString = NullPlaceholder;
string subtype = s_nullPlaceholder;
string metadata = s_nullPlaceholder;
string resultAsString = s_nullPlaceholder;
foreach (KeyValuePair<string, string?> pair in activity.Tags)
{
if (pair.Value == null)
Expand All @@ -63,7 +63,7 @@ public void SendActivityMetric(Activity activity)
#pragma warning disable CS0618 // Until it's used we need to include correlationId into events
string correlationId = activity.GetObsoleteCorrelationId()?.ToString()
?? activity.GetRootIdAsGuid()?.ToString()
?? NullPlaceholder;
?? s_nullPlaceholder;
#pragma warning restore CS0618

string nameAsString = SanitizeString(name, nameof(name), name);
Expand Down Expand Up @@ -116,13 +116,13 @@ private string SanitizeString(string value, string name, string activityName)
return value;
}

private const string StringLimitMessage =
private static readonly string StringLimitMessage =
"Log aggregation enforces a string length limit of {0} characters per dimension. Truncating length of dimension {1} on activity {2} from {3} chars in order to allow upload of the metric";

private readonly ActivityEventSource m_eventSource;
private readonly string m_serviceName;
private readonly ILogger<ActivityEventSender> m_logger;
private static readonly string s_logCategory = typeof(ActivityEventSource).FullName ?? nameof(ActivityEventSource);
private const string NullPlaceholder = "null";
private static readonly string s_nullPlaceholder = "null";
}
}
9 changes: 3 additions & 6 deletions src/Extensions/Activities/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Collections.Generic;
using System.Diagnostics;
using System.Text;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.ObjectPool;
using Microsoft.Omex.Extensions.Abstractions.Activities.Processing;
using Microsoft.Omex.Extensions.Abstractions.ExecutionContext;
Expand All @@ -18,8 +15,8 @@ namespace Microsoft.Extensions.DependencyInjection
/// </summary>
public static class ServiceCollectionExtensions
{
private const string ActivitySourceName = "OmexActivitySource";
private const string ActivitySourceVersion = "1.0.0.0";
private static readonly string s_activitySourceName = "OmexActivitySource";
Copy link
Member

@K-Cully K-Cully Nov 9, 2021

Choose a reason for hiding this comment

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

This class is already static so I expect these shouldn't be changed.

private static readonly string s_activitySourceVersion = "1.0.0.0";

/// <summary>
/// Add ActivitySource to ServiceCollection
Expand All @@ -44,7 +41,7 @@ public static IServiceCollection AddOmexActivitySource(this IServiceCollection s
serviceCollection.TryAddSingleton<IActivitiesEventSender, AggregatedActivitiesEventSender>();

serviceCollection.TryAddSingleton<IActivityListenerConfigurator, DefaultActivityListenerConfigurator>();
serviceCollection.TryAddSingleton(p => new ActivitySource(ActivitySourceName, ActivitySourceVersion));
serviceCollection.TryAddSingleton(p => new ActivitySource(s_activitySourceName, s_activitySourceVersion));
serviceCollection.TryAddSingleton(p => ActivityEventSource.Instance);

return serviceCollection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal class OmexHealthCheckPublisher : IHealthCheckPublisher

private IHealthStatusSender StatusSender { get; }

internal const string HealthReportSummaryProperty = "HealthReportSummary";
internal static readonly string HealthReportSummaryProperty = "HealthReportSummary";

public OmexHealthCheckPublisher(IHealthStatusSender statusSender, ObjectPoolProvider objectPoolProvider)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.Omex.Extensions.Diagnostics.HealthChecks
{
internal class RestHealthStatusSender : IHealthStatusSender
{
private const string HealthReportSourceId = nameof(RestHealthStatusSender);
private static readonly string s_healthReportSourceId = nameof(RestHealthStatusSender);

private readonly IServiceFabricClientWrapper m_clientWrapper;

Expand Down Expand Up @@ -44,7 +44,7 @@ public async Task SendStatusAsync(string checkName, HealthStatus status, string

await m_client.Nodes.ReportNodeHealthAsync(
nodeName: m_nodeName,
healthInformation: new HealthInformation(HealthReportSourceId, checkName, ToSfHealthState(status), description: description));
healthInformation: new HealthInformation(s_healthReportSourceId, checkName, ToSfHealthState(status), description: description));
}

private HealthState ToSfHealthState(HealthStatus healthStatus) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.Omex.Extensions.Diagnostics.HealthChecks
{
internal class ServiceContextHealthStatusSender : IHealthStatusSender
{
private const string HealthReportSourceId = nameof(ServiceContextHealthStatusSender);
private static readonly string s_healthReportSourceId = nameof(ServiceContextHealthStatusSender);

private readonly IAccessor<IServicePartition> m_partitionAccessor;

Expand Down Expand Up @@ -55,7 +55,7 @@ public Task SendStatusAsync(string checkName, HealthStatus status, string descri

try
{
m_reportHealth(new HealthInformation(HealthReportSourceId, checkName, ToSfHealthState(status))
m_reportHealth(new HealthInformation(s_healthReportSourceId, checkName, ToSfHealthState(status))
{
Description = description
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ private void ExtractCorrelationFromRequest(HttpRequest request)
!IsClientRequest(request)
&& Guid.TryParse(ExtractParameter(request.Query, s_correlationIdNames), out Guid correlation)
? correlation
: (Guid?)null;
: null;

private static Guid? ExtractCorrelationIdFromHeader(HttpRequest request) =>
!IsClientRequest(request)
&& Guid.TryParse(ExtractHeader(request.Headers, s_correlationIdNames), out Guid correlation)
? correlation
: (Guid?)null;
: null;

private static string? ExtractParameter(IQueryCollection dataSources, IEnumerable<string> names)
{
Expand Down Expand Up @@ -100,7 +100,7 @@ private void ExtractCorrelationFromRequest(HttpRequest request)
/// Checks if the context is for a request that contains identifiers indicating that the request originated from an Office client
/// </summary>
private static bool IsClientRequest(HttpRequest request) =>
request.Headers.ContainsKey(OfficeClientVersionHeader)
request.Headers.ContainsKey(s_officeClientVersionHeader)
? true
: request.Query.Count == 0 // Don't check empty parameters
? false
Expand All @@ -109,14 +109,18 @@ private static bool IsClientRequest(HttpRequest request) =>
private static uint? ParseUint(string? value) =>
uint.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out uint result)
? result
: (uint?)null;
: null;

private static readonly string s_correlationHeader = "X-CorrelationId";

private static readonly string s_officeClientVersionHeader = "X-Office-Version";

private static readonly string[] s_transactionsNames = {
"corrtid", // Correlation transaction query parameter
"X-TransactionId" };

private static readonly string[] s_correlationIdNames = {
CorrelationHeader,
s_correlationHeader,
"MS-CorrelationId", // Correlation Id header used by other Microsoft services
"corr", // Correlation query parameter
"HTTP_X_CORRELATIONID" };
Expand All @@ -125,9 +129,5 @@ private static bool IsClientRequest(HttpRequest request) =>
"client", // Identifies the client application and platform
"av", // Identifies the client application, platform and partial version
"app" }; // Identifies the client application

private const string CorrelationHeader = "X-CorrelationId";

private const string OfficeClientVersionHeader = "X-Office-Version";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ namespace Microsoft.Omex.Extensions.Logging.Internal.Replayable
{
internal static class ReplayebleActivityExtensions
{
private const string ReplayableLogKey = "ReplayableLogKey";
private static readonly string s_replayableLogKey = "ReplayableLogKey";
Copy link
Member

Choose a reason for hiding this comment

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

class is already static


private static ConcurrentQueue<LogMessageInformation>? GetReplayableLogsInternal(this Activity activity) =>
activity.GetCustomProperty(ReplayableLogKey) as ConcurrentQueue<LogMessageInformation>;
activity.GetCustomProperty(s_replayableLogKey) as ConcurrentQueue<LogMessageInformation>;

public static IEnumerable<LogMessageInformation> GetReplayableLogs(this Activity activity) =>
activity.GetReplayableLogsInternal() ?? Enumerable.Empty<LogMessageInformation>();
Expand All @@ -25,7 +25,7 @@ public static void AddReplayLog(this Activity activity, LogMessageInformation lo
if (queue == null)
{
queue = new ConcurrentQueue<LogMessageInformation>();
activity.SetCustomProperty(ReplayableLogKey, queue);
activity.SetCustomProperty(s_replayableLogKey, queue);
}

queue.Enqueue(logEvent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ internal static string DefaultFQDN()
return parts.Length < 2 ? DefaultServiceFabricClusterFQDN : string.Join(":", parts.Take(parts.Length - 1));
}

internal const string RuntimeConnectionAddressEvnVariableName = "Fabric_RuntimeConnectionAddress";
internal static readonly string RuntimeConnectionAddressEvnVariableName = "Fabric_RuntimeConnectionAddress";

internal const string DefaultServiceFabricClusterFQDN = "localhost";
internal static readonly string DefaultServiceFabricClusterFQDN = "localhost";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ internal class ServiceRemotingClientWrapper : IServiceRemotingClient
/// <remarks>
/// Should end with RequestOut to be enabled by our telemetry
/// </remarks>
private const string RequestOutActivitySuffix = "RequestOut";
private static readonly string s_requestOutActivitySuffix = "RequestOut";

private const string OneWayMessageActivityName = Diagnostics.DiagnosticListenerName + "OneWay" + RequestOutActivitySuffix;
private static readonly string s_oneWayMessageActivityName = Diagnostics.DiagnosticListenerName + "OneWay" + s_requestOutActivitySuffix;

private const string RequestActivityName = Diagnostics.DiagnosticListenerName + RequestOutActivitySuffix;
private static readonly string s_requestActivityName = Diagnostics.DiagnosticListenerName + s_requestOutActivitySuffix;

private readonly DiagnosticListener m_diagnosticListener;

Expand Down Expand Up @@ -58,7 +58,7 @@ public ResolvedServiceEndpoint Endpoint

public async Task<IServiceRemotingResponseMessage> RequestResponseAsync(IServiceRemotingRequestMessage requestMessage)
{
Activity? activity = m_diagnosticListener.CreateAndStartActivity(RequestActivityName);
Activity? activity = m_diagnosticListener.CreateAndStartActivity(s_requestActivityName);
requestMessage.AttachActivityToOutgoingRequest(activity);
IServiceRemotingResponseMessage? responseMessage = null;

Expand All @@ -82,7 +82,7 @@ public async Task<IServiceRemotingResponseMessage> RequestResponseAsync(IService

public void SendOneWay(IServiceRemotingRequestMessage requestMessage)
{
Activity? activity = m_diagnosticListener.CreateAndStartActivity(OneWayMessageActivityName);
Activity? activity = m_diagnosticListener.CreateAndStartActivity(s_oneWayMessageActivityName);
requestMessage.AttachActivityToOutgoingRequest(activity);

try
Expand Down
Loading