-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 resource to vmreceiver. #31
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #31 +/- ##
==========================================
+ Coverage 68.94% 69.05% +0.11%
==========================================
Files 82 84 +2
Lines 5461 5520 +59
==========================================
+ Hits 3765 3812 +47
- Misses 1480 1488 +8
- Partials 216 220 +4
Continue to review full report at Codecov.
|
@pjanotti vmmetrics is not on the list of receivers that we wanted to have in the core, however we haven't discussed it explicitly. Any thoughts on this? @rghetia for reference here is the currently approved list of what we agreed to have in the core: https://github.com/open-telemetry/opentelemetry-service/blob/master/docs/migrating-from-opencensus.md If we decide that we do not want to include vmmetrics then this change belongs to the "contrib" repo (which doesn't exist yet). |
I think vm metrics receiver should be in core. We didn't list it there because this receiver starts as an experimental component and lacks support for several platforms (e.g Windows, MacOS). Eventually it should be one of the default receivers. |
@tigrannajaryan In a sense this is not actually a receiver but a scraper/extractor of metrics. It is a good addition to core because it allows value out of box: just by running otelsvc one can use it to have metrics collected and shipped to their preferred backend. |
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.
@rghetia a couple of questions.
This sounds reasonable, let's make it part of the core. |
Build failure was unrelated, created #43 to follow up. Kicked another build. |
…e. (open-telemetry#31) The value of memory ballast can be now provided via SPLUNK_BALLAST_SIZE_MIB env variable. It will override the value provided via --mem-ballast-size-mib command line flag. The SPLUNK_BALLAST_SIZE_MIB env variable can be accessed from the config file. This is particularly useful for the memorylimiter processor and helps to avoid specifying the ballast size twice.
fixes #30.