[Preview] Adding support for Standard Metrics #1002
Conversation
hectorhdzg
requested review from
JacksonWeber,
MSNev,
lzchen,
ramthi and
trask
| private _httpRequestDone(metric: IHttpStandardMetric) { | ||
| // Done could be called multiple times, only process metric once | ||
| if (!metric.isProcessed) { | ||
| metric.isProcessed = true; |
There was a problem hiding this comment.
Curious as to why and how _httpRequestDone is called multiple times for the same request?
There was a problem hiding this comment.
It could be called multiple times for specific cases, like request timeout, abort, etc.
| if (this._metricHandler?.isStandardMetricsEnabled) { | ||
| let exceptionDimensions: IMetricExceptionDimensions = { | ||
| cloudRoleInstance: envelope.tags[KnownContextTagKeys.AiCloudRoleInstance], | ||
| cloudRoleName: envelope.tags[KnownContextTagKeys.AiCloudRole] |
There was a problem hiding this comment.
Another field client/type might be needed. Curious did you look into what this dimension represents?
There was a problem hiding this comment.
I was not aware of the existence of that dimension, currently only enabling what we had before
|
|
||
| private static _instance: HttpMetricsInstrumentation; | ||
| private _nodeVersion: string; | ||
| public totalRequestCount: number = 0; |
There was a problem hiding this comment.
Why do we need to keep track of these totals? Are you planning to replace performance counters with this instrumentation?
There was a problem hiding this comment.
Perfomance counters are currently calculated using these, is still not very clear how perf counters would be handled in OpenTelemetry so having this functionality in the meantime
|
Exporter related changes: Azure/azure-sdk-for-js#22886 |
Added duration metrics for server/client in internal MetricsInstrumentation
Exceptions and traces standard metrics continue to work as before
Added SpanProcessor to add extra properties in spans