Add statusCode and exceptionType Fields to Network Statsbeat #1007
Conversation
JacksonWeber
changed the title
JacksonWeber
changed the title
| this._statbeatMetrics.push({ | ||
| name: Constants.StatsbeatCounter.REQUEST_FAILURE, | ||
| value: currentCounter.count, | ||
| properties: properties |
There was a problem hiding this comment.
Where are you using currentCounter.statusCode?, I can see you have an accurate count but I'm not sure where are you actually adding it to the Statsbeat properties
There was a problem hiding this comment.
Should be fixed now. Added it to the properties in each counter where it's relevant per the spec.
| statsBeat.countRetry(0, "test", 206); | ||
| statsBeat.countRetry(0, "test", 206); | ||
| statsBeat.countThrottle(0, "test", 206); | ||
| statsBeat.countException(0, "test", "Statsbeat Exception"); |
There was a problem hiding this comment.
Add assert checking this exception type
There was a problem hiding this comment.
Added tests to determine the exceptionType field is populated correctly on the metric.
| statsBeat.countRequest(0, "test", 1, false, 200); | ||
| statsBeat.countRequest(0, "test", 1, false, 200); | ||
| statsBeat.countRetry(0, "test", 206); | ||
| statsBeat.countRetry(0, "test", 206); |
There was a problem hiding this comment.
Same here we want to validate you count multiple failed requests, retry and throttle and multiple Statsbeat are generated with the properties part of them
There was a problem hiding this comment.
Added some new tests that should ensure we're creating multiple metrics per statusCode now.
| statsBeat.countRetry(0, "test"); | ||
| statsBeat.countThrottle(0, "test"); | ||
| statsBeat.countException(0, "test"); | ||
| statsBeat.countRequest(0, "test", 1, true, 200); |
There was a problem hiding this comment.
StatusCode should be optional parameter if is not used all the time, like in this case
There was a problem hiding this comment.
If we make this parameter optional are we ok with creating entries in the request counter like { statusCode: undefined, count: <currentCount> }?
There was a problem hiding this comment.
no need to do that, successful count is only a number
| @@ -118,43 +118,63 @@ class Statsbeat { | |||
| this._instrumentation &= ~instrumentation; | |||
| } | |||
|
|
|||
| public countRequest(endpoint: number, host: string, duration: number, success: boolean) { | |||
| public countRequest(endpoint: number, host: string, duration: number, success: boolean, statusCode?: number) { | |||
| if (!this.isEnabled()) { | |||
| return; | |||
| } | |||
| let counter: Network.NetworkStatsbeat = this._getNetworkStatsbeatCounter(endpoint, host); | |||
| counter.totalRequestCount++; | |||
| counter.intervalRequestExecutionTime += duration; | |||
| if (success === false) { | |||
There was a problem hiding this comment.
Add check for statusCode here as well or you will create a null statusCode counter if this code path is ever triggered
…ub.com/microsoft/ApplicationInsights-node.js into jacksonweber/add-status-code-statsbeat
This adds the statusCode and exceptionType fields to the appropriate network StatsBeat metrics per the spec https://github.com/microsoft/Telemetry-Collection-Spec/blob/main/ApplicationInsights/statsbeat.md