Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MetricsAdvisor] Made AnomalyAlertConfiguration constructor parameterless #18458

Merged
merged 4 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions sdk/metricsadvisor/Azure.AI.MetricsAdvisor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@
- The constructor of the `AnomalyDetectionConfiguration` class is now parameterless. Required properties should be set via setters.
- The constructor of the `MetricSingleSeriesDetectionCondition` class is now parameterless. Dimension columns can be added directly to `SeriesKey`.
- The constructor of the `MetricSeriesGroupDetectionCondition` class is now parameterless. Dimension columns can be added directly to `SeriesGroupKey`.
- The constructor of the `AnomalyAlertConfiguration` class is now parameterless. Required properties should be set via setters.
- In `DataFeed`, added property setters to `Name`, `DataSource`, `Granularity`, `IngestionSettings`, and `Schema`.
- In `DataFeedIngestionSettings`, added a property setter to `IngestionStartTime`.
- In `AnomalyDetectionConfiguration`, added property setters to `MetricId`, `Name`, and `WholeSeriesDetectionConditions`.
- In `AnomalyAlertConfiguration`, added a property setter to `Name`.
- In `MetricAnomalyAlertSnoozeCondition`, added property setters to `AutoSnooze`, `IsOnlyForSuccessive`, and `SnoozeScope`.
- In `MetricBoundaryCondition`, added a property setter to `Direction`.
- In `SeverityCondition`, added property setters to `MaximumAlertSeverity` and `MinimumAlertSeverity`.
- In `DataFeed`, removed the setters of the properties `Administrators` and `Viewers`.
- In `DataFeedSchema`, removed the setter of the property `DimensionColumns`.
- In `DataFeedRollupSettings`, removed the setter of the property `AutoRollupGroupByColumnNames`.
Expand All @@ -23,6 +28,7 @@
- `DataFeed.IngestionStartTime` is now nullable.
- `MetricsAdvisorAdministrationClient.CreateDataFeed` sync and async methods now throw an `ArgumentException` if required properties are not set properly.
- `MetricsAdvisorAdministrationClient.CreateDetectionConfiguration` sync and async methods now throw an `ArgumentException` if required properties are not set properly.
- `MetricsAdvisorAdministrationClient.CreateAlertConfiguration` sync and async methods now throw an `ArgumentException` if required properties are not set properly.
- In `MetricsAdvisorKeyCredential`, renamed the parameter `key` to `subscriptionKey` in the method `UpdateSubscriptionKey`.
- In `MetricsAdvisorKeyCredential`, renamed the parameter `key` to `apiKey` in the method `UpdateApiKey`.

Expand Down
13 changes: 8 additions & 5 deletions sdk/metricsadvisor/Azure.AI.MetricsAdvisor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,15 +334,18 @@ string hookId = "<hookId>";
string anomalyDetectionConfigurationId = "<anomalyDetectionConfigurationId>";

string configurationName = "Sample anomaly alert configuration";
var idsOfHooksToAlert = new List<string>() { hookId };

var scope = MetricAnomalyAlertScope.GetScopeForWholeSeries();
var metricAlertConfigurations = new List<MetricAnomalyAlertConfiguration>()
AnomalyAlertConfiguration alertConfiguration = new AnomalyAlertConfiguration()
{
new MetricAnomalyAlertConfiguration(anomalyDetectionConfigurationId, scope)
Name = configurationName
};

AnomalyAlertConfiguration alertConfiguration = new AnomalyAlertConfiguration(configurationName, idsOfHooksToAlert, metricAlertConfigurations);
alertConfiguration.IdsOfHooksToAlert.Add(hookId);

var scope = MetricAnomalyAlertScope.GetScopeForWholeSeries();
var metricAlertConfiguration = new MetricAnomalyAlertConfiguration(anomalyDetectionConfigurationId, scope);

alertConfiguration.MetricAlertConfigurations.Add(metricAlertConfiguration);

Response<string> response = await adminClient.CreateAlertConfigurationAsync(alertConfiguration);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,13 @@ internal AnomalyAlert() { }
}
public partial class AnomalyAlertConfiguration
{
public AnomalyAlertConfiguration(string name, System.Collections.Generic.IList<string> idsOfHooksToAlert, System.Collections.Generic.IList<Azure.AI.MetricsAdvisor.Models.MetricAnomalyAlertConfiguration> metricAlertConfigurations) { }
public AnomalyAlertConfiguration() { }
public Azure.AI.MetricsAdvisor.Models.MetricAnomalyAlertConfigurationsOperator? CrossMetricsOperator { get { throw null; } set { } }
public string Description { get { throw null; } set { } }
public string Id { get { throw null; } }
public System.Collections.Generic.IList<string> IdsOfHooksToAlert { get { throw null; } }
public System.Collections.Generic.IList<Azure.AI.MetricsAdvisor.Models.MetricAnomalyAlertConfiguration> MetricAlertConfigurations { get { throw null; } }
public string Name { get { throw null; } }
public string Name { get { throw null; } set { } }
}
public partial class AnomalyDetectionConfiguration
{
Expand Down Expand Up @@ -957,9 +957,9 @@ internal MetricAnomalyAlertScope() { }
public partial class MetricAnomalyAlertSnoozeCondition
{
public MetricAnomalyAlertSnoozeCondition(int autoSnooze, Azure.AI.MetricsAdvisor.Models.SnoozeScope snoozeScope, bool isOnlyForSuccessive) { }
public int AutoSnooze { get { throw null; } }
public bool IsOnlyForSuccessive { get { throw null; } }
public Azure.AI.MetricsAdvisor.Models.SnoozeScope SnoozeScope { get { throw null; } }
public int AutoSnooze { get { throw null; } set { } }
public bool IsOnlyForSuccessive { get { throw null; } set { } }
public Azure.AI.MetricsAdvisor.Models.SnoozeScope SnoozeScope { get { throw null; } set { } }
Comment on lines +960 to +962
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding setters to some properties to make it easier for users to update them (instead of having to create a whole new object). They're not nullable and we do no validation on them, so I believe making them settable is convenient and won't do harm. They're still required in the constructor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all because this is an In-Out object right?
If so it seems consistent with the behavior for MA. I still find it weird :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all because this is an In-Out object right?

Correct.

}
public partial class MetricAnomalyFeedback : Azure.AI.MetricsAdvisor.Models.MetricFeedback
{
Expand All @@ -974,7 +974,7 @@ public partial class MetricBoundaryCondition
{
public MetricBoundaryCondition(Azure.AI.MetricsAdvisor.Models.BoundaryDirection direction) { }
public string CompanionMetricId { get { throw null; } set { } }
public Azure.AI.MetricsAdvisor.Models.BoundaryDirection Direction { get { throw null; } }
public Azure.AI.MetricsAdvisor.Models.BoundaryDirection Direction { get { throw null; } set { } }
public double? LowerBound { get { throw null; } set { } }
public bool? TriggerForMissing { get { throw null; } set { } }
public double? UpperBound { get { throw null; } set { } }
Expand Down Expand Up @@ -1101,8 +1101,8 @@ public PostgreSqlDataFeedSource(string connectionString, string query) { }
public partial class SeverityCondition
{
public SeverityCondition(Azure.AI.MetricsAdvisor.Models.AnomalySeverity minimumAlertSeverity, Azure.AI.MetricsAdvisor.Models.AnomalySeverity maximumAlertSeverity) { }
public Azure.AI.MetricsAdvisor.Models.AnomalySeverity MaximumAlertSeverity { get { throw null; } }
public Azure.AI.MetricsAdvisor.Models.AnomalySeverity MinimumAlertSeverity { get { throw null; } }
public Azure.AI.MetricsAdvisor.Models.AnomalySeverity MaximumAlertSeverity { get { throw null; } set { } }
public Azure.AI.MetricsAdvisor.Models.AnomalySeverity MinimumAlertSeverity { get { throw null; } set { } }
}
public partial class SmartDetectionCondition
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1030,12 +1030,12 @@ private static void ValidateDetectionConfigurationToCreate(AnomalyDetectionConfi
/// A <see cref="Response{T}"/> containing the result of the operation. The result is a <c>string</c>
/// containing the ID of the newly created configuration.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="alertConfiguration"/> or <paramref name="alertConfiguration"/>.MetricAlertConfigurations is null.</exception>
/// <exception cref="ArgumentException"><paramref name="alertConfiguration"/>.MetricAlertConfigurations is empty.</exception>
/// <exception cref="ArgumentNullException"><paramref name="alertConfiguration"/> or <paramref name="alertConfiguration"/>.Name is null.</exception>
/// <exception cref="ArgumentException"><paramref name="alertConfiguration"/>.Name is empty.</exception>
public virtual async Task<Response<string>> CreateAlertConfigurationAsync(AnomalyAlertConfiguration alertConfiguration, CancellationToken cancellationToken = default)
{
Argument.AssertNotNull(alertConfiguration, nameof(alertConfiguration));
Argument.AssertNotNullOrEmpty(alertConfiguration.MetricAlertConfigurations, $"{nameof(alertConfiguration)}.{nameof(alertConfiguration.MetricAlertConfigurations)}");
Argument.AssertNotNullOrEmpty(alertConfiguration.Name, $"{nameof(alertConfiguration)}.{nameof(alertConfiguration.Name)}");

using DiagnosticScope scope = _clientDiagnostics.CreateScope($"{nameof(MetricsAdvisorAdministrationClient)}.{nameof(CreateAlertConfiguration)}");
scope.Start();
Expand Down Expand Up @@ -1063,12 +1063,12 @@ public virtual async Task<Response<string>> CreateAlertConfigurationAsync(Anomal
/// A <see cref="Response{T}"/> containing the result of the operation. The result is a <c>string</c>
/// containing the ID of the newly created configuration.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="alertConfiguration"/> or <paramref name="alertConfiguration"/>.MetricAlertConfigurations is null.</exception>
/// <exception cref="ArgumentException"><paramref name="alertConfiguration"/>.MetricAlertConfigurations is empty.</exception>
/// <exception cref="ArgumentNullException"><paramref name="alertConfiguration"/> or <paramref name="alertConfiguration"/>.Name is null.</exception>
/// <exception cref="ArgumentException"><paramref name="alertConfiguration"/>.Name is empty.</exception>
public virtual Response<string> CreateAlertConfiguration(AnomalyAlertConfiguration alertConfiguration, CancellationToken cancellationToken = default)
{
Argument.AssertNotNull(alertConfiguration, nameof(alertConfiguration));
Argument.AssertNotNullOrEmpty(alertConfiguration.MetricAlertConfigurations, $"{nameof(alertConfiguration)}.{nameof(alertConfiguration.MetricAlertConfigurations)}");
Argument.AssertNotNullOrEmpty(alertConfiguration.Name, $"{nameof(alertConfiguration)}.{nameof(alertConfiguration.Name)}");

using DiagnosticScope scope = _clientDiagnostics.CreateScope($"{nameof(MetricsAdvisorAdministrationClient)}.{nameof(CreateAlertConfiguration)}");
scope.Start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,10 @@ public partial class AnomalyAlertConfiguration
/// <summary>
/// Initializes a new instance of the <see cref="AnomalyAlertConfiguration"/> class.
/// </summary>
/// <param name="name">A custom name for this <see cref="AnomalyAlertConfiguration"/> to be displayed on fired alerts.</param>
/// <param name="idsOfHooksToAlert">The unique identifiers of the <see cref="NotificationHook"/>s that must be notified when an alert is detected by this configuration.</param>
/// <param name="metricAlertConfigurations">The configurations that define which anomalies are eligible for triggering an alert.</param>
/// <exception cref="ArgumentNullException"><paramref name="name"/>, <paramref name="idsOfHooksToAlert"/>, or <paramref name="metricAlertConfigurations"/> is null.</exception>
/// <exception cref="ArgumentException"><paramref name="name"/> is empty.</exception>
public AnomalyAlertConfiguration(string name, IList<string> idsOfHooksToAlert, IList<MetricAnomalyAlertConfiguration> metricAlertConfigurations)
public AnomalyAlertConfiguration()
{
Argument.AssertNotNullOrEmpty(name, nameof(name));
Argument.AssertNotNull(idsOfHooksToAlert, nameof(idsOfHooksToAlert));
Argument.AssertNotNull(metricAlertConfigurations, nameof(metricAlertConfigurations));

Name = name;
IdsOfHooksToAlert = idsOfHooksToAlert;
MetricAlertConfigurations = metricAlertConfigurations;
IdsOfHooksToAlert = new ChangeTrackingList<string>();
MetricAlertConfigurations = new ChangeTrackingList<MetricAnomalyAlertConfiguration>();
}

/// <summary>
Expand All @@ -43,7 +33,7 @@ public AnomalyAlertConfiguration(string name, IList<string> idsOfHooksToAlert, I
/// <summary>
/// A custom name for this <see cref="AnomalyAlertConfiguration"/> to be displayed on fired alerts.
/// </summary>
public string Name { get; }
public string Name { get; set; }

/// <summary>
/// The unique identifiers of the <see cref="NotificationHook"/>s that must be notified when an alert is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ public MetricAnomalyAlertSnoozeCondition(int autoSnooze, SnoozeScope snoozeScope
/// Defines the set of time series to which this <see cref="MetricAnomalyAlertSnoozeCondition"/>
/// applies.
/// </summary>
public SnoozeScope SnoozeScope { get; }
public SnoozeScope SnoozeScope { get; set; }

/// <summary>
/// The number of data points to be ingested before alerts are enabled again.
/// </summary>
public int AutoSnooze { get; }
public int AutoSnooze { get; set; }

/// <summary>
/// If <c>true</c>, snoozing will stop as soon as a data point that's not an anomaly is found.
/// If <c>false</c>, snoozing only stops when the amount of points specified by <see cref="AutoSnooze"/>
/// have been ingested.
/// </summary>
[CodeGenMember("OnlyForSuccessive")]
public bool IsOnlyForSuccessive { get; }
public bool IsOnlyForSuccessive { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public MetricBoundaryCondition(BoundaryDirection direction)
/// The direction of the specified boundaries. Depending on its value, <see cref="LowerBound"/>
/// and/or <see cref="UpperBound"/> may be required.
/// </summary>
public BoundaryDirection Direction { get; }
public BoundaryDirection Direction { get; set; }

/// <summary>
/// The minimum value a data point can assume to be able to trigger an alert. Must be set if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ public SeverityCondition(AnomalySeverity minimumAlertSeverity, AnomalySeverity m
/// The minimum severity an anomaly must have to be able to trigger alerts.
/// </summary>
[CodeGenMember("MinAlertSeverity")]
public AnomalySeverity MinimumAlertSeverity { get; }
public AnomalySeverity MinimumAlertSeverity { get; set; }

/// <summary>
/// The maximum severity an anomaly must have to be able to trigger alerts.
/// </summary>
[CodeGenMember("MaxAlertSeverity")]
public AnomalySeverity MaximumAlertSeverity { get; }
public AnomalySeverity MaximumAlertSeverity { get; set; }
}
}
Loading