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

Reduces overall memory allocation in DynatraceExporterV2 while exporting #3766

Conversation

belks
Copy link
Contributor

@belks belks commented Apr 12, 2023

In applications with a lot of metrics the Dynatrace exporter collects all lines to be exported in one list before sending them in batches. This allocates quite a lot of unnecessary memory.
This PR changes this behavior so that batches are sent immediately and already sent lines are not kept in memory.

@pivotal-cla
Copy link

@belks Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@belks Thank you for signing the Contributor License Agreement!

@belks belks force-pushed the stbe/dynatrace_exporter_allocated_memory branch from 2492e56 to ff9135a Compare April 12, 2023 15:14
@belks belks changed the base branch from main to 1.9.x April 12, 2023 15:14
@shakuzen shakuzen added registry: dynatrace A Dynatrace Registry related issue performance Issues related to general performance enhancement A general enhancement labels Apr 13, 2023
@shakuzen shakuzen added this to the 1.9.11 milestone Apr 13, 2023
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. @pirgeo let us know if you have any concerns with this change.

@@ -139,9 +136,22 @@ public void export(List<Meter> meters) {
// Lines that are too long to be ingested into Dynatrace, as well as lines that
// contain NaN or Inf values are not returned from "toMetricLines", and are
// therefore dropped.
Copy link
Member

Choose a reason for hiding this comment

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

This comment should probably also be moved to where toMetricLines is being called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Moved the comment closer to toMetricLines

Copy link
Contributor

@pirgeo pirgeo left a comment

Choose a reason for hiding this comment

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

No concerns from my side!

@belks belks force-pushed the stbe/dynatrace_exporter_allocated_memory branch from ff9135a to 3deb668 Compare April 13, 2023 07:03
@belks belks force-pushed the stbe/dynatrace_exporter_allocated_memory branch from 3deb668 to 978ed02 Compare April 13, 2023 07:10
@shakuzen shakuzen merged commit 134e7ab into micrometer-metrics:1.9.x Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement performance Issues related to general performance registry: dynatrace A Dynatrace Registry related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants