refactor tokencredential (Azure#35019)

Timothy Mothra · web-flow · commit 23bad1bbd7d9 · 2023-03-21T15:21:27.000-07:00
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterLoggingExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterLoggingExtensions.cs
@@ -18,7 +18,11 @@ public static class AzureMonitorExporterLoggingExtensions
         /// </summary>
         /// <param name="loggerOptions"><see cref="OpenTelemetryLoggerOptions"/> options to use.</param>
         /// <param name="configure">Exporter configuration options.</param>
-        /// <param name="credential"><see cref="TokenCredential" /></param>
+        /// <param name="credential">
+        /// An Azure <see cref="TokenCredential" /> capable of providing an OAuth token.
+        /// Note: if a credential is provided to both <see cref="AzureMonitorExporterOptions"/> and this parameter,
+        /// the Options will take precedence.
+        /// </param>
         /// <returns>The instance of <see cref="OpenTelemetryLoggerOptions"/> to chain the calls.</returns>
         public static OpenTelemetryLoggerOptions AddAzureMonitorLogExporter(this OpenTelemetryLoggerOptions loggerOptions, Action<AzureMonitorExporterOptions>? configure = null, TokenCredential? credential = null)
         {
@@ -34,7 +38,14 @@ public static OpenTelemetryLoggerOptions AddAzureMonitorLogExporter(this OpenTel
             var options = new AzureMonitorExporterOptions();
             configure?.Invoke(options);
 
-            return loggerOptions.AddProcessor(new BatchLogRecordExportProcessor(new AzureMonitorLogExporter(options, options.Credential ?? credential)));
+            if (credential != null)
+            {
+                // Credential can be set by either AzureMonitorExporterOptions or Extension Method Parameter.
+                // Options should take precedence.
+                options.Credential ??= credential;
+            }
+
+            return loggerOptions.AddProcessor(new BatchLogRecordExportProcessor(new AzureMonitorLogExporter(options)));
         }
     }
 }
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterMetricExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterMetricExtensions.cs
@@ -20,7 +20,11 @@ public static class AzureMonitorExporterMetricExtensions
         /// </summary>
         /// <param name="builder"><see cref="MeterProviderBuilder"/> builder to use.</param>
         /// <param name="configure">Exporter configuration options.</param>
-        /// <param name="credential"><see cref="TokenCredential" /></param>
+        /// <param name="credential">
+        /// An Azure <see cref="TokenCredential" /> capable of providing an OAuth token.
+        /// Note: if a credential is provided to both <see cref="AzureMonitorExporterOptions"/> and this parameter,
+        /// the Options will take precedence.
+        /// </param>
         /// <param name="name">Name which is used when retrieving options.</param>
         /// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
         public static MeterProviderBuilder AddAzureMonitorMetricExporter(
@@ -59,7 +63,14 @@ public static MeterProviderBuilder AddAzureMonitorMetricExporter(
                     configure(exporterOptions);
                 }
 
-                return new PeriodicExportingMetricReader(new AzureMonitorMetricExporter(exporterOptions, exporterOptions.Credential ?? credential))
+                if (credential != null)
+                {
+                    // Credential can be set by either AzureMonitorExporterOptions or Extension Method Parameter.
+                    // Options should take precedence.
+                    exporterOptions.Credential ??= credential;
+                }
+
+                return new PeriodicExportingMetricReader(new AzureMonitorMetricExporter(exporterOptions))
                            { TemporalityPreference = MetricReaderTemporalityPreference.Delta };
             });
         }
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterTraceExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorExporterTraceExtensions.cs
@@ -22,7 +22,11 @@ public static class AzureMonitorExporterTraceExtensions
         /// </summary>
         /// <param name="builder"><see cref="TracerProviderBuilder"/> builder to use.</param>
         /// <param name="configure">Callback action for configuring <see cref="AzureMonitorExporterOptions"/>.</param>
-        /// <param name="credential"><see cref="TokenCredential" /></param>
+        /// <param name="credential">
+        /// An Azure <see cref="TokenCredential" /> capable of providing an OAuth token.
+        /// Note: if a credential is provided to both <see cref="AzureMonitorExporterOptions"/> and this parameter,
+        /// the Options will take precedence.
+        /// </param>
         /// <param name="name">Name which is used when retrieving options.</param>
         /// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
         public static TracerProviderBuilder AddAzureMonitorTraceExporter(
@@ -59,10 +63,17 @@ public static TracerProviderBuilder AddAzureMonitorTraceExporter(
                     configure(exporterOptions);
                 }
 
+                if (credential != null)
+                {
+                    // Credential can be set by either AzureMonitorExporterOptions or Extension Method Parameter.
+                    // Options should take precedence.
+                    exporterOptions.Credential ??= credential;
+                }
+
                 return new CompositeProcessor<Activity>(new BaseProcessor<Activity>[]
                 {
                     new StandardMetricsExtractionProcessor(),
-                    new BatchActivityExportProcessor(new AzureMonitorTraceExporter(exporterOptions, exporterOptions.Credential?? credential))
+                    new BatchActivityExportProcessor(new AzureMonitorTraceExporter(exporterOptions))
                 });
             });
         }
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorLogExporter.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorLogExporter.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.Threading;
-using Azure.Core;
 using Azure.Core.Pipeline;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals.PersistentStorage;
@@ -20,7 +19,7 @@ internal class AzureMonitorLogExporter : BaseExporter<LogRecord>
         private AzureMonitorResource? _resource;
         private bool _disposed;
 
-        public AzureMonitorLogExporter(AzureMonitorExporterOptions options, TokenCredential? credential = null) : this(TransmitterFactory.Instance.Get(options, credential))
+        public AzureMonitorLogExporter(AzureMonitorExporterOptions options) : this(TransmitterFactory.Instance.Get(options))
         {
         }
 
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorMetricExporter.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorMetricExporter.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.Threading;
-using Azure.Core;
 using Azure.Core.Pipeline;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals.PersistentStorage;
@@ -20,7 +19,7 @@ internal class AzureMonitorMetricExporter : BaseExporter<Metric>
         private AzureMonitorResource? _resource;
         private bool _disposed;
 
-        public AzureMonitorMetricExporter(AzureMonitorExporterOptions options, TokenCredential? credential = null) : this(TransmitterFactory.Instance.Get(options, credential))
+        public AzureMonitorMetricExporter(AzureMonitorExporterOptions options) : this(TransmitterFactory.Instance.Get(options))
         {
         }
 
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorTraceExporter.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorTraceExporter.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Diagnostics;
 using System.Threading;
-using Azure.Core;
 using Azure.Core.Pipeline;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals;
 using Azure.Monitor.OpenTelemetry.Exporter.Internals.PersistentStorage;
@@ -20,7 +19,7 @@ internal class AzureMonitorTraceExporter : BaseExporter<Activity>
         private AzureMonitorResource? _resource;
         private bool _disposed;
 
-        public AzureMonitorTraceExporter(AzureMonitorExporterOptions options, TokenCredential? credential = null) : this(TransmitterFactory.Instance.Get(options, credential))
+        public AzureMonitorTraceExporter(AzureMonitorExporterOptions options) : this(TransmitterFactory.Instance.Get(options))
         {
         }
 
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/AzureMonitorTransmitter.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/AzureMonitorTransmitter.cs
@@ -30,7 +30,7 @@ internal class AzureMonitorTransmitter : ITransmitter
         private readonly ConnectionVars _connectionVars;
         private bool _disposed;
 
-        public AzureMonitorTransmitter(AzureMonitorExporterOptions options, TokenCredential? credential = null)
+        public AzureMonitorTransmitter(AzureMonitorExporterOptions options)
         {
             if (options == null)
             {
@@ -41,7 +41,7 @@ public AzureMonitorTransmitter(AzureMonitorExporterOptions options, TokenCredent
 
             _connectionVars = InitializeConnectionVars(options);
 
-            _applicationInsightsRestClient = InitializeRestClient(options, _connectionVars, credential);
+            _applicationInsightsRestClient = InitializeRestClient(options, _connectionVars);
 
             _fileBlobProvider = InitializeOfflineStorage(options);
 
@@ -67,21 +67,21 @@ private static ConnectionVars InitializeConnectionVars(AzureMonitorExporterOptio
             throw new InvalidOperationException("A connection string was not found. Please set your connection string.");
         }
 
-        private static ApplicationInsightsRestClient InitializeRestClient(AzureMonitorExporterOptions options, ConnectionVars connectionVars, TokenCredential? credential)
+        private static ApplicationInsightsRestClient InitializeRestClient(AzureMonitorExporterOptions options, ConnectionVars connectionVars)
         {
             HttpPipeline pipeline;
 
-            if (credential != null)
+            if (options.Credential != null)
             {
                 var scope = AadHelper.GetScope(connectionVars.AadAudience);
                 var httpPipelinePolicy = new HttpPipelinePolicy[]
                 {
-                    new BearerTokenAuthenticationPolicy(credential, scope),
+                    new BearerTokenAuthenticationPolicy(options.Credential, scope),
                     new IngestionRedirectPolicy()
                 };
 
                 pipeline = HttpPipelineBuilder.Build(options, httpPipelinePolicy);
-                AzureMonitorExporterEventSource.Log.WriteInformational("SetAADCredentialsToPipeline", $"HttpPipelineBuilder is built with AAD Credentials. TokenCredential: {credential.GetType().Name} Scope: {scope}");
+                AzureMonitorExporterEventSource.Log.WriteInformational("SetAADCredentialsToPipeline", $"HttpPipelineBuilder is built with AAD Credentials. TokenCredential: {options.Credential.GetType().Name} Scope: {scope}");
             }
             else
             {
diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TransmitterFactory.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TransmitterFactory.cs
@@ -19,7 +19,7 @@ internal class TransmitterFactory
         internal readonly Dictionary<string, AzureMonitorTransmitter> _transmitters = new();
         private readonly object _lockObj = new();
 
-        public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExporterOptions, TokenCredential? tokenCredential = null)
+        public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExporterOptions)
         {
             var key = azureMonitorExporterOptions.ConnectionString ?? string.Empty;
 
@@ -29,7 +29,7 @@ public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExpor
                 {
                     if (!_transmitters.TryGetValue(key, out transmitter))
                     {
-                        transmitter = new AzureMonitorTransmitter(azureMonitorExporterOptions, tokenCredential);
+                        transmitter = new AzureMonitorTransmitter(azureMonitorExporterOptions);
 
                         _transmitters.Add(key, transmitter);
                     }

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,6 @@`
`3`	`3`
`4`	`4`	`using System;`
`5`	`5`	`using System.Threading;`
`6`		`-using Azure.Core;`
`7`	`6`	`using Azure.Core.Pipeline;`
`8`	`7`	`using Azure.Monitor.OpenTelemetry.Exporter.Internals;`
`9`	`8`	`using Azure.Monitor.OpenTelemetry.Exporter.Internals.PersistentStorage;`
`@@ -20,7 +19,7 @@ internal class AzureMonitorLogExporter : BaseExporter<LogRecord>`
`20`	`19`	`private AzureMonitorResource? _resource;`
`21`	`20`	`private bool _disposed;`
`22`	`21`
`23`		`- public AzureMonitorLogExporter(AzureMonitorExporterOptions options, TokenCredential? credential = null) : this(TransmitterFactory.Instance.Get(options, credential))`
	`22`	`+ public AzureMonitorLogExporter(AzureMonitorExporterOptions options) : this(TransmitterFactory.Instance.Get(options))`
`24`	`23`	`{`
`25`	`24`	`}`
`26`	`25`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@`
`4`	`4`	`using System;`
`5`	`5`	`using System.Diagnostics;`
`6`	`6`	`using System.Threading;`
`7`		`-using Azure.Core;`
`8`	`7`	`using Azure.Core.Pipeline;`
`9`	`8`	`using Azure.Monitor.OpenTelemetry.Exporter.Internals;`
`10`	`9`	`using Azure.Monitor.OpenTelemetry.Exporter.Internals.PersistentStorage;`
`@@ -20,7 +19,7 @@ internal class AzureMonitorTraceExporter : BaseExporter<Activity>`
`20`	`19`	`private AzureMonitorResource? _resource;`
`21`	`20`	`private bool _disposed;`
`22`	`21`
`23`		`- public AzureMonitorTraceExporter(AzureMonitorExporterOptions options, TokenCredential? credential = null) : this(TransmitterFactory.Instance.Get(options, credential))`
	`22`	`+ public AzureMonitorTraceExporter(AzureMonitorExporterOptions options) : this(TransmitterFactory.Instance.Get(options))`
`24`	`23`	`{`
`25`	`24`	`}`
`26`	`25`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ internal class AzureMonitorTransmitter : ITransmitter`
`30`	`30`	`private readonly ConnectionVars _connectionVars;`
`31`	`31`	`private bool _disposed;`
`32`	`32`
`33`		`- public AzureMonitorTransmitter(AzureMonitorExporterOptions options, TokenCredential? credential = null)`
	`33`	`+ public AzureMonitorTransmitter(AzureMonitorExporterOptions options)`
`34`	`34`	`{`
`35`	`35`	`if (options == null)`
`36`	`36`	`{`
`@@ -41,7 +41,7 @@ public AzureMonitorTransmitter(AzureMonitorExporterOptions options, TokenCredent`
`41`	`41`
`42`	`42`	`_connectionVars = InitializeConnectionVars(options);`
`43`	`43`
`44`		`- _applicationInsightsRestClient = InitializeRestClient(options, _connectionVars, credential);`
	`44`	`+ _applicationInsightsRestClient = InitializeRestClient(options, _connectionVars);`
`45`	`45`
`46`	`46`	`_fileBlobProvider = InitializeOfflineStorage(options);`
`47`	`47`
`@@ -67,21 +67,21 @@ private static ConnectionVars InitializeConnectionVars(AzureMonitorExporterOptio`
`67`	`67`	`throw new InvalidOperationException("A connection string was not found. Please set your connection string.");`
`68`	`68`	`}`
`69`	`69`
`70`		`- private static ApplicationInsightsRestClient InitializeRestClient(AzureMonitorExporterOptions options, ConnectionVars connectionVars, TokenCredential? credential)`
	`70`	`+ private static ApplicationInsightsRestClient InitializeRestClient(AzureMonitorExporterOptions options, ConnectionVars connectionVars)`
`71`	`71`	`{`
`72`	`72`	`HttpPipeline pipeline;`
`73`	`73`
`74`		`- if (credential != null)`
	`74`	`+ if (options.Credential != null)`
`75`	`75`	`{`
`76`	`76`	`var scope = AadHelper.GetScope(connectionVars.AadAudience);`
`77`	`77`	`var httpPipelinePolicy = new HttpPipelinePolicy[]`
`78`	`78`	`{`
`79`		`- new BearerTokenAuthenticationPolicy(credential, scope),`
	`79`	`+ new BearerTokenAuthenticationPolicy(options.Credential, scope),`
`80`	`80`	`new IngestionRedirectPolicy()`
`81`	`81`	`};`
`82`	`82`
`83`	`83`	`pipeline = HttpPipelineBuilder.Build(options, httpPipelinePolicy);`
`84`		`- AzureMonitorExporterEventSource.Log.WriteInformational("SetAADCredentialsToPipeline", $"HttpPipelineBuilder is built with AAD Credentials. TokenCredential: {credential.GetType().Name} Scope: {scope}");`
	`84`	`+ AzureMonitorExporterEventSource.Log.WriteInformational("SetAADCredentialsToPipeline", $"HttpPipelineBuilder is built with AAD Credentials. TokenCredential: {options.Credential.GetType().Name} Scope: {scope}");`
`85`	`85`	`}`
`86`	`86`	`else`
`87`	`87`	`{`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ internal class TransmitterFactory`
`19`	`19`	`internal readonly Dictionary<string, AzureMonitorTransmitter> _transmitters = new();`
`20`	`20`	`private readonly object _lockObj = new();`
`21`	`21`
`22`		`- public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExporterOptions, TokenCredential? tokenCredential = null)`
	`22`	`+ public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExporterOptions)`
`23`	`23`	`{`
`24`	`24`	`var key = azureMonitorExporterOptions.ConnectionString ?? string.Empty;`
`25`	`25`
`@@ -29,7 +29,7 @@ public AzureMonitorTransmitter Get(AzureMonitorExporterOptions azureMonitorExpor`
`29`	`29`	`{`
`30`	`30`	`if (!_transmitters.TryGetValue(key, out transmitter))`
`31`	`31`	`{`
`32`		`- transmitter = new AzureMonitorTransmitter(azureMonitorExporterOptions, tokenCredential);`
	`32`	`+ transmitter = new AzureMonitorTransmitter(azureMonitorExporterOptions);`
`33`	`33`
`34`	`34`	`_transmitters.Add(key, transmitter);`
`35`	`35`	`}`