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.

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

Feedback request - Metrics API in .NET Runtime #2221

Closed
cijothomas opened this issue Aug 3, 2021 · 11 comments
Closed

Feedback request - Metrics API in .NET Runtime #2221

cijothomas opened this issue Aug 3, 2021 · 11 comments
Labels
enhancement New feature or request metrics need-runtime-change Issues which likely require changes from dotnet runtime - typically DiagnosticSource changes

Comments

@cijothomas
Copy link
Member

As previously announced, the OpenTelemetry compatible Metrics API is made available as part of .NET runtime via the DiagnosticSource package version 6.0.0. The package is already available as preview today, and is planned to be released as stable, along with the broader .NET 6 release in Nov 2021.

We are trying to gather feedback about a particular API, to consider making changes in .NET 6 timeframe itself.

The Counter API currently has multiple Add() overloads.
The overloads taking 1 or 2 or 3 tags can be called without a heap allocation.
For example, the following calls does not cause heap allocation.
counter.Add(10, new KeyValuePair("key1", "value1"));
counter.Add(10, new KeyValuePair("key1", "value1"),new KeyValuePair("key2", "value2"));
counter.Add(10, new KeyValuePair("key1", "value1",new KeyValuePair("key2", "value2"),new KeyValuePair("key3", "value3"));

For passing more than 3 tags, there are 2 choices:

  1. public void Add(T delta, params KeyValuePair<string, object?>[] tags) -- This allocates []
  2. public void Add(T delta, ReadOnlySpan<KeyValuePair<string, object?>> tags) -- This overload, along with some additional work (leveraging ThreadLocal) can be done by users to avoid the allocations, but it does require user to do additional work.

We are looking to gather feedback, to see if it makes sense to add more overloads to support upto "N" (tag1,tag2,..tagN) tags without allocation, and without user doing any additional work.

While the change is completely additive, and can be added in any future versions. (i.e .NET 7, 8 ...), we are trying to see if this can be part of .NET 6 itself, so users can benefit right away.

Ask from Otel community, APM vendors
Please comment on this issue here, if your typical use case has more than 3 dimensions, and if yes, how many dimensions. (Please note that, we are not checking how many maximum dimensions are being used, instead checking what is the number of dimensions used by more than 90% users/scenarios).

  1. If there is sufficient ask from OpenTelemetry community that the typical Metric usage involves more than N dimensions (where N > 3), we'll let .NET team know about this.
  2. If there is not enough demand, this topic will be revisited in the future, and the additional APIs may be added the future .NET versions (.NET 7 , 8....)

Important to note:

  1. The APIs do not have any restriction on the number of dimensions. But the API is optimized for upto 3 dimension scenarios, and the ask is, if this optimization should be extended for up to N dimensions.
  2. I used Counter as an example - the same optimization can be done for other instruments (Gauge, Histogram)
  3. .NET 6 is nearing code-freeze, so we will collect feedback upto Aug 6th 2021.
@cijothomas cijothomas added the enhancement New feature or request label Aug 3, 2021
@cijothomas
Copy link
Member Author

Feedback based on Otel semantic conventions (its experimental at this stage, but still sharing)
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#attributes
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md

It looks like conventions mark most attributes(tags) as optional; the number is between 5-8.

@cijothomas cijothomas added metrics need-runtime-change Issues which likely require changes from dotnet runtime - typically DiagnosticSource changes labels Aug 3, 2021
@vgorbenko
Copy link

When users adopt dimensional metrics, it is common to see 6-8 dimensions (tags) per metric, and occasionally more. An indication to how many dimensions to curate the API for is in platform metrics as they were defined by different teams at different times, for different scenarios. Seeing a similar volume of dimensions for APM domain with Application Insights: standard metrics. From this, I suggest picking a number between 8 and 10. Going over 10 would likely be an overkill.

@pjanotti
Copy link
Contributor

pjanotti commented Aug 4, 2021

@cijothomas do the N dimensions mentioned above include things that typically come via OTel Resources (that will take care of dimensions like process, host, service name, etc)?

@cijothomas
Copy link
Member Author

@cijothomas do the N dimensions mentioned above include things that typically come via OTel Resources (that will take care of dimensions like process, host, service name, etc)?

No. Similar to tracing, the Resource would come separately.

@pjanotti
Copy link
Contributor

pjanotti commented Aug 4, 2021

@cijothomas from our side (Splunk) having up to 6 dimensions + Resources will cover the typical case. There are some with many more dimensions than that but they are not typical.

@cijothomas
Copy link
Member Author

When users adopt dimensional metrics, it is common to see 6-8 dimensions (tags) per metric, and occasionally more. An indication to how many dimensions to curate the API for is in platform metrics as they were defined by different teams at different times, for different scenarios. Seeing a similar volume of dimensions for APM domain with Application Insights: standard metrics. From this, I suggest picking a number between 8 and 10. Going over 10 would likely be an overkill.

Looking at the list shared, 2 of the dimensions in the ones which has the highest number of dimensions are things like RoleName, RoleInstance, which are treated as Resources in Otel SDKs, and won't be considered dimension for in-memory aggregation. So I can subtract 2 from your ask to say "suggest picking a number between 6 and 8".

@alanwest
Copy link
Member

alanwest commented Aug 5, 2021

Somewhere between 6 and 8 is reasonable. I've investigated this a bit at New Relic and ~6 is definitely common.

Feedback based on Otel semantic conventions (its experimental at this stage, but still sharing)
It looks like conventions mark most attributes(tags) as optional; the number is between 5-8.

Given this, I think it makes sense to lean towards 8. In my initial stab at the ASP.NET Core instrumentation, I was adding 7 attributes

@cijothomas
Copy link
Member Author

Thanks all for sharing the feedback. .NET is considering adding upto 8, in .NET 6 timeframe (subject to approvals)
Tracking issue in .NET runtime repo: dotnet/runtime#56936

@shaynevanasperen
Copy link

There is no support for the UpDownCounter in the OpenTelemetry specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#updowncounter

Is this an oversight, or was it decided that this would not be added?

@cijothomas
Copy link
Member Author

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#updowncounter

Its not an oversight. It was discussed a lot in Metric SIG meetings and finally .NET decided to skip it for .NET 6 and come back for next version, based on feedbacks. See #2362

@cijothomas
Copy link
Member Author

Closing this as the proposals and feedback from all were incorporated in RC1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request metrics need-runtime-change Issues which likely require changes from dotnet runtime - typically DiagnosticSource changes
Projects
None yet
Development

No branches or pull requests

5 participants