Skip to content

Commit 2f5de7c

Browse files
andrewlockandrewlock
authored
Repurpose PublicApiAnalyzer as InternalForTestingAnalyzer (#7788)
## Summary of changes Repurposes the now-defunct `PublicApiAnalyzer` to an `InternalForTestingAnalyzer` ## Reason for change We have various methods in `Datadog.Trace` that should never be called by production code, and are only exposed to make testing functionality easier. Currently we rely on method names and comments to avoid this. With this change, we can add an attribute instead, and using the method from Datadog.Trace etc becomes a build error. ## Implementation details - Basically just renamed `PublicApi*` -> `InternalForTesting*` - Decorated methods that were currently relying on comments/naming to avoid being called ## Test coverage Refactored the analyzer tests, so should still be covered the same ## Other details Note that there are a bunch of comments for methods `// internal for testing` which are a bit different. In those cases, we _do_ call it in Datadog.Trace, the comment is just explaining why the method isn't `private`. Those remain as-is, this attribute is purely if you have a "don't call this method in production code, use that other one instead" method 😄 Part of a stack - #7786 - #7787 - #7788 👈
1 parent 76fa850 commit 2f5de7c

File tree

50 files changed

+293
-121
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+293
-121
lines changed

‎tracer/src/Datadog.Trace.Tools.Analyzers/PublicApiAnalyzer/PublicApiAnalyzer.cs‎ renamed to ‎tracer/src/Datadog.Trace.Tools.Analyzers/TestingOnlyAnalyzer/TestingOnlyAnalyzer.cs‎

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// <copyright file="PublicApiAnalyzer.cs" company="Datadog">
1+
// <copyright file="TestingOnlyAnalyzer.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>
@@ -14,37 +14,37 @@
1414
using Microsoft.CodeAnalysis.Diagnostics;
1515
using Microsoft.CodeAnalysis.Operations;
1616

17-
namespace Datadog.Trace.Tools.Analyzers.PublicApiAnalyzer
17+
namespace Datadog.Trace.Tools.Analyzers.TestingOnlyAnalyzer
1818
{
1919
/// <summary>
20-
/// DD002: Incorrect usage of public API
20+
/// DD002: Incorrect usage of internal API
2121
///
22-
/// Finds internal usages of APIs specifically marked with the [PublicApi] flag.
23-
/// These methods should not be called directly by our library code, only users should invoke them.
24-
/// The analyzer enforces that. requirement
22+
/// Finds internal usages of APIs specifically marked with the [TestingOnly] or [TestingAndPrivateOnly] flag.
23+
/// - [TestingOnly]: Methods should not be called directly by our library code, only from test code.
24+
/// - [TestingAndPrivateOnly]: Methods can be called from within the same type, or from test code, but not from other types.
25+
/// The analyzer enforces these requirements.
2526
///
2627
/// </summary>
2728
[DiagnosticAnalyzer(LanguageNames.CSharp)]
28-
public sealed class PublicApiAnalyzer : DiagnosticAnalyzer
29+
public sealed class TestingOnlyAnalyzer : DiagnosticAnalyzer
2930
{
3031
/// <summary>
3132
/// The diagnostic ID displayed in error messages
3233
/// </summary>
3334
public const string DiagnosticId = "DD0002";
3435

35-
private const string PublicApiAttribute = nameof(PublicApiAttribute);
36-
37-
private static readonly ImmutableArray<string> PublicApiAttributeNames = ImmutableArray.Create(PublicApiAttribute);
36+
private const string TestingOnlyAttribute = nameof(TestingOnlyAttribute);
37+
private const string TestingAndPrivateOnlyAttribute = nameof(TestingAndPrivateOnlyAttribute);
3838

3939
#pragma warning disable RS2008 // Enable analyzer release tracking for the analyzer project
4040
private static readonly DiagnosticDescriptor Rule = new(
4141
DiagnosticId,
42-
title: "Incorrect usage of public API",
43-
messageFormat: "This API is only for public usage and should not be called internally",
42+
title: "Incorrect usage of internal API",
43+
messageFormat: "This API is only for use in tests and should not be called internally",
4444
category: "CodeQuality",
4545
defaultSeverity: DiagnosticSeverity.Error,
4646
isEnabledByDefault: true,
47-
description: "This API is only for public usage and should not be called internally. Use an alternative method.");
47+
description: "This API is only for internal testing and should not be called internally. Use an alternative method.");
4848
#pragma warning restore RS2008
4949

5050
/// <inheritdoc />
@@ -58,23 +58,24 @@ public override void Initialize(AnalysisContext context)
5858

5959
context.RegisterCompilationStartAction(context =>
6060
{
61-
var publicApiMembers = new ConcurrentDictionary<ISymbol, PublicApiStatus>(SymbolEqualityComparer.Default);
61+
var internalApiMembers = new ConcurrentDictionary<ISymbol, InternalApiStatus>(SymbolEqualityComparer.Default);
6262

6363
context.RegisterOperationBlockStartAction(
64-
context => AnalyzeOperationBlock(context, publicApiMembers));
64+
ctx => AnalyzeOperationBlock(ctx, internalApiMembers));
6565
});
6666
}
6767

68-
private void AnalyzeOperationBlock(
68+
private static void AnalyzeOperationBlock(
6969
OperationBlockStartAnalysisContext context,
70-
ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers)
70+
ConcurrentDictionary<ISymbol, InternalApiStatus> internalApiMembers)
7171
{
72-
var publicApiOperations = PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool>.GetInstance();
72+
var internalApiOperations = PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool>.GetInstance();
73+
var callingType = context.OwningSymbol?.ContainingType;
7374

7475
context.RegisterOperationAction(
75-
context =>
76+
ctx =>
7677
{
77-
Helpers.AnalyzeOperation(context.Operation, publicApiOperations, publicApiMembers);
78+
Helpers.AnalyzeOperation(ctx.Operation, internalApiOperations, internalApiMembers, callingType);
7879
},
7980
OperationKind.MethodReference,
8081
OperationKind.EventReference,
@@ -83,23 +84,23 @@ private void AnalyzeOperationBlock(
8384
OperationKind.ObjectCreation,
8485
OperationKind.PropertyReference);
8586

86-
context.RegisterOperationBlockEndAction(context =>
87+
context.RegisterOperationBlockEndAction(ctx =>
8788
{
8889
try
8990
{
90-
if (publicApiOperations.IsEmpty)
91+
if (internalApiOperations.IsEmpty)
9192
{
9293
return;
9394
}
9495

95-
foreach (var kvp in publicApiOperations)
96+
foreach (var kvp in internalApiOperations)
9697
{
97-
context.ReportDiagnostic(Diagnostic.Create(Rule, kvp.Key.Key.Syntax.GetLocation()));
98+
ctx.ReportDiagnostic(Diagnostic.Create(Rule, kvp.Key.Key.Syntax.GetLocation()));
9899
}
99100
}
100101
finally
101102
{
102-
publicApiOperations.Free(context.CancellationToken);
103+
internalApiOperations.Free(ctx.CancellationToken);
103104
}
104105
});
105106
}
@@ -108,8 +109,9 @@ private static class Helpers
108109
{
109110
internal static void AnalyzeOperation(
110111
IOperation operation,
111-
PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool> publicApiOperations,
112-
ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers)
112+
PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool> internalApiOperations,
113+
ConcurrentDictionary<ISymbol, InternalApiStatus> internalApiMembers,
114+
INamedTypeSymbol? callingType)
113115
{
114116
var symbol = GetOperationSymbol(operation);
115117

@@ -184,9 +186,22 @@ void CheckTypeArgumentsCore(ImmutableArray<ITypeSymbol> typeArguments, PooledHas
184186

185187
void CheckOperationAttributes(ISymbol symbol, bool checkParents)
186188
{
187-
if (TryGetOrCreatePublicApiAttributes(symbol, checkParents, publicApiMembers, out _))
189+
if (TryGetOrCreatePublicApiAttributes(symbol, checkParents, internalApiMembers, out var attributes))
188190
{
189-
publicApiOperations.TryAdd(new KeyValuePair<IOperation, ISymbol>(operation, symbol), true);
191+
// If the marked member has [TestingAndPrivateOnly], allow calls from within the same type
192+
if (attributes.IsTestingAndPrivateOnly)
193+
{
194+
var targetType = symbol.ContainingType;
195+
196+
// Allow calls from within the same type
197+
if (callingType != null && targetType != null &&
198+
SymbolEqualityComparer.Default.Equals(callingType, targetType))
199+
{
200+
return;
201+
}
202+
}
203+
204+
internalApiOperations.TryAdd(new KeyValuePair<IOperation, ISymbol>(operation, symbol), true);
190205
}
191206
}
192207
}
@@ -238,11 +253,12 @@ void CheckOperationAttributes(ISymbol symbol, bool checkParents)
238253
return iEvent;
239254
}
240255

241-
private static PublicApiStatus CopyAttributes(PublicApiStatus copyAttributes) =>
256+
private static InternalApiStatus CopyAttributes(InternalApiStatus copyAttributes) =>
242257
new()
243258
{
244259
IsAssemblyAttribute = copyAttributes.IsAssemblyAttribute,
245-
IsPublicApi = copyAttributes.IsPublicApi
260+
IsTestingApi = copyAttributes.IsTestingApi,
261+
IsTestingAndPrivateOnly = copyAttributes.IsTestingAndPrivateOnly
246262
};
247263

248264
private static bool IsWithinConditionalOperation(IFieldReferenceOperation pOperation) =>
@@ -260,8 +276,8 @@ or BinaryOperatorKind.LessThanOrEqual
260276
private static bool TryGetOrCreatePublicApiAttributes(
261277
ISymbol symbol,
262278
bool checkParents,
263-
ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers,
264-
out PublicApiStatus attributes)
279+
ConcurrentDictionary<ISymbol, InternalApiStatus> publicApiMembers,
280+
out InternalApiStatus attributes)
265281
{
266282
if (!publicApiMembers.TryGetValue(symbol, out attributes))
267283
{
@@ -281,34 +297,44 @@ private static bool TryGetOrCreatePublicApiAttributes(
281297
}
282298
}
283299

284-
attributes ??= new PublicApiStatus() { IsAssemblyAttribute = symbol is IAssemblySymbol };
300+
attributes ??= new InternalApiStatus() { IsAssemblyAttribute = symbol is IAssemblySymbol };
285301
MergePlatformAttributes(symbol.GetAttributes(), ref attributes);
286302
attributes = publicApiMembers.GetOrAdd(symbol, attributes);
287303
}
288304

289-
return attributes.IsPublicApi;
305+
return attributes.IsTestingApi;
290306

291307
static void MergePlatformAttributes(
292308
ImmutableArray<AttributeData> immediateAttributes,
293-
ref PublicApiStatus parentAttributes)
309+
ref InternalApiStatus parentAttributes)
294310
{
295311
foreach (AttributeData attribute in immediateAttributes)
296312
{
297-
if (PublicApiAttributeNames.Contains(attribute.AttributeClass!.Name))
313+
var attributeName = attribute.AttributeClass!.Name;
314+
if (attributeName == TestingAndPrivateOnlyAttribute)
298315
{
299-
parentAttributes.IsPublicApi = true;
316+
parentAttributes.IsTestingApi = true;
317+
parentAttributes.IsTestingAndPrivateOnly = true;
318+
return;
319+
}
320+
else if (attributeName == TestingOnlyAttribute)
321+
{
322+
parentAttributes.IsTestingApi = true;
323+
parentAttributes.IsTestingAndPrivateOnly = false;
300324
return;
301325
}
302326
}
303327
}
304328
}
305329
}
306330

307-
private sealed class PublicApiStatus
331+
private sealed class InternalApiStatus
308332
{
309333
public bool IsAssemblyAttribute { get; set; }
310334

311-
public bool IsPublicApi { get; set; }
335+
public bool IsTestingApi { get; set; }
336+
337+
public bool IsTestingAndPrivateOnly { get; set; }
312338
}
313339
}
314340
}

‎tracer/src/Datadog.Trace.Tools.Runner/AnalyzeInstrumentationErrorsCommand.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ private string GetLogDirectory(int? pid)
197197
logDirectory = ProcessConfiguration.GetProcessLogDirectory(pid.Value);
198198
}
199199

200+
#pragma warning disable DD0002
200201
return logDirectory ?? Logging.DatadogLoggingFactory.GetLogDirectory(NullConfigurationTelemetry.Instance);
202+
#pragma warning restore DD0002
201203
}
202204
}

‎tracer/src/Datadog.Trace/Agent/Api.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Datadog.Trace.DogStatsd;
1313
using Datadog.Trace.Logging;
1414
using Datadog.Trace.PlatformHelpers;
15+
using Datadog.Trace.SourceGenerators;
1516
using Datadog.Trace.Telemetry;
1617
using Datadog.Trace.Telemetry.Metrics;
1718
using Datadog.Trace.Util.Http;
@@ -94,7 +95,7 @@ public Task<bool> SendTracesAsync(ArraySegment<byte> traces, int numberOfTraces,
9495
return SendWithRetry(_tracesEndpoint, _sendTraces, state);
9596
}
9697

97-
// internal for testing
98+
[TestingAndPrivateOnly]
9899
internal bool LogPartialFlushWarningIfRequired(string agentVersion)
99100
{
100101
if (agentVersion != _agentVersion)

‎tracer/src/Datadog.Trace/Agent/MovingAverageKeepRateCalculator.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99
using Datadog.Trace.Logging;
10+
using Datadog.Trace.SourceGenerators;
1011
using Datadog.Trace.Util;
1112

1213
namespace Datadog.Trace.Agent
@@ -106,6 +107,7 @@ public void CancelUpdates()
106107
/// <summary>
107108
/// Update the current rate. Internal for testing only. Should not be called in normal usage.
108109
/// </summary>
110+
[TestingAndPrivateOnly]
109111
internal void UpdateBucket()
110112
{
111113
var index = _index;

‎tracer/src/Datadog.Trace/Ci/Configuration/TestOptimizationSettings.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
1414
using Datadog.Trace.Configuration.Telemetry;
1515
using Datadog.Trace.LibDatadog;
16+
using Datadog.Trace.SourceGenerators;
1617
using Datadog.Trace.Telemetry;
1718
using Datadog.Trace.Util;
1819

@@ -361,7 +362,7 @@ internal void SetDefaultManualInstrumentationSettings()
361362
private TracerSettings InitializeTracerSettings()
362363
=> InitializeTracerSettings(GlobalConfigurationSource.Instance);
363364

364-
// Internal for testing
365+
[TestingAndPrivateOnly]
365366
internal TracerSettings InitializeTracerSettings(IConfigurationSource source)
366367
{
367368
// This is a somewhat hacky way to "tell" TracerSettings that we're running in CI Visibility

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Logging/ILogger/DirectSubmission/LoggerFactoryIntegrationCommon.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Reflection.Emit;
1010
using Datadog.Trace.DuckTyping;
1111
using Datadog.Trace.Logging;
12+
using Datadog.Trace.SourceGenerators;
1213

1314
namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.ILogger.DirectSubmission
1415
{
@@ -17,7 +18,7 @@ internal static class LoggerFactoryIntegrationCommon<TLoggerFactory>
1718
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(LoggerFactoryIntegrationCommon<TLoggerFactory>));
1819

1920
// ReSharper disable once StaticMemberInGenericType
20-
// Internal for testing
21+
[TestingAndPrivateOnly]
2122
internal static readonly Type? ProviderInterfaces;
2223

2324
static LoggerFactoryIntegrationCommon()
@@ -84,7 +85,7 @@ internal static bool TryAddDirectSubmissionLoggerProvider(TLoggerFactory loggerF
8485
return TryAddDirectSubmissionLoggerProvider(loggerFactory, provider);
8586
}
8687

87-
// Internal for testing
88+
[TestingAndPrivateOnly]
8889
internal static bool TryAddDirectSubmissionLoggerProvider(TLoggerFactory loggerFactory, DirectSubmissionLoggerProvider provider)
8990
{
9091
if (ProviderInterfaces is null)

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Logging/Log4Net/DirectSubmission/DirectSubmissionLog4NetAppender.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Datadog.Trace.DuckTyping;
99
using Datadog.Trace.Logging.DirectSubmission;
1010
using Datadog.Trace.Logging.DirectSubmission.Sink;
11+
using Datadog.Trace.SourceGenerators;
1112
using Datadog.Trace.Telemetry;
1213
using Datadog.Trace.Telemetry.Metrics;
1314

@@ -23,7 +24,7 @@ internal class DirectSubmissionLog4NetAppender
2324
private readonly IDirectSubmissionLogSink _sink;
2425
private readonly DirectSubmissionLogLevel _minimumLevel;
2526

26-
// internal for testing
27+
[TestingAndPrivateOnly]
2728
internal DirectSubmissionLog4NetAppender(IDirectSubmissionLogSink sink, DirectSubmissionLogLevel minimumLevel)
2829
{
2930
_sink = sink;

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Logging/Log4Net/DirectSubmission/DirectSubmissionLog4NetLegacyAppender.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Datadog.Trace.DuckTyping;
99
using Datadog.Trace.Logging.DirectSubmission;
1010
using Datadog.Trace.Logging.DirectSubmission.Sink;
11+
using Datadog.Trace.SourceGenerators;
1112
using Datadog.Trace.Telemetry;
1213
using Datadog.Trace.Telemetry.Metrics;
1314

@@ -23,7 +24,7 @@ internal class DirectSubmissionLog4NetLegacyAppender
2324
private readonly IDirectSubmissionLogSink _sink;
2425
private readonly DirectSubmissionLogLevel _minimumLevel;
2526

26-
// internal for testing
27+
[TestingAndPrivateOnly]
2728
internal DirectSubmissionLog4NetLegacyAppender(IDirectSubmissionLogSink sink, DirectSubmissionLogLevel minimumLevel)
2829
{
2930
_sink = sink;

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Logging/NLog/DirectSubmission/DirectSubmissionNLogLegacyTarget.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Datadog.Trace.Logging.DirectSubmission;
1313
using Datadog.Trace.Logging.DirectSubmission.Formatting;
1414
using Datadog.Trace.Logging.DirectSubmission.Sink;
15+
using Datadog.Trace.SourceGenerators;
1516
using Datadog.Trace.Telemetry;
1617
using Datadog.Trace.Telemetry.Metrics;
1718

@@ -32,7 +33,7 @@ internal DirectSubmissionNLogLegacyTarget(IDirectSubmissionLogSink sink, DirectS
3233
{
3334
}
3435

35-
// internal for testing
36+
[TestingAndPrivateOnly]
3637
internal DirectSubmissionNLogLegacyTarget(
3738
IDirectSubmissionLogSink sink,
3839
DirectSubmissionLogLevel minimumLevel,

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Logging/NLog/DirectSubmission/DirectSubmissionNLogTarget.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using Datadog.Trace.Logging.DirectSubmission;
1111
using Datadog.Trace.Logging.DirectSubmission.Formatting;
1212
using Datadog.Trace.Logging.DirectSubmission.Sink;
13+
using Datadog.Trace.SourceGenerators;
1314
using Datadog.Trace.Telemetry;
1415
using Datadog.Trace.Telemetry.Metrics;
1516

@@ -30,7 +31,7 @@ internal DirectSubmissionNLogTarget(IDirectSubmissionLogSink sink, DirectSubmiss
3031
{
3132
}
3233

33-
// internal for testing
34+
[TestingAndPrivateOnly]
3435
internal DirectSubmissionNLogTarget(
3536
IDirectSubmissionLogSink sink,
3637
DirectSubmissionLogLevel minimumLevel,

0 commit comments

Comments
 (0)