[MetricsAdvisor] Fix Update methods issues #16696
Conversation
kinelski
added
Client
| @@ -78,7 +78,7 @@ internal AnomalyAlertingConfigurationPatch GetPatchModel() | |||
| { | |||
| return new AnomalyAlertingConfigurationPatch() | |||
| { | |||
| CrossMetricsOperator = CrossMetricsOperator.HasValue ? new AnomalyAlertingConfigurationPatchCrossMetricsOperator(CrossMetricsOperator.Value.ToString()) : default, | |||
| CrossMetricsOperator = CrossMetricsOperator.HasValue ? new AnomalyAlertingConfigurationPatchCrossMetricsOperator(CrossMetricsOperator.Value.ToString()) : default(AnomalyAlertingConfigurationPatchCrossMetricsOperator?), | |||
There was a problem hiding this comment.
This was the source of the issue in UpdateAlertConfiguration. CrossMetricsOperator is a nullable struct, and we were mistakenly expecting default to be null.
To make it easier to illustrate, let's use a clearer example:
bool condition = ...;
int? a = condition ? 10 : default; If condition == true, we'll have a = 10. If condition == false, we'll have a = 0 (instead of null).
The ternary operator is evaluated before the attribution, and the compiler guesses the result must be an int, so it makes default(int) instead of default(int?). For this reason, we need to make it clear:
int? a = condition ? 10 : default(int?);I have updated other parts of the code in which we had the same issue.
There was a problem hiding this comment.
Interesting bug - Seems like it is defined in this issue dotnet/csharplang#33
| CustomFillValue = dataFeedDetail.FillMissingPointValue; | ||
| CustomFillValue = dataFeedDetail.FillMissingPointType == DataFeedMissingDataPointFillType.CustomValue | ||
| ? dataFeedDetail.FillMissingPointValue | ||
| : null; |
There was a problem hiding this comment.
This was the source of the issue in UpdateDataFeed. DataFeedMissingDataPointFillSettings has two properties.
One of them is FillMissingPointType (the strategy used to fill missing points in time series: NoFilling, PreviousValue, CustomValue, ...).
There's another property that must be specified when type is CustomValue: CustomFillValue, the const value used to fill the missing point.
When the fill type is NOT CustomValue, I'd expect the service to not return the CustomFillValue (or return null). However, it returns 0.0. When we call Get followed by Update, we store the 0 returned by the service and try to send it back (which is not allowed, because type is not CustomValue).
We're doing a workaround, but it may be something to fix on the service side.
Fixes #16168.