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.

Sign up for GitHub

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

Jump to bottom

AWS EMF Exporter does not properly enforce map TTL #25058

Closed
bryan-aguilar opened this issue Aug 7, 2023 · 1 comment · Fixed by #25066
Closed

AWS EMF Exporter does not properly enforce map TTL #25058

bryan-aguilar opened this issue Aug 7, 2023 · 1 comment · Fixed by #25066
Assignees
@bryan-aguilar
Labels
bug Something isn't working exporter/awsemf awsemf exporter priority:p1 High receiver/awscontainerinsight

Comments

@bryan-aguilar
Copy link
Contributor

Component(s)

exporter/awsemf, receiver/awscontainerinsight

What happened?

Description

The AWS EMF Exporter does not properly enforce a TTL in the map used to store metric information. This map is used when performing cummulative to delta conversions. The result of this is that the map will continue to grow along with the collectors memory footprint.

We were able to observe this behavior and also quickly patch in a proof of concept fix that aggressively cleans the map each time the exporter exports. A memory graph can be seen below.

image

Green line is a collector using a published EMF Exporter and the orange line is a collector with patched EMF Exporter.

Collector version

v0.82.0

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

I am currently working on a fix for this. The change requires modifications to the /internal/aws/metrics pacakge which is also used by the Container insights receiver.
@bryan-aguilar bryan-aguilar added bug Something isn't working needs triage New item requiring triage labels Aug 7, 2023
@bryan-aguilar bryan-aguilar self-assigned this Aug 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@bryan-aguilar bryan-aguilar added priority:p1 High and removed needs triage New item requiring triage labels Aug 7, 2023
@bryan-aguilar bryan-aguilar mentioned this issue Aug 8, 2023
Merged
dmitryax pushed a commit that referenced this issue Aug 8, 2023
@bryan-aguilar
[exporter/awsemf] Enforce TTL on metric calculator maps (#25066)
a24294f 
**Description:** This PR implements a ticker to automatically enforce
the TimeToLive value on `MapWithExpiry` objects inside the
`internal/aws/metrics` package. This map object is used directly by the
Container Insights receiver and the MetricCalculator object in the same
package. Metric calculators are used by the EMF Exporter.

Changes to container insights receiver interfaces had to be made so that
shutdown on all calculators could be enforced.

In the EMF Exporter, the `metrictranslator` object now owns an
`emfCalculator` object which can be passed down to the data point
interface. The metric translator now has a shutdown method which can be
called by the emf exporter on close.

**Link to tracking Issue:** Closes #25058
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bryan-aguilar bryan-aguilar

Labels
bug Something isn't working exporter/awsemf awsemf exporter priority:p1 High receiver/awscontainerinsight
Projects
None yet
Milestone
No milestone
1 participant
@bryan-aguilar