Closed csperbeck closed 5 years ago
@csperbeck Nice!
Initial feedback... the APIM ingredient should support wiring up App Insights and the diagnostics Event Hub at the same time. There is a lot of overlap between the monitoring data available between the two but I have found that there are enough differences that we should allow both to be configured. The App Insights ingredient currently has a function that can look up the iKey in a recipe. The diagnostics event hub can be wired up through a diagnostic settings added to APIM. There are examples of other ingredients wiring up their diagnostic settings. Take a look at Service Bus Namespace in the plugin.ts Execute() method for one example.
@bdschaap True story, I wanted to get the PR out there without delaying based on the EvenHub logger. You can include multiple loggers, mix and match, with ApplicationInsights
and EventHub
in a yaml object array.
logger:
testai:
resourceGroup: test
type: applicationInsights
clean: true
testeh:
resourceGroup: test
type: eventHub
Good that it will support both at the same time. There is an example of a recipe using the iKey lookup function here. Basically pass a resource short name in and it sets the token. Here is an example of the event hub diagnostics setting. It uses a "diagnostics" short name to send it to a diagnostics event hub namespace. The diagnostics ehn is precreated by a different Bake recipe.
@bdschaap I have it looking up the key by the AppInsights/EventHub resource and resource group itself. Is that not an amenable approach? I can add the input for instrumentationKey
and if it sees the key in the recipe then to not resolve the key itself.
Looks like it does the job and it could be a better way than the other implementations. I saw there was another approach using only the ARM template in the Azure Functions ingredient. I wasn't aware that approach existed but like that as well. If needed we can always circle back to refactor the implementations later when we learn more about each.
@bdschaap the primary reason I didn't use the template, is the ingredients don't support output from the deployments. Also, the secondary reason I didn't pull in other ingredients, is the references for the loggers, keys, ids, and all are already on the API Management instance. Agreed on the continual improvement and future refactor. Definitely could make it a bit prettier. @whilke @JasonPagel @jasonshamilton would you mind reviewing?
This is the ingredient for creating a base API Management instance as well as managing it, including named property key/value pairs. The ingredient includes the base template, deployment functions for properties and ApplicationInsights loggers, as well as the functions to retrieve these pieces of information for usage in other ingredients/recipes.