-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add metrics. #44
Add metrics. #44
Conversation
Codecov Report
@@ Coverage Diff @@
## master #44 +/- ##
==========================================
- Coverage 27.57% 22.16% -5.42%
==========================================
Files 16 18 +2
Lines 475 591 +116
==========================================
Hits 131 131
- Misses 344 460 +116
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
README.md
Outdated
|
||
### MeterRecord | ||
|
||
MeterRecord is the simple builder for the MeterData, which is the metrics format of Skywalking. |
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.
You should provide a full list of the metric data types with explanations.
We have a short version on main repo docs, https://skywalking.apache.org/docs/main/latest/en/protocols/readme/#metrics which targets to proto file.
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.
Updated.
From a metrics APIs pespective, to ease the usage, the single value metric is usually separated as counter or gauge to provide different use patterns. You could refer Java meter APIs from here, https://skywalking.apache.org/docs/skywalking-java/latest/en/setup/service-agent/java-agent/java-plugin-development-guide/#meter-plugin |
I have refer to java agent and update the API. |
Just confirm, all metrics objects(counter, gauge, histogram), are they thread safe? Such as concurrency write, and write and report concurrently. |
Yes, because they all implement |
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.
LGTM
Add metrics support.