Skip to content

Commit

Permalink
EnableDataCollection and DisplayBreakingChangeWarnings configs (#318)
Browse files Browse the repository at this point in the history
* configs: data collection; survey; warning message

* revert survey config

* do not obsolete AzurePSDataCollectionProfile(bool enable)

* remove s in DisplayBreakingChangeWarnings

* Add a config scope: Environment

* refactor breaking change / preview attribute

* add scope to config filter
  • Loading branch information
isra-fel authored May 17, 2022
1 parent a346bb0 commit 8d70507
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 124 deletions.
32 changes: 28 additions & 4 deletions src/Authentication.Abstractions/AzurePSDataCollectionProfile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Newtonsoft.Json;
using Microsoft.Azure.PowerShell.Common.Config;
using System;

namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
Expand All @@ -23,16 +23,40 @@ public class AzurePSDataCollectionProfile
OldDefaultFileName = "AzureDataCollectionProfile.json",
DefaultFileName = "AzurePSDataCollectionProfile.json";

private bool? _enableAzureDataCollection = null;

/// <summary>
/// Creates a data collection profile who relies on config API to determine whether it is enabled.
/// </summary>
public AzurePSDataCollectionProfile()
{
}

/// <summary>
/// Creates a data collection profile with a predefined state of whether it is enabled.
/// </summary>
public AzurePSDataCollectionProfile(bool enable)
{
EnableAzureDataCollection = enable;
_enableAzureDataCollection = enable;
}

[JsonProperty(PropertyName = "enableAzureDataCollection")]
public bool? EnableAzureDataCollection { get; set; }
public bool? EnableAzureDataCollection {
get
{
if (_enableAzureDataCollection.HasValue)
{
return _enableAzureDataCollection.Value;
}
if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager))
{
_enableAzureDataCollection = configManager.GetConfigValue<bool>(ConfigKeysForCommon.EnableDataCollection);
}
return _enableAzureDataCollection;
}
set
{
throw new NotSupportedException("Setting data collection directly is not supported. Use Config API instead.");
}
}
}
}
73 changes: 4 additions & 69 deletions src/Authentication.Abstractions/DataCollectionController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using Newtonsoft.Json;
using Microsoft.Azure.PowerShell.Common.Config;
using System;
using System.IO;

namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions
{
Expand All @@ -25,60 +24,12 @@ public abstract class DataCollectionController

static AzurePSDataCollectionProfile Initialize(IAzureSession session)
{
AzurePSDataCollectionProfile result = new AzurePSDataCollectionProfile(true);
try
{
var environmentValue = Environment.GetEnvironmentVariable(AzurePSDataCollectionProfile.EnvironmentVariableName);
bool enabled = true;
if (!string.IsNullOrWhiteSpace(environmentValue) && bool.TryParse(environmentValue, out enabled))
{
result.EnableAzureDataCollection = enabled;
}
else
{
var store = session.DataStore;
string dataPath = Path.Combine(session.ProfileDirectory, AzurePSDataCollectionProfile.DefaultFileName);
if (store.FileExists(dataPath))
{
string contents = store.ReadFileAsText(dataPath);
var localResult = JsonConvert.DeserializeObject<AzurePSDataCollectionProfile>(contents);
if (localResult != null && localResult.EnableAzureDataCollection.HasValue)
{
result = localResult;
}
}
else
{
WritePSDataCollectionProfile(session, new AzurePSDataCollectionProfile(true));
}
}
}
catch
{
// do not throw for i/o or serialization errors
}

return result;
return new AzurePSDataCollectionProfile();
}

[Obsolete("Use config API to update data collection settings.")]
public static void WritePSDataCollectionProfile(IAzureSession session, AzurePSDataCollectionProfile profile)
{
try
{
var store = session.DataStore;
string dataPath = Path.Combine(session.ProfileDirectory, AzurePSDataCollectionProfile.DefaultFileName);
if (!store.DirectoryExists(session.ProfileDirectory))
{
store.CreateDirectory(session.ProfileDirectory);
}

string contents = JsonConvert.SerializeObject(profile);
store.WriteFile(dataPath, contents);
}
catch
{
// do not throw for i/o or serialization errors
}
}

public static DataCollectionController Create(IAzureSession session)
Expand All @@ -93,33 +44,17 @@ public static DataCollectionController Create(AzurePSDataCollectionProfile profi

class MemoryDataCollectionController : DataCollectionController
{
object _lock;
bool? _enabled;

public MemoryDataCollectionController()
{
_lock = new object();
_enabled = null;
}

public MemoryDataCollectionController(AzurePSDataCollectionProfile enabled)
{
_lock = new object();
_enabled = enabled?.EnableAzureDataCollection;
}

public override AzurePSDataCollectionProfile GetProfile(Action warningWriter)
{
lock (_lock)
{
if (!_enabled.HasValue)
{
_enabled = true;
warningWriter();
}

return new AzurePSDataCollectionProfile(_enabled.Value);
}
return new AzurePSDataCollectionProfile();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Collections.Generic;

namespace Microsoft.Azure.PowerShell.Common.Config
Expand Down
30 changes: 29 additions & 1 deletion src/Authentication.Abstractions/Models/ConfigDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,35 @@ public abstract class ConfigDefinition
/// <summary>
/// Gets the name of the environment variable that can control this config.
/// </summary>
public virtual string EnvironmentVariableName { get; } = null;
/// <remarks>
/// For most cases, where the config is connected to only 1 environment variable,
/// and the value can be parsed without extra logic, use this property,
/// else override <see cref="ParseFromEnvironmentVariables(IReadOnlyDictionary{string, string})"/>.
/// </remarks>
protected virtual string EnvironmentVariableName { get; } = null;

/// <summary>
/// Customizes how to parse config value from environment variables.
/// </summary>
/// <param name="environmentVariables">All the environment variables.</param>
/// <returns>The parsed config value, in string. Returns null if the environment variable of this config is not set.</returns>
/// <remarks>
/// Note: use <see cref="EnvironmentVariableName"/> if there's no need for customized parsing logic. <br />
/// The return type is string because it will be further parsed into other types (int, bool...) by the config provider.
/// Make sure the return value matches <see cref="ValueType"/>.
/// For example if <see cref="ValueType"/> is bool, the return value cannot be like "Disabled".
/// </remarks>
public virtual string ParseFromEnvironmentVariables(IReadOnlyDictionary<string, string> environmentVariables)
{
if (string.IsNullOrEmpty(EnvironmentVariableName))
{
return null;
}
else
{
return environmentVariables.TryGetValue(EnvironmentVariableName, out string value) ? value : null;
}
}

/// <summary>
/// Gets how the config can be applied to.
Expand Down
6 changes: 6 additions & 0 deletions src/Authentication.Abstractions/Models/ConfigFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,11 @@ public class ConfigFilter
/// - Name of a cmdlet: the config applies to a certain cmdlet of Azure PowerShell. For example, "Get-AzKeyVault".
/// </remarks>
public string AppliesTo { get; set; } = null;

/// <summary>
/// Scope of the configs to filter. By default all scopes are included.
/// </summary>
/// <value></value>
public ConfigScope? Scope { get; set; } = null;
}
}
31 changes: 31 additions & 0 deletions src/Authentication.Abstractions/Models/ConfigKeysForCommon.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// ----------------------------------------------------------------------------------
//
// Copyright Microsoft Corporation
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------------------------------------------------------------

namespace Microsoft.Azure.PowerShell.Common.Config
{
/// <summary>
/// This class stores keys of pre-defined configs.
/// </summary>
/// <remarks>
/// All keys should be defined in ConfigKeys class in Azure/azure-powershell repo.
/// If the key is used in common code, duplicate it here.
/// Keys defined here should NEVER be removed or changed to prevent breaking change.
/// </remarks>
public static class ConfigKeysForCommon
{
public const string EnableInterceptSurvey = "EnableInterceptSurvey";
public const string DisplayBreakingChangeWarning = "DisplayBreakingChangeWarning";
public const string EnableDataCollection = "EnableDataCollection";
}
}
10 changes: 8 additions & 2 deletions src/Authentication.Abstractions/Models/ConfigScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Microsoft.Azure.PowerShell.Common.Config
public enum ConfigScope
{
/// <summary>
/// Config will be persitent on the disk, available for all the PowerShell sessions initiated by the current user.
/// Config will be persistent on the disk, available for all the PowerShell sessions initiated by the current user.
/// </summary>
CurrentUser,

Expand All @@ -33,6 +33,12 @@ public enum ConfigScope
/// Config is never set.
/// </summary>
/// <remarks>This option is not available when updating or clearing a config.</remarks>
Default
Default,

/// <summary>
/// Config is set by environment variables.
/// </summary>
/// <remarks>This option is not available when updating or clearing a config.</remarks>
Environment
}
}
24 changes: 19 additions & 5 deletions src/Common/AzurePSCmdlet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

using Microsoft.Azure.Commands.Common.Authentication;
using Microsoft.Azure.Commands.Common.Authentication.Abstractions;
using Microsoft.Azure.PowerShell.Common.Config;
using Microsoft.Azure.PowerShell.Common.Share.Survey;
using Microsoft.Azure.ServiceManagement.Common.Models;
using Microsoft.WindowsAzure.Commands.Common;
Expand Down Expand Up @@ -65,7 +66,7 @@ protected AzurePSDataCollectionProfile _dataCollectionProfile
}
else if (_cachedProfile == null)
{
_cachedProfile = new AzurePSDataCollectionProfile(true);
_cachedProfile = new AzurePSDataCollectionProfile();
WriteWarning(DataCollectionWarning);
}

Expand Down Expand Up @@ -361,8 +362,17 @@ protected override void BeginProcessing()

//Now see if the cmdlet has any Breaking change attributes on it and process them if it does
//This will print any breaking change attribute messages that are applied to the cmdlet
BreakingChangeAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteDebug);
WriteBreakingChangeOrPreviewMessage();
}

private void WriteBreakingChangeOrPreviewMessage()
{
if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
&& configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisplayBreakingChangeWarning))
{
BreakingChangeAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteDebug);
}
}

/// <summary>
Expand Down Expand Up @@ -451,7 +461,7 @@ protected void WriteSurvey()
Message = "Open-AzSurveyLink",
NoNewLine = true,
};
}
}
HostInformationMessage action = new HostInformationMessage()
{
Message = " to fill out a short Survey"
Expand All @@ -471,7 +481,11 @@ protected void WriteSurvey()
_qosEvent.IsSuccess = false;
}
base.WriteError(errorRecord);
PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
if (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
&& configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisplayBreakingChangeWarning))
{
PreviewAttributeHelper.ProcessCustomAttributesAtRuntime(this.GetType(), this.MyInvocation, WriteWarning);
}
}

protected new void ThrowTerminatingError(ErrorRecord errorRecord)
Expand Down
Loading

0 comments on commit 8d70507

Please sign in to comment.