Do not send metric NaN values #2119
Conversation
|
@trask can we please add a unit test for this scenario? |
| if (filteredPoints.isEmpty()) { | ||
| if (metricsData.getMetrics().isEmpty()) { | ||
| // this is unexpected | ||
| logger.debug("MetricsData has no metric point"); |
There was a problem hiding this comment.
should it throw an exception if it's unexpected?
There was a problem hiding this comment.
I was being conservative, but feel pretty confident, so bumped it up to an exception
| } | ||
|
|
||
| if (!Double.isFinite(point.getValue())) { | ||
| // breeze doesn't like these values |
There was a problem hiding this comment.
do we drop that data point and then send the rest to breeze? here because of bad NaN we drop the whole metric. i wonder how other sdks handle it.
There was a problem hiding this comment.
unless that data point is the key dimension for that metric. it makes sense to drop it. without it metric doesn't make sense.
There was a problem hiding this comment.
ya, metric value is required
There was a problem hiding this comment.
I wonder how other sdks handle it
https://github.com/microsoft/Telemetry-Collection-Spec/issues/29
|
can we add a test for NaN MetricDataPoint? |
it's tricky currently b/c |
No description provided.