[receiver/mongodbatlas] Adds additional disk & process metrics #36694
Conversation
There was a problem hiding this comment.
I don't think the content of this file is what you intended?
There was a problem hiding this comment.
@schmikei nope, forgot to push up those changes!. should be good now though
| @@ -8,7 +8,7 @@ | |||
| | Issues | [](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aopen+is%3Aissue+label%3Areceiver%2Fmongodbatlas) [](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aclosed+is%3Aissue+label%3Areceiver%2Fmongodbatlas) | | |||
| | [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [@schmikei](https://www.github.com/schmikei) \| Seeking more code owners! | | |||
|
|
|||
| [beta]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#beta | |||
| [beta]: https://github.com/open-telemetry/opentelemetry-collector#beta | |||
There was a problem hiding this comment.
Not super familiar but just following this link right now does not take me to appropriate beta description located here: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#beta
There was a problem hiding this comment.
@schmikei hmm, I don't think I changed this manually (just had it generated...). But I can double check.
There was a problem hiding this comment.
yeah...mdatagen makes the change you see here...
| switch meas.Name { | ||
| case "DISK_PARTITION_THROUGHPUT_READ": | ||
| fallthrough | ||
| case "DISK_PARTITION_THROUGHPUT_WRITE": |
There was a problem hiding this comment.
| case "DISK_PARTITION_THROUGHPUT_WRITE": | |
| case "DISK_PARTITION_THROUGHPUT_WRITE", "DISK_PARTITION_THROUGHPUT_READ": |
Would it not be simpler to not relying on the fallthrough? I at least think it reads a tad bit better
There was a problem hiding this comment.
@schmikei can you tell I haven't been writing go consistently in awhile...Looks like a good change to me!
|
|
||
| // Combine data point values with matching timestamps | ||
| for j, totalMeas := range dptTotalMeas.DataPoints { | ||
| if totalMeas.Timestamp != meas.DataPoints[j].Timestamp || |
There was a problem hiding this comment.
Will probably need to test to make sure matching timestamps is sufficent enough to combine datapoints. I may have to poke around the API to make sure that is a valid assumption at this step.
There was a problem hiding this comment.
@schmikei I have done some manual spot checking and it has looked good so far (multiple datapoints coming through in a single collection, and the totals being created properly).
Outside of that I'll leave it up to you. I can rip this out if need be, as ultimately it was a "nice to have" although hopefully it doesn't come to it.
StefanKurek
force-pushed
the
feat/update-mongodbatlas
branch
3 times, most recently
from
d27bbb9 to
034b24f
Compare
StefanKurek
force-pushed
the
feat/update-mongodbatlas
branch
from
034b24f to
ace3750
Compare
…tor-contrib into feat/update-mongodbatlas
|
@djaglowski as a codeowner I've tested this change and I think it should be good to merge at this point |
…telemetry#36694) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds additional disk and process metrics to flesh out available monitoring metrics from the API. No new API calls were needed as we were already getting this data already. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#36525 <!--Describe what testing was performed and which tests were added.--> #### Testing New tests generated and ran. <!--Describe the documentation added.--> #### Documentation New documentation generated. <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: schmikei <[email protected]>
…telemetry#36694) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds additional disk and process metrics to flesh out available monitoring metrics from the API. No new API calls were needed as we were already getting this data already. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#36525 <!--Describe what testing was performed and which tests were added.--> #### Testing New tests generated and ran. <!--Describe the documentation added.--> #### Documentation New documentation generated. <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: schmikei <[email protected]>
Description
Adds additional disk and process metrics to flesh out available monitoring metrics from the API. No new API calls were needed as we were already getting this data already.
Link to tracking issue
Fixes #36525
Testing
New tests generated and ran.
Documentation
New documentation generated.