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/extensions metric names to follow some convention #4432

Closed
klauco opened this issue Sep 18, 2023 · 7 comments · Fixed by #4482
Closed

dotnet/extensions metric names to follow some convention #4432

klauco opened this issue Sep 18, 2023 · 7 comments · Fixed by #4482
Assignees

Comments

@klauco
Copy link

klauco commented Sep 18, 2023

Currently, the metric names are using backslashes. OTel uses dots. We need to go through the metrics and ensure we stick to the same naming convention (and semantic grouping as part of the metric name).

Proposal:

  1. use dots instead of backslashes
  2. follow pluralization rules as described in https://opentelemetry.io/docs/specs/otel/metrics/semantic_conventions/
@klauco
Copy link
Author

klauco commented Sep 18, 2023

cc @geeknoid @xakep139 @noahfalk

@xakep139
Copy link
Contributor

xakep139 commented Sep 20, 2023

As @noahfalk suggested, we should consider using instrument names that follow pattern like: aspnetcore.header_parsing.parse_errors instead of HeaderParsing.ParsingErrors that we have currently.

@xakep139
Copy link
Contributor

I can apply this pattern within #4342, but we need to reach an agreement first

@martintmk
Copy link
Contributor

martintmk commented Sep 20, 2023

As @noahfalk suggested, we should consider using instrument names that follow pattern like: aspnetcore.header_parsing.parse_errors instead of HeaderParsing.ParsingErrors that we have currently.

We have also aligned the naming in Polly with OpenTelemetry by using the convention as suggested above. See App-vNext/Polly#1583 for reference.

Also attributes names were changed:

  • event_name -> event.name
  • exception_name -> exception.type (this one is standard in OT)
  • etc

@xakep139
Copy link
Contributor

@joperezr
Copy link
Member

joperezr commented Oct 5, 2023

@xakep139 is working on this this week, and the target is to merge by next week.

@joperezr
Copy link
Member

Update:

  • PR seems in a good state, we are just waiting on a signoff from Ludmila.

@ghost ghost removed the work in progress 🚧 label Oct 23, 2023
@ghost ghost removed the untriaged label Oct 23, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants