Skip to content

Commit 77649c3

Browse files
committed
Add analyzer to enforce ConfigurationBuilder only with configuration keys or platform keys
1 parent 2538f11 commit 77649c3

File tree

9 files changed

+248
-30
lines changed

9 files changed

+248
-30
lines changed

tracer/src/Datadog.Trace.BenchmarkDotNet/DatadogExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ internal static IConfig WithDatadog(this IConfig config, bool? enableProfiler)
3333
{
3434
var cfg = config.AddLogger(DatadogSessionLogger.Default);
3535

36-
enableProfiler ??= (EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.ProfilingEnabled) ?? string.Empty).ToBoolean() ?? false;
36+
enableProfiler ??= (EnvironmentHelpers.GetEnvironmentVariable(Datadog.Trace.Configuration.ConfigurationKeys.Profiler.ProfilingEnabled) ?? string.Empty).ToBoolean() ?? false;
3737
switch (enableProfiler)
3838
{
3939
case true:
4040
cfg = cfg.WithOption(ConfigOptions.KeepBenchmarkFiles, true);
4141
break;
4242
case false:
43-
EnvironmentHelpers.SetEnvironmentVariable(ConfigurationKeys.ProfilingEnabled, null);
43+
EnvironmentHelpers.SetEnvironmentVariable(Datadog.Trace.Configuration.ConfigurationKeys.Profiler.ProfilingEnabled, null);
4444
break;
4545
}
4646

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
// <copyright file="ConfigurationBuilderWithKeysAnalyzer.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
using System.Collections.Immutable;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CSharp;
9+
using Microsoft.CodeAnalysis.CSharp.Syntax;
10+
using Microsoft.CodeAnalysis.Diagnostics;
11+
12+
namespace Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers
13+
{
14+
/// <summary>
15+
/// Analyzer to ensure that ConfigurationBuilder.WithKeys method calls only accept string constants
16+
/// from PlatformKeys or ConfigurationKeys classes, not hardcoded strings or variables.
17+
/// </summary>
18+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
19+
public class ConfigurationBuilderWithKeysAnalyzer : DiagnosticAnalyzer
20+
{
21+
/// <summary>
22+
/// Diagnostic descriptor for when WithKeys or Or is called with a hardcoded string instead of a constant from PlatformKeys or ConfigurationKeys.
23+
/// </summary>
24+
public static readonly DiagnosticDescriptor UseConfigurationConstantsRule = new DiagnosticDescriptor(
25+
id: "DD0007",
26+
title: "Use configuration constants instead of hardcoded strings in WithKeys/Or calls",
27+
messageFormat: "{0} method should use constants from PlatformKeys or ConfigurationKeys classes instead of hardcoded string '{1}'",
28+
category: "Usage",
29+
defaultSeverity: DiagnosticSeverity.Error,
30+
isEnabledByDefault: true,
31+
description: "ConfigurationBuilder.WithKeys and HasKeys.Or method calls should only accept string constants from PlatformKeys or ConfigurationKeys classes to ensure consistency and avoid typos.");
32+
33+
/// <summary>
34+
/// Diagnostic descriptor for when WithKeys or Or is called with a variable instead of a constant from PlatformKeys or ConfigurationKeys.
35+
/// </summary>
36+
public static readonly DiagnosticDescriptor UseConfigurationConstantsNotVariablesRule = new DiagnosticDescriptor(
37+
id: "DD0008",
38+
title: "Use configuration constants instead of variables in WithKeys/Or calls",
39+
messageFormat: "{0} method should use constants from PlatformKeys or ConfigurationKeys classes instead of variable '{1}'",
40+
category: "Usage",
41+
defaultSeverity: DiagnosticSeverity.Error,
42+
isEnabledByDefault: true,
43+
description: "ConfigurationBuilder.WithKeys and HasKeys.Or method calls should only accept string constants from PlatformKeys or ConfigurationKeys classes, not variables or computed values.");
44+
45+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
46+
ImmutableArray.Create(UseConfigurationConstantsRule, UseConfigurationConstantsNotVariablesRule);
47+
48+
public override void Initialize(AnalysisContext context)
49+
{
50+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
51+
context.EnableConcurrentExecution();
52+
context.RegisterSyntaxNodeAction(AnalyzeInvocationExpression, SyntaxKind.InvocationExpression);
53+
}
54+
55+
private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context)
56+
{
57+
var invocation = (InvocationExpressionSyntax)context.Node;
58+
59+
// Check if this is a WithKeys or Or method call
60+
var methodName = GetConfigurationMethodName(invocation, context.SemanticModel);
61+
if (methodName == null)
62+
{
63+
return;
64+
}
65+
66+
// Analyze each argument to the method
67+
var argumentList = invocation.ArgumentList;
68+
if (argumentList?.Arguments.Count > 0)
69+
{
70+
var argument = argumentList.Arguments[0]; // Both WithKeys and Or take a single string argument
71+
AnalyzeConfigurationArgument(context, argument, methodName);
72+
}
73+
}
74+
75+
private static string GetConfigurationMethodName(InvocationExpressionSyntax invocation, SemanticModel semanticModel)
76+
{
77+
if (invocation.Expression is MemberAccessExpressionSyntax memberAccess)
78+
{
79+
var methodName = memberAccess.Name.Identifier.ValueText;
80+
81+
// Check if the method being called is "WithKeys" or "Or"
82+
if (methodName == "WithKeys" || methodName == "Or")
83+
{
84+
// Get the symbol info for the method
85+
var symbolInfo = semanticModel.GetSymbolInfo(memberAccess);
86+
if (symbolInfo.Symbol is IMethodSymbol method)
87+
{
88+
var containingType = method.ContainingType?.Name;
89+
var containingNamespace = method.ContainingNamespace?.ToDisplayString();
90+
91+
// Check if this is the ConfigurationBuilder.WithKeys method
92+
if (methodName == "WithKeys" &&
93+
containingType == "ConfigurationBuilder" &&
94+
containingNamespace == "Datadog.Trace.Configuration.Telemetry")
95+
{
96+
return "WithKeys";
97+
}
98+
99+
// Check if this is the HasKeys.Or method
100+
if (methodName == "Or" &&
101+
containingType == "HasKeys" &&
102+
containingNamespace == "Datadog.Trace.Configuration.Telemetry")
103+
{
104+
return "Or";
105+
}
106+
}
107+
}
108+
}
109+
110+
return null;
111+
}
112+
113+
private static void AnalyzeConfigurationArgument(SyntaxNodeAnalysisContext context, ArgumentSyntax argument, string methodName)
114+
{
115+
var expression = argument.Expression;
116+
117+
switch (expression)
118+
{
119+
case LiteralExpressionSyntax literal when literal.Token.IsKind(SyntaxKind.StringLiteralToken):
120+
// This is a hardcoded string literal - report diagnostic
121+
var literalValue = literal.Token.ValueText;
122+
var diagnostic = Diagnostic.Create(
123+
UseConfigurationConstantsRule,
124+
literal.GetLocation(),
125+
methodName,
126+
literalValue);
127+
context.ReportDiagnostic(diagnostic);
128+
break;
129+
130+
case MemberAccessExpressionSyntax memberAccess:
131+
// Check if this is accessing a constant from PlatformKeys or ConfigurationKeys
132+
if (!IsValidConfigurationConstant(memberAccess, context.SemanticModel))
133+
{
134+
// This is accessing something else - report diagnostic
135+
var memberName = memberAccess.ToString();
136+
var memberDiagnostic = Diagnostic.Create(
137+
UseConfigurationConstantsNotVariablesRule,
138+
memberAccess.GetLocation(),
139+
methodName,
140+
memberName);
141+
context.ReportDiagnostic(memberDiagnostic);
142+
}
143+
144+
break;
145+
146+
case IdentifierNameSyntax identifier:
147+
// This is a variable or local constant - report diagnostic
148+
var identifierName = identifier.Identifier.ValueText;
149+
var variableDiagnostic = Diagnostic.Create(
150+
UseConfigurationConstantsNotVariablesRule,
151+
identifier.GetLocation(),
152+
methodName,
153+
identifierName);
154+
context.ReportDiagnostic(variableDiagnostic);
155+
break;
156+
157+
default:
158+
// Any other expression type (method calls, computed values, etc.) - report diagnostic
159+
var expressionText = expression.ToString();
160+
var defaultDiagnostic = Diagnostic.Create(
161+
UseConfigurationConstantsNotVariablesRule,
162+
expression.GetLocation(),
163+
methodName,
164+
expressionText);
165+
context.ReportDiagnostic(defaultDiagnostic);
166+
break;
167+
}
168+
}
169+
170+
private static bool IsValidConfigurationConstant(MemberAccessExpressionSyntax memberAccess, SemanticModel semanticModel)
171+
{
172+
var symbolInfo = semanticModel.GetSymbolInfo(memberAccess);
173+
if (symbolInfo.Symbol is IFieldSymbol field)
174+
{
175+
// Check if this is a const string field
176+
if (field.IsConst && field.Type?.SpecialType == SpecialType.System_String)
177+
{
178+
var containingType = field.ContainingType;
179+
if (containingType != null)
180+
{
181+
// Check if the containing type is PlatformKeys or ConfigurationKeys (or their nested classes)
182+
return IsValidConfigurationClass(containingType);
183+
}
184+
}
185+
}
186+
187+
return false;
188+
}
189+
190+
private static bool IsValidConfigurationClass(INamedTypeSymbol typeSymbol)
191+
{
192+
// Check if this is PlatformKeys or ConfigurationKeys class or their nested classes
193+
var currentType = typeSymbol;
194+
while (currentType != null)
195+
{
196+
var typeName = currentType.Name;
197+
var namespaceName = currentType.ContainingNamespace?.ToDisplayString();
198+
199+
// Check for PlatformKeys class
200+
if (typeName == "PlatformKeys" && namespaceName == "Datadog.Trace.Configuration")
201+
{
202+
return true;
203+
}
204+
205+
// Check for ConfigurationKeys class
206+
if (typeName == "ConfigurationKeys" && namespaceName == "Datadog.Trace.Configuration")
207+
{
208+
return true;
209+
}
210+
211+
// Check nested classes within PlatformKeys or ConfigurationKeys
212+
currentType = currentType.ContainingType;
213+
}
214+
215+
return false;
216+
}
217+
}
218+
}

tracer/src/Datadog.Trace/ContinuousProfiler/ConfigurationKeys.cs renamed to tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.Profiler.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1-
// <copyright file="ConfigurationKeys.cs" company="Datadog">
1+
// <copyright file="ConfigurationKeys.Profiler.cs" company="Datadog">
22
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
5+
#nullable enable
56

6-
namespace Datadog.Trace.ContinuousProfiler
7+
namespace Datadog.Trace.Configuration;
8+
9+
internal partial class ConfigurationKeys
710
{
8-
internal static class ConfigurationKeys
11+
internal class Profiler
912
{
1013
public const string ProfilingEnabled = "DD_PROFILING_ENABLED";
1114
public const string CodeHotspotsEnabled = "DD_PROFILING_CODEHOTSPOTS_ENABLED";

tracer/src/Datadog.Trace/Configuration/DynamicConfigurationManager.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ private static void OnConfigurationChanged(ConfigurationBuilder settings)
7575
{
7676
var oldSettings = Tracer.Instance.Settings;
7777

78-
var headerTags = MutableSettings.InitializeHeaderTags(settings, ConfigurationKeys.HeaderTags, headerTagsNormalizationFixEnabled: true);
78+
var headerTags = MutableSettings.InitializeHeaderTags(settings.WithKeys(ConfigurationKeys.HeaderTags), headerTagsNormalizationFixEnabled: true);
7979
// var serviceNameMappings = TracerSettings.InitializeServiceNameMappings(settings, ConfigurationKeys.ServiceNameMappings);
8080

8181
var globalTags = settings.WithKeys(ConfigurationKeys.GlobalTags).AsDictionary();

tracer/src/Datadog.Trace/Configuration/MutableSettings.cs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,9 @@ private MutableSettings(
249249

250250
internal IConfigurationTelemetry Telemetry { get; }
251251

252-
internal static ReadOnlyDictionary<string, string>? InitializeHeaderTags(ConfigurationBuilder config, string key, bool headerTagsNormalizationFixEnabled)
252+
internal static ReadOnlyDictionary<string, string>? InitializeHeaderTags(ConfigurationBuilder.HasKeys headerTagsKeys, bool headerTagsNormalizationFixEnabled)
253253
{
254-
var configurationDictionary = config
255-
.WithKeys(key)
254+
var configurationDictionary = headerTagsKeys
256255
.AsDictionary(allowOptionalMappings: true, defaultValue: null, "[]");
257256

258257
if (configurationDictionary == null)
@@ -746,10 +745,10 @@ public static MutableSettings Create(
746745
.AsBool(defaultValue: true);
747746

748747
// Filter out tags with empty keys or empty values, and trim whitespaces
749-
var headerTags = InitializeHeaderTags(config, ConfigurationKeys.HeaderTags, headerTagsNormalizationFixEnabled) ?? ReadOnlyDictionary.Empty;
748+
var headerTags = InitializeHeaderTags(config.WithKeys(ConfigurationKeys.HeaderTags), headerTagsNormalizationFixEnabled) ?? ReadOnlyDictionary.Empty;
750749

751750
// Filter out tags with empty keys or empty values, and trim whitespaces
752-
var grpcTags = InitializeHeaderTags(config, ConfigurationKeys.GrpcTags, headerTagsNormalizationFixEnabled: true) ?? ReadOnlyDictionary.Empty;
751+
var grpcTags = InitializeHeaderTags(config.WithKeys(ConfigurationKeys.GrpcTags), headerTagsNormalizationFixEnabled: true) ?? ReadOnlyDictionary.Empty;
753752

754753
var customSamplingRules = config.WithKeys(ConfigurationKeys.CustomSamplingRules).AsString();
755754

@@ -768,7 +767,7 @@ public static MutableSettings Create(
768767
var kafkaCreateConsumerScopeEnabled = config
769768
.WithKeys(ConfigurationKeys.KafkaCreateConsumerScopeEnabled)
770769
.AsBool(defaultValue: true);
771-
var serviceNameMappings = TracerSettings.InitializeServiceNameMappings(config, ConfigurationKeys.ServiceNameMappings) ?? ReadOnlyDictionary.Empty;
770+
var serviceNameMappings = TracerSettings.TrimConfigKeysValues(config.WithKeys(ConfigurationKeys.ServiceNameMappings)) ?? ReadOnlyDictionary.Empty;
772771

773772
var tracerMetricsEnabled = config
774773
.WithKeys(ConfigurationKeys.TracerMetricsEnabled)

tracer/src/Datadog.Trace/Configuration/TracerSettings.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ internal TracerSettings(IConfigurationSource? source, IConfigurationTelemetry te
150150
.WithKeys(ConfigurationKeys.SpanPointersEnabled)
151151
.AsBool(defaultValue: true);
152152

153-
PeerServiceNameMappings = InitializeServiceNameMappings(config, ConfigurationKeys.PeerServiceNameMappings);
153+
PeerServiceNameMappings = TrimConfigKeysValues(config.WithKeys(ConfigurationKeys.PeerServiceNameMappings));
154154

155155
MetadataSchemaVersion = config
156156
.WithKeys(ConfigurationKeys.MetadataSchemaVersion)
@@ -1187,11 +1187,9 @@ not null when string.Equals(x, "lowmemory", StringComparison.OrdinalIgnoreCase)
11871187
internal static TracerSettings FromDefaultSourcesInternal()
11881188
=> new(GlobalConfigurationSource.Instance, new ConfigurationTelemetry(), new());
11891189

1190-
internal static ReadOnlyDictionary<string, string>? InitializeServiceNameMappings(ConfigurationBuilder config, string key)
1190+
internal static ReadOnlyDictionary<string, string>? TrimConfigKeysValues(ConfigurationBuilder.HasKeys key)
11911191
{
1192-
var mappings = config
1193-
.WithKeys(key)
1194-
.AsDictionary()
1192+
var mappings = key.AsDictionary()
11951193
?.Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key) && !string.IsNullOrWhiteSpace(kvp.Value))
11961194
.ToDictionary(kvp => kvp.Key.Trim(), kvp => kvp.Value.Trim());
11971195
return mappings is not null ? new(mappings) : null;

tracer/src/Datadog.Trace/ContinuousProfiler/ContextTracker.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
using System.Runtime.CompilerServices;
88
using System.Runtime.InteropServices;
99
using System.Threading;
10-
using Datadog.Trace.ExtensionMethods;
10+
using Datadog.Trace.Configuration;
1111
using Datadog.Trace.Logging;
1212
using Datadog.Trace.Util;
1313

@@ -43,8 +43,8 @@ internal class ContextTracker : IContextTracker
4343
public ContextTracker(IProfilerStatus status)
4444
{
4545
_status = status;
46-
_isCodeHotspotsEnabled = EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.CodeHotspotsEnabled)?.ToBoolean() ?? true;
47-
_isEndpointProfilingEnabled = EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.EndpointProfilingEnabled)?.ToBoolean() ?? true;
46+
_isCodeHotspotsEnabled = EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.Profiler.CodeHotspotsEnabled)?.ToBoolean() ?? true;
47+
_isEndpointProfilingEnabled = EnvironmentHelpers.GetEnvironmentVariable(ConfigurationKeys.Profiler.EndpointProfilingEnabled)?.ToBoolean() ?? true;
4848
_traceContextPtr = new ThreadLocal<IntPtr>();
4949
}
5050

tracer/src/Datadog.Trace/ContinuousProfiler/ProfilerSettings.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ internal ProfilerSettings(IConfigurationSource config, IConfigurationSource envC
2424
if (!IsProfilingSupported)
2525
{
2626
ProfilerState = ProfilerState.Disabled;
27-
telemetry.Record(ConfigurationKeys.ProfilingEnabled, false, ConfigurationOrigins.Calculated);
27+
telemetry.Record(ConfigurationKeys.Profiler.ProfilingEnabled, false, ConfigurationOrigins.Calculated);
2828
return;
2929
}
3030

3131
// If managed activation is enabled, we need to _just_ read from the environment variables,
3232
// as that's all that applies
3333
var envConfigBuilder = new ConfigurationBuilder(envConfig, telemetry);
3434
var managedActivationEnabled = envConfigBuilder
35-
.WithKeys(ConfigurationKeys.ProfilerManagedActivationEnabled)
35+
.WithKeys(ConfigurationKeys.Profiler.ProfilerManagedActivationEnabled)
3636
.AsBool(true);
3737

3838
// If we're using managed activation, we use the "full" config source set.
@@ -45,7 +45,7 @@ internal ProfilerSettings(IConfigurationSource config, IConfigurationSource envC
4545
// the profiler could be enabled via ContinuousProfiler.ConfigurationKeys.SsiDeployed. If it is non-empty, then the
4646
// profiler is "active", though won't begin profiling until 30 seconds have passed + at least 1 span has been generated.
4747
var profilingEnabled = profilingConfig
48-
.WithKeys(ConfigurationKeys.ProfilingEnabled)
48+
.WithKeys(ConfigurationKeys.Profiler.ProfilingEnabled)
4949
// We stick with strings here instead of using the `GetAs` method,
5050
// so that telemetry continues to store true/false/auto, instead of the enum values.
5151
.AsString(

0 commit comments

Comments
 (0)