Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] trackMetric does not track stdDev nor sum #1680

Open
stefanedwards opened this issue Oct 1, 2021 · 11 comments
Open

[BUG] trackMetric does not track stdDev nor sum #1680

stefanedwards opened this issue Oct 1, 2021 · 11 comments
Assignees
@MSNev
Labels
investigation required Further investigation or discussions required

Comments

@stefanedwards
Copy link

When tracking a (custom) metric, the standard deviation is not tracked (as valueStdDev) and valueSum is identical to value.
Additionally, when min-value equals 1,

Steps to Reproduce

  • OS/Browser: Google Chrome 93.0.4577.82 (64-bit), Windows 10
  • SDK Version [e.g. 22]: javascript:2.7.0
  • How you initialized the SDK:
<!DOCTYPE html>
<html>
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
  <script src="https://js.monitor.azure.com/scripts/b/ai.2.7.0.min.js"></script>
  <script>
  
  let msg = { config: {
    "instrumentationKey": "<<>>",
    "appId": "Debug Metrics"
}}
  
let init = new Microsoft.ApplicationInsights.ApplicationInsights(msg);
let appInsights = init.loadAppInsights();  
  </script>
</head>
<body>
	<script>
	appInsights.trackMetric({name: 'my_metric', average: 1, sampleCount: 2, min: 1, max: 2, stdDev: 1.0});
	appInsights.flush();
	</script>
</body>
</html>

When viewing the tracked metric in Application Insights on portal.azure.com, the values do not match what was provided.

Metric in Application Insights log on portal.azure.com

Expected behavior

  1. It should be possible to track the sum.
  2. When retrieving the metric in the Application Insights logs on portal.azure.com, the value, valueCount, valueSum, valueMin, valueMax, and valueStdDev should match (within some reasonable precision) to the values submitted from the JS script.

Additional context
The documentation on appInsights.trackMetric is entirely missing (for the JS-part), and this is my best guess for actually getting something through. A lot of other attempts (e.g. trackMetric('name',value) didn't even raise an error -- but nothing appeared to be sent).

I have further found, that the second argument to trackMetric can be used to send additional properties (i.e. customDimensions), such as appInsights.trackMetric({name: 'bacon', average: 5, sampleCount: 6, min: 1, max: 15}, {PageName: "foo", "PageUrl": "/", "questionable_field": "zap" });
@Karlie-777
Copy link
Contributor

Hi @stefanedwards
for custom properties, they should be passed as an object
trackMetric({name:"name", average: 1, sampleCount?:1, min?:1, max?:1,properties?: { "customProp1": "value1" }});

and for more trackMetrics details, please refer https://github.com/microsoft/ApplicationInsights-JS/blob/master/AISKU/API.md#trackmetric

please let us know if it works

@stefanedwards
Copy link
Author

No, that does change the issue mentioned. I am reporting that when reporting aggregates of multiple measurements with trackMetric (cf. the documentation), that the reported sum is misleading (identical to the average) and there is no way of reporting the standard deviation.

Furthermore, thank you for the link to the documentation. But the code line in

trackMetric({name:"name", average: 1, sampleCount?:1, min?:1, max?:1,properties?: { "customProp1": "value1" }});

contradicts the documentation (properties should be a second argument), but it still works. In fact, both approaches works.

@MSNev MSNev added the investigation required Further investigation or discussions required label Oct 26, 2021
@MSNev
Copy link
Collaborator

MSNev commented Oct 26, 2021

Investigations required

  • Looking at the current API, it doesn't support passing the stddev, need to check whether the server will accept the value if we pass it.
  • Validate the values passed and how they are processed, I suspect that passing the sampleCount is causing some recalculations on the server.

@MSNev MSNev self-assigned this Oct 28, 2021
@MSNev
Copy link
Collaborator

MSNev commented Oct 28, 2021

When the SDK is updated to send the stdDev it is successfully included and displayed in the portal -- will have a PR shortly to update the trackMetric() to support the stdDev value and pass it to the backend.

Still investigating the passed values not matching issue

@MSNev
Copy link
Collaborator

MSNev commented Oct 28, 2021

Ok, the issue with the average / sampleCount is because there is a different calculation between what the SDK takes and how the backend "processes" the values.

Simplistically, the backend expects the total "value" of the metric and the number of events "count" and from these it calculates the "average" value / count and if the min is smaller or the max is larger then the passed values are replaced with this calculation.

So, the fix is problematic... As technically the SDK should pass the value as the (average * (sampleCount || 1)) instead of just the average. However, doing this will probably break someone as now the reported value won't be the value of the average passed (if the sampleCount is > 1)... Will think about this overnight.

@MSNev
Copy link
Collaborator

MSNev commented Oct 28, 2021

And the backend doesn't have the ability to be passed "sum", I'm checking with the portal team on how the valueSum is calculated because of this

MSNev added a commit that referenced this issue Oct 28, 2021
@MSNev
[BUG] trackMetric does not track stdDev nor sum #1680
883f30b 
- add (missing) stdDev sending for metrics
@MSNev
Copy link
Collaborator

MSNev commented Oct 28, 2021

Pushed out the PR to just add stdDev as an option that is sent -- need to wait for the valueSum and above details for another PR

@MSNev
Copy link
Collaborator

MSNev commented Oct 28, 2021 •
edited
Loading

Ok, the valueSum value column in the portal is actually a deprecated and cannot be directly populated (it will always be the same as the value valueSum moving forward), so there is no way to directly populate it.

Still thinking about the "average" -> "value and "sampleCount" -> "valueCount" and the issue with the "valueMin" (and valueMax) when "sampleCount" > 1...

MSNev added a commit that referenced this issue Oct 29, 2021
@MSNev
[BUG] trackMetric does not track stdDev nor sum #1680 (#1701)
5101bdf 
- add (missing) stdDev sending for metrics
@MSNev
Copy link
Collaborator

MSNev commented Nov 4, 2021

The stdDev change is now published to NPM in v2.7.1, still determining the best approach for the sum / sample -- most likely will add some additional override properties to maintain backward compatibility as the "average" value is misleading but would be a breaking change

@dballenger85
Copy link

This change is broken for the JS api. This change missed updating the constructor invocation in MetricEnvelopeCreator under EnvelopeCreator.ts and so is now passing props as the stdDev argument. All customDimensions are now lost when tracking any kind of metric.

@MSNev
Copy link
Collaborator

MSNev commented Dec 6, 2021

Well crap!

@MSNev MSNev mentioned this issue Dec 6, 2021
Closed
MSNev added a commit to MSNev/ApplicationInsights-JS that referenced this issue Dec 6, 2021
@MSNev
[BUG] Addition of stdDev metric support has broken custom metric repo…
8941045 
…rting from microsoft#1680 microsoft#1729
MSNev added a commit that referenced this issue Dec 7, 2021
@MSNev
[BUG] Addition of stdDev metric support has broken custom metric repo…
1789620 
…rting from #1680 #1729 (#1730)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MSNev MSNev

Labels
investigation required Further investigation or discussions required
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stefanedwards @dballenger85 @MSNev @Karlie-777