-
Notifications
You must be signed in to change notification settings - Fork 483
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
chore: metrics collector - proposition - ON HOLD #36
Conversation
@mwear ^^ |
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
==========================================
+ Coverage 94.32% 94.74% +0.42%
==========================================
Files 57 54 -3
Lines 3295 3749 +454
Branches 351 396 +45
==========================================
+ Hits 3108 3552 +444
- Misses 187 197 +10
|
"collector" name is potentially confusing with the opentelemetry collector |
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.
Except the naming issue and my comment, i'm okay merging this as-is.
However in the long term i hope that we can provide a lightweight alternative without C++ addon (which is hard to maintain), most metrics are accesible anyway from javascript.
Also i'm not a fan of systeminformations
because of it rely on .exec` which spawn process which is quite performance costly.
"nyc": "^15.0.0", | ||
"rimraf": "^3.0.2", | ||
"sinon": "^7.5.0", | ||
"systeminformation": "^4.23.1", |
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.
I believe systeminformation
is needed at runtime for network metrics.
I'm keeping this on hold, as it needs anyway refactoring due to changes in metrics which are still either in review or waits for those in review and then few new to be implemented |
refactored in new PR |
Which problem is this PR solving?
metrics, add support for auto collecting default recommended metrics opentelemetry-js#623
and
Auto Metrics collector like micrometer.io for java opentelemetry-js#955
Short description of the changes
FYI
Compiling packages works fine locally, compiling on circleCI and then installing from there is not yet done / wip
This is a proposition, naming conventions can be changed easily.