Adding Instrumentation Statsbeat #802
Conversation
| @@ -153,6 +153,18 @@ class fakeHttpsServer extends events.EventEmitter { | |||
| } | |||
|
|
|||
| describe("EndToEnd", () => { | |||
| var originalEnv = {}; | |||
|
|
|||
| before(() => { | |||
There was a problem hiding this comment.
E2E tests here rely on some fakeHttpServer serving all requests, now that Azure VM request is in place is making these tests unreliable, we need to refactor this code and use nock to handle all SDK HTTP requests correctly, created a task to take care of that in different PR
AutoCollection/Statsbeat.ts
Outdated
| "rpid": this._resourceIdentifier, | ||
| }, commonProperties); | ||
| this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.ATTACH, value: 1, properties: attachProperties }); | ||
| this._statbeatMetrics.push({ name: Constants.StatsbeatCounter.INSTRUMENTATION, value: 1, properties: instrumentationProperties }); |
There was a problem hiding this comment.
I don't think it's confirmed yet to create a new instrumentation type.. we need to reconsider customDimentions for this. can you provide the payload for it? same as feature?
There was a problem hiding this comment.
yep same as feature but with instrumentation property instead, let me know if we need to update the properties here
There was a problem hiding this comment.
think about it more, we might not need to create a new metric for instrumentation.. we can reuse the feature metric.. they're identical. maybe just rename the "feature" to 'encodedLong' to support both feature and instrumentation and then add a flag to indicate type of encodedLong.
Declarations/Constants.ts
Outdated
|
|
||
| export enum StatsbeatFeatureType { | ||
| Features, |
| @@ -245,13 +246,14 @@ describe("AutoCollection/Statsbeat", () => { | |||
| setTimeout(() => { | |||
| let envelope = spy.args[0][0][1]; | |||
| let baseData: Contracts.MetricData = envelope.data.baseData; | |||
| assert.equal(baseData.metrics[0].name, "Instrumentation"); | |||
| assert.equal(baseData.metrics[0].name, "Feature"); | |||
There was a problem hiding this comment.
all the properties in the custom dimensions are using lower case. please be consistent.
There was a problem hiding this comment.
This is the metric name, must be fist letter upper case, please confirm
| } | ||
|
|
||
| export enum StatsbeatFeatureType { |
There was a problem hiding this comment.
All types and enums in this file start with Statsbeat word to quickly find them
No description provided.