Add filesystem scraper parsing to opentelemetry-lib #25
Conversation
remappers/hostmetrics/filesystem.go
Outdated
| } | ||
|
|
||
| for deviceKey, totalfsusage := range totalUsagePerDevice { | ||
| device, mpoint, fstype = parseDeviceKey(deviceKey) |
There was a problem hiding this comment.
I am wondering what happens if deviceKey isn't a valid one? The parseDeviceKey function should return an error to handle it, and propagate it to the caller, which should then return it to its caller as an error. The remapFilesystemMetrics returns an error, but in this it may fail silently.
There was a problem hiding this comment.
The whole logic of using the device key and then parsing looks like introduced complexity from us. We are first creating the device key by doing a sum and then parsing it(?) I am quite sure we should be able to merge this for loop in the first one but even if we are not we should be able to avoid the re-parsing of the key.
There was a problem hiding this comment.
Let me try to simplify the logic
|
@lahsivjar : Refactored the code with below changes mainly: |
lahsivjar
left a comment
lahsivjar
left a comment
There was a problem hiding this comment.
Some slight modifications to simplify the device key creation logic a bit. Rest LGTM!
lahsivjar
left a comment
lahsivjar
left a comment
There was a problem hiding this comment.
Approving it now in case this is time sensitive and needs to be merged quickly. If not then would get great to get the above refactor done otherwise okay to do it in a followup PR to, I will leave it up to you.
lahsivjar
left a comment
lahsivjar
left a comment
There was a problem hiding this comment.
Thanks for making all the changes, LGTM!
5 participants
Add filesystem scraper parsing to opentelemetry-lib
Relates: https://github.com/elastic/opentelemetry-dev/issues/191