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 👈

Author: andrewlock

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

@@ -1,4 +1,4 @@
-// <copyright file="PublicApiAnalyzer.cs" company="Datadog">
+// <copyright file="TestingOnlyAnalyzer.cs" company="Datadog">
 // Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
@@ -14,37 +14,37 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
 
-namespace Datadog.Trace.Tools.Analyzers.PublicApiAnalyzer
+namespace Datadog.Trace.Tools.Analyzers.TestingOnlyAnalyzer
 {
     /// <summary>
-    /// DD002: Incorrect usage of public API
+    /// DD002: Incorrect usage of internal API
     ///
-    /// Finds internal usages of APIs specifically marked with the [PublicApi] flag.
-    /// These methods should not be called directly by our library code, only users should invoke them.
-    /// The analyzer enforces that. requirement
+    /// Finds internal usages of APIs specifically marked with the [TestingOnly] or [TestingAndPrivateOnly] flag.
+    /// - [TestingOnly]: Methods should not be called directly by our library code, only from test code.
+    /// - [TestingAndPrivateOnly]: Methods can be called from within the same type, or from test code, but not from other types.
+    /// The analyzer enforces these requirements.
     ///
     /// </summary>
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class PublicApiAnalyzer : DiagnosticAnalyzer
+    public sealed class TestingOnlyAnalyzer : DiagnosticAnalyzer
     {
         /// <summary>
         /// The diagnostic ID displayed in error messages
         /// </summary>
         public const string DiagnosticId = "DD0002";
 
-        private const string PublicApiAttribute = nameof(PublicApiAttribute);
-
-        private static readonly ImmutableArray<string> PublicApiAttributeNames = ImmutableArray.Create(PublicApiAttribute);
+        private const string TestingOnlyAttribute = nameof(TestingOnlyAttribute);
+        private const string TestingAndPrivateOnlyAttribute = nameof(TestingAndPrivateOnlyAttribute);
 
 #pragma warning disable RS2008 // Enable analyzer release tracking for the analyzer project
         private static readonly DiagnosticDescriptor Rule = new(
             DiagnosticId,
-            title: "Incorrect usage of public API",
-            messageFormat: "This API is only for public usage and should not be called internally",
+            title: "Incorrect usage of internal API",
+            messageFormat: "This API is only for use in tests and should not be called internally",
             category: "CodeQuality",
             defaultSeverity: DiagnosticSeverity.Error,
             isEnabledByDefault: true,
-            description: "This API is only for public usage and should not be called internally. Use an alternative method.");
+            description: "This API is only for internal testing and should not be called internally. Use an alternative method.");
 #pragma warning restore RS2008
 
         /// <inheritdoc />
@@ -58,23 +58,24 @@ public override void Initialize(AnalysisContext context)
 
             context.RegisterCompilationStartAction(context =>
             {
-                var publicApiMembers = new ConcurrentDictionary<ISymbol, PublicApiStatus>(SymbolEqualityComparer.Default);
+                var internalApiMembers = new ConcurrentDictionary<ISymbol, InternalApiStatus>(SymbolEqualityComparer.Default);
 
                 context.RegisterOperationBlockStartAction(
-                    context => AnalyzeOperationBlock(context, publicApiMembers));
+                    ctx => AnalyzeOperationBlock(ctx, internalApiMembers));
             });
         }
 
-        private void AnalyzeOperationBlock(
+        private static void AnalyzeOperationBlock(
             OperationBlockStartAnalysisContext context,
-            ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers)
+            ConcurrentDictionary<ISymbol, InternalApiStatus> internalApiMembers)
         {
-            var publicApiOperations = PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool>.GetInstance();
+            var internalApiOperations = PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool>.GetInstance();
+            var callingType = context.OwningSymbol?.ContainingType;
 
             context.RegisterOperationAction(
-                context =>
+                ctx =>
                 {
-                    Helpers.AnalyzeOperation(context.Operation, publicApiOperations, publicApiMembers);
+                    Helpers.AnalyzeOperation(ctx.Operation, internalApiOperations, internalApiMembers, callingType);
                 },
                 OperationKind.MethodReference,
                 OperationKind.EventReference,
@@ -83,23 +84,23 @@ private void AnalyzeOperationBlock(
                 OperationKind.ObjectCreation,
                 OperationKind.PropertyReference);
 
-            context.RegisterOperationBlockEndAction(context =>
+            context.RegisterOperationBlockEndAction(ctx =>
             {
                 try
                 {
-                    if (publicApiOperations.IsEmpty)
+                    if (internalApiOperations.IsEmpty)
                     {
                         return;
                     }
 
-                    foreach (var kvp in publicApiOperations)
+                    foreach (var kvp in internalApiOperations)
                     {
-                        context.ReportDiagnostic(Diagnostic.Create(Rule, kvp.Key.Key.Syntax.GetLocation()));
+                        ctx.ReportDiagnostic(Diagnostic.Create(Rule, kvp.Key.Key.Syntax.GetLocation()));
                     }
                 }
                 finally
                 {
-                    publicApiOperations.Free(context.CancellationToken);
+                    internalApiOperations.Free(ctx.CancellationToken);
                 }
             });
         }
@@ -108,8 +109,9 @@ private static class Helpers
         {
             internal static void AnalyzeOperation(
                 IOperation operation,
-                PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool> publicApiOperations,
-                ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers)
+                PooledConcurrentDictionary<KeyValuePair<IOperation, ISymbol>, bool> internalApiOperations,
+                ConcurrentDictionary<ISymbol, InternalApiStatus> internalApiMembers,
+                INamedTypeSymbol? callingType)
             {
                 var symbol = GetOperationSymbol(operation);
 
@@ -184,9 +186,22 @@ void CheckTypeArgumentsCore(ImmutableArray<ITypeSymbol> typeArguments, PooledHas
 
                 void CheckOperationAttributes(ISymbol symbol, bool checkParents)
                 {
-                    if (TryGetOrCreatePublicApiAttributes(symbol, checkParents, publicApiMembers, out _))
+                    if (TryGetOrCreatePublicApiAttributes(symbol, checkParents, internalApiMembers, out var attributes))
                     {
-                        publicApiOperations.TryAdd(new KeyValuePair<IOperation, ISymbol>(operation, symbol), true);
+                        // If the marked member has [TestingAndPrivateOnly], allow calls from within the same type
+                        if (attributes.IsTestingAndPrivateOnly)
+                        {
+                            var targetType = symbol.ContainingType;
+
+                            // Allow calls from within the same type
+                            if (callingType != null && targetType != null &&
+                                SymbolEqualityComparer.Default.Equals(callingType, targetType))
+                            {
+                                return;
+                            }
+                        }
+
+                        internalApiOperations.TryAdd(new KeyValuePair<IOperation, ISymbol>(operation, symbol), true);
                     }
                 }
             }
@@ -238,11 +253,12 @@ void CheckOperationAttributes(ISymbol symbol, bool checkParents)
                 return iEvent;
             }
 
-            private static PublicApiStatus CopyAttributes(PublicApiStatus copyAttributes) =>
+            private static InternalApiStatus CopyAttributes(InternalApiStatus copyAttributes) =>
                 new()
                 {
                     IsAssemblyAttribute = copyAttributes.IsAssemblyAttribute,
-                    IsPublicApi = copyAttributes.IsPublicApi
+                    IsTestingApi = copyAttributes.IsTestingApi,
+                    IsTestingAndPrivateOnly = copyAttributes.IsTestingAndPrivateOnly
                 };
 
             private static bool IsWithinConditionalOperation(IFieldReferenceOperation pOperation) =>
@@ -260,8 +276,8 @@ or BinaryOperatorKind.LessThanOrEqual
             private static bool TryGetOrCreatePublicApiAttributes(
                 ISymbol symbol,
                 bool checkParents,
-                ConcurrentDictionary<ISymbol, PublicApiStatus> publicApiMembers,
-                out PublicApiStatus attributes)
+                ConcurrentDictionary<ISymbol, InternalApiStatus> publicApiMembers,
+                out InternalApiStatus attributes)
             {
                 if (!publicApiMembers.TryGetValue(symbol, out attributes))
                 {
@@ -281,34 +297,44 @@ private static bool TryGetOrCreatePublicApiAttributes(
                         }
                     }
 
-                    attributes ??= new PublicApiStatus() { IsAssemblyAttribute = symbol is IAssemblySymbol };
+                    attributes ??= new InternalApiStatus() { IsAssemblyAttribute = symbol is IAssemblySymbol };
                     MergePlatformAttributes(symbol.GetAttributes(), ref attributes);
                     attributes = publicApiMembers.GetOrAdd(symbol, attributes);
                 }
 
-                return attributes.IsPublicApi;
+                return attributes.IsTestingApi;
 
                 static void MergePlatformAttributes(
                     ImmutableArray<AttributeData> immediateAttributes,
-                    ref PublicApiStatus parentAttributes)
+                    ref InternalApiStatus parentAttributes)
                 {
                     foreach (AttributeData attribute in immediateAttributes)
                     {
-                        if (PublicApiAttributeNames.Contains(attribute.AttributeClass!.Name))
+                        var attributeName = attribute.AttributeClass!.Name;
+                        if (attributeName == TestingAndPrivateOnlyAttribute)
                         {
-                            parentAttributes.IsPublicApi = true;
+                            parentAttributes.IsTestingApi = true;
+                            parentAttributes.IsTestingAndPrivateOnly = true;
+                            return;
+                        }
+                        else if (attributeName == TestingOnlyAttribute)
+                        {
+                            parentAttributes.IsTestingApi = true;
+                            parentAttributes.IsTestingAndPrivateOnly = false;
                             return;
                         }
                     }
                 }
             }
         }
 
-        private sealed class PublicApiStatus
+        private sealed class InternalApiStatus
         {
             public bool IsAssemblyAttribute { get; set; }
 
-            public bool IsPublicApi { get; set; }
+            public bool IsTestingApi { get; set; }
+
+            public bool IsTestingAndPrivateOnly { get; set; }
         }
     }
 }

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

@@ -197,6 +197,8 @@ private string GetLogDirectory(int? pid)
             logDirectory = ProcessConfiguration.GetProcessLogDirectory(pid.Value);
         }
 
+#pragma warning disable DD0002
         return logDirectory ?? Logging.DatadogLoggingFactory.GetLogDirectory(NullConfigurationTelemetry.Instance);
+#pragma warning restore DD0002
     }
 }

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

@@ -12,6 +12,7 @@
 using Datadog.Trace.DogStatsd;
 using Datadog.Trace.Logging;
 using Datadog.Trace.PlatformHelpers;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 using Datadog.Trace.Util.Http;
@@ -94,7 +95,7 @@ public Task<bool> SendTracesAsync(ArraySegment<byte> traces, int numberOfTraces,
             return SendWithRetry(_tracesEndpoint, _sendTraces, state);
         }
 
-        // internal for testing
+        [TestingAndPrivateOnly]
         internal bool LogPartialFlushWarningIfRequired(string agentVersion)
         {
             if (agentVersion != _agentVersion)

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

@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Datadog.Trace.Logging;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Util;
 
 namespace Datadog.Trace.Agent
@@ -106,6 +107,7 @@ public void CancelUpdates()
         /// <summary>
         /// Update the current rate. Internal for testing only. Should not be called in normal usage.
         /// </summary>
+        [TestingAndPrivateOnly]
         internal void UpdateBucket()
         {
             var index = _index;

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

@@ -13,6 +13,7 @@
 using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
 using Datadog.Trace.Configuration.Telemetry;
 using Datadog.Trace.LibDatadog;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Util;
 
@@ -361,7 +362,7 @@ internal void SetDefaultManualInstrumentationSettings()
         private TracerSettings InitializeTracerSettings()
             => InitializeTracerSettings(GlobalConfigurationSource.Instance);
 
-        // Internal for testing
+        [TestingAndPrivateOnly]
         internal TracerSettings InitializeTracerSettings(IConfigurationSource source)
         {
             // 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

@@ -9,6 +9,7 @@
 using System.Reflection.Emit;
 using Datadog.Trace.DuckTyping;
 using Datadog.Trace.Logging;
+using Datadog.Trace.SourceGenerators;
 
 namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.ILogger.DirectSubmission
 {
@@ -17,7 +18,7 @@ internal static class LoggerFactoryIntegrationCommon<TLoggerFactory>
         private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(LoggerFactoryIntegrationCommon<TLoggerFactory>));
 
         // ReSharper disable once StaticMemberInGenericType
-        // Internal for testing
+        [TestingAndPrivateOnly]
         internal static readonly Type? ProviderInterfaces;
 
         static LoggerFactoryIntegrationCommon()
@@ -84,7 +85,7 @@ internal static bool TryAddDirectSubmissionLoggerProvider(TLoggerFactory loggerF
             return TryAddDirectSubmissionLoggerProvider(loggerFactory, provider);
         }
 
-        // Internal for testing
+        [TestingAndPrivateOnly]
         internal static bool TryAddDirectSubmissionLoggerProvider(TLoggerFactory loggerFactory, DirectSubmissionLoggerProvider provider)
         {
             if (ProviderInterfaces is null)

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

@@ -8,6 +8,7 @@
 using Datadog.Trace.DuckTyping;
 using Datadog.Trace.Logging.DirectSubmission;
 using Datadog.Trace.Logging.DirectSubmission.Sink;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 
@@ -23,7 +24,7 @@ internal class DirectSubmissionLog4NetAppender
         private readonly IDirectSubmissionLogSink _sink;
         private readonly DirectSubmissionLogLevel _minimumLevel;
 
-        // internal for testing
+        [TestingAndPrivateOnly]
         internal DirectSubmissionLog4NetAppender(IDirectSubmissionLogSink sink, DirectSubmissionLogLevel minimumLevel)
         {
             _sink = sink;

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

@@ -8,6 +8,7 @@
 using Datadog.Trace.DuckTyping;
 using Datadog.Trace.Logging.DirectSubmission;
 using Datadog.Trace.Logging.DirectSubmission.Sink;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 
@@ -23,7 +24,7 @@ internal class DirectSubmissionLog4NetLegacyAppender
         private readonly IDirectSubmissionLogSink _sink;
         private readonly DirectSubmissionLogLevel _minimumLevel;
 
-        // internal for testing
+        [TestingAndPrivateOnly]
         internal DirectSubmissionLog4NetLegacyAppender(IDirectSubmissionLogSink sink, DirectSubmissionLogLevel minimumLevel)
         {
             _sink = sink;

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

@@ -12,6 +12,7 @@
 using Datadog.Trace.Logging.DirectSubmission;
 using Datadog.Trace.Logging.DirectSubmission.Formatting;
 using Datadog.Trace.Logging.DirectSubmission.Sink;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 
@@ -32,7 +33,7 @@ internal DirectSubmissionNLogLegacyTarget(IDirectSubmissionLogSink sink, DirectS
         {
         }
 
-        // internal for testing
+        [TestingAndPrivateOnly]
         internal DirectSubmissionNLogLegacyTarget(
             IDirectSubmissionLogSink sink,
             DirectSubmissionLogLevel minimumLevel,

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

@@ -10,6 +10,7 @@
 using Datadog.Trace.Logging.DirectSubmission;
 using Datadog.Trace.Logging.DirectSubmission.Formatting;
 using Datadog.Trace.Logging.DirectSubmission.Sink;
+using Datadog.Trace.SourceGenerators;
 using Datadog.Trace.Telemetry;
 using Datadog.Trace.Telemetry.Metrics;
 
@@ -30,7 +31,7 @@ internal DirectSubmissionNLogTarget(IDirectSubmissionLogSink sink, DirectSubmiss
         {
         }
 
-        // internal for testing
+        [TestingAndPrivateOnly]
         internal DirectSubmissionNLogTarget(
             IDirectSubmissionLogSink sink,
             DirectSubmissionLogLevel minimumLevel,