-
Notifications
You must be signed in to change notification settings - Fork 897
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
Adding the Metrics Counter API #1578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I were you I would split this into 2 PRs:
- One that clarifies instrument properties
- One that adds the Counter details
|
||
var counterPowerUsed = meter.CreateCounter<double, PowerConsumption>("power_consumption", unit="kWh"); | ||
counterPowerUsed.Add(13.5, new PowerConsumption { customer = "Tom" }); | ||
counterPowerUsed.Add(200, new PowerConsumption { customer = "Jerry" }, ("is_green_energy", true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I don't anticipate our .NET 6 implementation to include this strongly typed struct capability it does do a good job indicating the kind of flexibility we'd like language implementors to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meta Comment:
Strings in OTel Specification SHOULD be specified somewhere globally and you should just denote that things are strings, referencing that specification. We have a lot off "string" references in the Trace Spec where this spec is far more explicit.
I think it's worth opening a bug to address that and not blocking this PR. That said, not 100% happy with ASCII vs BMP and wanted to understand why.
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
* counter API * fix link * clarify unit and description * address review comments * avoid money * move the counter creation example to the right place * update the example to avoid anything related to billing/accounting * mention that BMP is basically utf8mb3 * Update specification/metrics/new_api.md Co-authored-by: Leighton Chen <lechen@microsoft.com> * Update specification/metrics/new_api.md Co-authored-by: Leighton Chen <lechen@microsoft.com> * Update specification/metrics/new_api.md Co-authored-by: Leighton Chen <lechen@microsoft.com> * explain why 63 was chosen for unit string length Co-authored-by: Leighton Chen <lechen@microsoft.com> Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
Changes
During the 03/18/2021 Metrics API/SDK SIG Meeting, we've agreed to focus on
Counter
as the 1st step to tackle the instruments.This will be discussed during the upcoming (3/25) Metrics API/SDK SIG meeting.
Related issues #
Related oteps OTEP146