Skip to content
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

Fix todo issue in googlecloudmonitoringreceiver #35760

Conversation

abhishek-at-cloudwerx
Copy link
Contributor

@abhishek-at-cloudwerx abhishek-at-cloudwerx commented Oct 13, 2024

Description:

Fix todo comments issue in googlecloudmonitoringreceiver receiver component

Link to tracking issue

Testing

Documentation

- Remove redundant TODO lines from googlecloudmonitoringreceiver receiver component
@abhishek-at-cloudwerx abhishek-at-cloudwerx changed the title Fix todo in googlecloudmonitoringreceiver Fix todo issue in googlecloudmonitoringreceiver Oct 13, 2024
- Add changelog entry file for googlecloudmonitoringreceiver component
- Mention issue type in .chloggen file
@@ -314,8 +314,6 @@ func (mr *monitoringReceiver) convertGCPTimeSeriesToMetrics(metrics pmetric.Metr
mr.metricsBuilder.ConvertSumToMetrics(timeSeries, m)
case metric.MetricDescriptor_DELTA:
mr.metricsBuilder.ConvertDeltaToMetrics(timeSeries, m)
// TODO: Add support for HISTOGRAM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we are removing these. We should keep them around until it is implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct.
I'll close the PR for now as it's important to implement this in the future.

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, this TODO needs to be implemented.

@abhishek-at-cloudwerx
Copy link
Contributor Author

Support for histograms will be needed in the future, so the TODO will remain as is.

@abhishek-at-cloudwerx abhishek-at-cloudwerx deleted the remove-todo-googlecloudmonitoringreceiver branch October 16, 2024 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants