Add Application Insights Processor for Response Codes #43
Conversation
loic-sharma
requested review from
joelverhagen,
agr,
chenriksson,
skofman1 and
scottbommarito
|
@loic-sharma, |
|
|
||
| if (request != null && int.TryParse(request.ResponseCode, out responseCode)) | ||
| { | ||
| if (responseCode == 400 || responseCode == 404) |
There was a problem hiding this comment.
The list of allowed error codes should be configurable. The reason is that for every service (search/gallery) we want to be aware of a different set of issues. For example, for the gallery, we should consider adding 409, but not for search. 400 and 404 can be the default config.
There was a problem hiding this comment.
Sure, sounds good to me.
There was a problem hiding this comment.
This work is already done in status page.
|
|
||
| namespace NuGet.Services.Logging.Tests | ||
| { | ||
| class TelemetryCallbackProcessor : ITelemetryProcessor |
There was a problem hiding this comment.
this can be a private class inside TelemetryResponseCodeProcessorFacts, since it's not going to be used anywhere else.
| using Microsoft.ApplicationInsights.Extensibility; | ||
| using Microsoft.ApplicationInsights.Channel; | ||
| using Microsoft.ApplicationInsights.DataContracts; | ||
| using System; |
There was a problem hiding this comment.
nit: sort System namespaces first
There was a problem hiding this comment.
Good catch, thanks. My Visual Studio settings got messed up, should be fixed now :)
| @@ -0,0 +1,21 @@ | |||
| using Microsoft.ApplicationInsights.Channel; | |||
There was a problem hiding this comment.
I merged this file into TelemetryResposneCodeProcessorFacts.cs, so this is no longer needed.
| } |
There was a problem hiding this comment.
probably undo it, helps in keeping the history of changes useful on a file.
loic-sharma
force-pushed
the
loshar-404fix
branch
from
ef1aacf to
2c3de26
Compare
This adds the Gallery's
TelemetryResponseCodeFilterto this common repository so that it can be used by both the Gallery and the Search Service. I tweaked the name and added some basic tests.Related to Gallery #3793