-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use Singleton Metrics Provider for Listener #285
Changes from 8 commits
b7e7995
62fca50
309526e
83a9554
47cdece
c0b2720
16e4bf5
349156c
fbb899a
89de7d9
5c161b8
d95d8ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -23,14 +23,14 @@ class SqlScaleMonitor : IScaleMonitor<SqlScaleMetric> | |||||
|
||||||
int? previousWorkerCount = -1; | ||||||
|
||||||
public SqlScaleMonitor(string taskHubName, SqlMetricsProvider sqlMetricsProvider) | ||||||
public SqlScaleMonitor(string functionId, string taskHubName, SqlMetricsProvider sqlMetricsProvider) | ||||||
{ | ||||||
// Scalers in Durable Functions are shared for all functions in the same task hub. | ||||||
// So instead of using a function ID, we use the task hub name as the basis for the descriptor ID. | ||||||
string id = $"DurableTask-SqlServer:{taskHubName ?? "default"}"; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! does this string id needs to add ToLower(CultureInfo.InvariantCulture) to be lower case? |
||||||
|
||||||
#if FUNCTIONS_V4 | ||||||
this.Descriptor = new ScaleMonitorDescriptor(id: id, functionId: id); | ||||||
this.Descriptor = new ScaleMonitorDescriptor(id: id, functionId: functionId); | ||||||
#else | ||||||
this.Descriptor = new ScaleMonitorDescriptor(id); | ||||||
#endif | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,14 +12,13 @@ public class SqlTargetScaler : ITargetScaler | |
{ | ||
readonly SqlMetricsProvider sqlMetricsProvider; | ||
|
||
public SqlTargetScaler(string taskHubName, SqlMetricsProvider sqlMetricsProvider) | ||
public SqlTargetScaler(string functionId, SqlMetricsProvider sqlMetricsProvider) | ||
{ | ||
this.sqlMetricsProvider = sqlMetricsProvider; | ||
|
||
// Scalers in Durable Functions are shared for all functions in the same task hub. | ||
// So instead of using a function ID, we use the task hub name as the basis for the descriptor ID. | ||
string id = $"DurableTask-SqlServer:{taskHubName ?? "default"}"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
this.TargetScalerDescriptor = new TargetScalerDescriptor(id); | ||
this.TargetScalerDescriptor = new TargetScalerDescriptor(functionId); | ||
} | ||
|
||
public TargetScalerDescriptor TargetScalerDescriptor { get; } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing. I recall the final fix that Alexey made to the Azure Storage metrics provider was to throttle it so that we only fetch metrics once every few seconds or so. This is how we ensured that we don't have too many storage transactions, even if the app has a lot of triggers. I think it will be important to do that for the
SqlMetricsProvider
too so that we don't overwhelm the customer's database with expensive SQL queries.