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

dotnet-monitor Bad Prometheus Metric Names for S.D.Metrics #7647

Open
pinkfloydx33 opened this issue Nov 10, 2024 · 1 comment
Open

dotnet-monitor Bad Prometheus Metric Names for S.D.Metrics #7647

pinkfloydx33 opened this issue Nov 10, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@pinkfloydx33
Copy link

pinkfloydx33 commented Nov 10, 2024

Description

When configured to export a Meter from System.Diagnostics.Metics , the Prometheus metrics names reported via /metrics are a bit off. The metrics end up with trailing underscores and also groups of double underscores. This does not match the behavior of exporting Event Counters, nor does it match most metrics I've seen exported in other Prometheus-based systems. Also, dotnet-monitor is appending the units of the Instrument which I believe is incorrect as metrics following Otel/Prometheus conventions should already include that portion (in non-abbreviated form) in their names.

Here are some of the metric names being reported:

  • systemnethttp_http_client_active_requests__request_
    • trailing and double underscores
  • systemnethttp_http_client_open_connections__connection_
    • trailing and double
  • npgsql_db_client_commands_executing__command_
    • trailing and double
  • microsoftextensionsdiagnosticsresourcemonitoring_container_memory_limit_utilization__
    • double trailing

For what it's worth, here's how Prometheus.NET reports these same metrics:

  • system_net_http_http_client_active_requests
  • system_net_http_http_client_open_connections
  • npgsql_db_client_commands_executing
  • microsoft_extensions_diagnostics_resourcemonitoring_container_memory_limit_utilization

As you can see, the units field isn't included in the name at all. They just concatenate the instrument and meter name before normalizing the remaining characters.

This isn't necessarily a bug in dot-monitor; it's creating metric names based on a specific algorithm and the offending source metrics seem to be following a pattern for the unit field (the UCUM standard) that dotnet-monitor does not recognize (see below). That said I think there's room for improvement here as these names don't conform to most you'll see (or that I've seen) when working with Prometheus. To boot, the metric names in my examples already include the units in appropriate pluralized form, so you end up with double units in the name where the ones appended here are singular (and may be abbreviated, ex. s vs seconds which would be in the name already)

I think that dotnet-monitor should only use the units field of an Instrument in the HELP section of the export, and not when building the metric names. Anyone that has been following open-telemetry and Prometheus naming conventions is already naming their Meter/Instruments correctly. At the very least the runtime is which should be a big flag here.

I realize that changing metric names can break anyone already relying them. Perhaps an option could be added to indicate whether or not the units should be appended to the name.

Configuration

  • dotnet-monitor 8.0
  • container: mcr.microsoft.com/dotnet/monitor:latest
  • Azure Kubernetes
  • The application being monitored is .NET 8, single-file, self-contained ASP.NET and Worker applications

Other information

In the case of the first three items above, the units are being reported as {request}, {connection} and {command} (respectively) and the non-alphanumeric characters are being converted to _. Apparently this is how npgsql and even the runtime reports some of it's units, which tells me that this is some sort of convention that should probably be recognized by the Prometheus exporter (assuming it keeps appending the units). EDIT: this is the "UCUM" standard and is reflected in dotnet's "best practices" for Metrics see here

In the case of the fourth item, the unit of measure is 1 which I think makes sense in this case, yet the exporter drops it

@pinkfloydx33 pinkfloydx33 added the bug Something isn't working label Nov 10, 2024
Copy link
Contributor

Welcome to dotnet-monitor!

Thanks for creating your first issue; let us know what you think of dotnet-monitor by filling out our survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant