[WIP - GH-42] Add Service Bus Subscription Count Client #49
Conversation
| import ( | ||
| "context" | ||
|
|
||
| "github.com/Azure/azure-sdk-for-go/services/servicebus/mgmt/2017-04-01/servicebus" |
There was a problem hiding this comment.
I currently use the Azure SDK for Go as it allowed for the retrieval of each type of count within CountDetails. In the current case of azure-service-bus-go this number can be skewed if there are letters in the DLQ not being pulled
There was a problem hiding this comment.
|
Based on #39, this is the structure I'd see for configuring the CRD apiVersion: azure.com/v1alpha2
kind: ExternalMetric
metadata:
name: example-external-metric-service-bus-subscription
spec:
type: servicebussubscription
azure:
resourceGroup: storage-rg
namespace: sample-ns
topic: sample-topic
subscription: sample-subscription
metric:
# This would default to activeMessageCount, but could be updated to one of the counts from https://github.com/Azure/azure-sdk-for-go/blob/master/services/servicebus/mgmt/2017-04-01/servicebus/models.go#L1116
field: activeMessageCount |
main.go
Outdated
| monitorClient := monitor.NewClient(defaultSubscriptionID) | ||
| appinsightsClient := appinsights.NewClient() | ||
|
|
||
| azureProvider := azureprovider.NewAzureProvider(defaultSubscriptionID, mapper, dynamicClient, appinsightsClient, monitorClient, metricsCache) | ||
| azureProvider := azureprovider.NewAzureProvider(defaultSubscriptionID, mapper, dynamicClient, appinsightsClient, monitorClient, serviceBusSubscriptionClient, metricsCache) |
There was a problem hiding this comment.
I think this will get unwieldy as we add more clients such as the Storage bus client. I suggest we create a factory that creates the correct client based on the type specified in the new spec.
There was a problem hiding this comment.
Agreed, I'll move it towards the factory pattern
|
|
||
| type AzureExternalMetricClient interface { | ||
| GetAzureMetric(azMetricRequest AzureExternalMetricRequest) (AzureExternalMetricResponse, error) | ||
| } |
There was a problem hiding this comment.
This looks good and right along the lines of what I was thinking.
| merticReq.Filter = fmt.Sprintf("%s %s '%s'", filterStrings[0], filterStrings[1], filterStrings[2]) | ||
| glog.V(2).Infof("filter formatted: %s", merticReq.Filter) | ||
| // Service Bus | ||
| case "namespace": |
There was a problem hiding this comment.
I don't think we need to continue to update this function. It was a left over the first attempt that proved not to work very well. My plan is to depreciate the support of label selectors as we approach 1.0 and only support CRD's. I left this in for now for anyone that might have had something running so as to not break existing solutions but don't want to encourage using it any further.
| import ( | ||
| "context" | ||
|
|
||
| "github.com/Azure/azure-sdk-for-go/services/servicebus/mgmt/2017-04-01/servicebus" |
|
@jsturtevant currently didn't link up the providers just yet as I didn't want to get too far into the weeds without agreement from your end. I'll keep it moving forward based on the initial feedback |
|
Looks good to move forward on. Let me know if there are any challenges or blockers. Thanks again! |
|
Just checking in to see of there are any blockers I can help with. |
|
No blockers currently. I got backlogged on professional and personal commitments and will be getting back to this PR. |
Adds the client and metric request structure for gathering message counts from Azure Service Bus Topic Subscriptions
Allows for providers of External Metrics to implement a shared interface
|
@marc-sensenich Thanks so much for this PR. On Friday afternoon I was going to take the great start you have here and start to finish it up for the next release. Let me know if you have any thoughts or objections. |
|
@jsturtevant my apologies for leaving this one hanging. I have no objections from you getting this put together for the next release. I have some lingering changes I'll push up here and then you can keep them or drop them |
|
Thanks and no worries! I appreciate the initial start to this. I have a few spare cycles coming up and this is a frequently asked for feature. So thank you for getting it started! |
|
I am closing this as I did not have permissions to push changes to @marc-sensenich branch. Thanks for your initial implementation! |
|
Thanks for taking this over @jsturtevant! |
Allow for users of the metrics adapter to gather message counts from Azure Service Bus Topic Subscriptions to decide their scaling targets
Closes #42