-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[awsemfexporter] Group exported metrics by labels #2317
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2317 +/- ##
==========================================
+ Coverage 72.72% 73.75% +1.02%
==========================================
Files 410 412 +2
Lines 25355 25475 +120
==========================================
+ Hits 18440 18789 +349
+ Misses 6368 6133 -235
- Partials 547 553 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
780abbb
to
32be859
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
e229569
to
ce8e948
Compare
@bogdandrutu Kindly ping for reviewing and merge. Thanks. |
@mxiamxia please resolve comments that are addressed and resolve merge conflicts. |
@mxiamxia this needs a rebase |
60f0eb4
to
a38f82c
Compare
a38f82c
to
6078537
Compare
6078537
to
868dc42
Compare
Rebased the commits and Resolved the conflicts. |
This PR is the 2nd part of splitting #1891 which was originally done by @kohrapha.
Currently, each incoming metric is pushed to CloudWatch logs as a separate log. However, many metrics share the same labels so this results in a lot of duplicate data. To solve this, this PR implements batching of metrics by their labels such that metrics with the same set of labels will be exported together.
Specifically, metrics are batched together if they have the same:
The batched metrics are further split up if
metric_declarations
are defined. Currently, the filtered metrics are split up by the metric declaration rules they match. Since they have the same labels, they will have the same dimensions if they match the same metric declaration rules.Caveat: 2 groups of filtered metrics can still share the same dimension sets if their metric declarations result in the same dimension set. We currently don't perform this check to group the 2 groups together.
Implementation Details
Since this PR includes a lot of refactoring, I will give an overview of how the new metric translation logic works. Given a list of
ResourceMetrics
viaemfExporter.pushMetricsData
,ResourceMetrics
in the list, we will add its metrics intogroupedMetrics
(a map consisting of batched metrics).ResourceMetrics
, we create aCWMetricMetadata
which consists of metadata (i.e. namespace, timestamp, log group, log stream, instrumentation library name) associated with the given metric. This will be added togroupedMetrics
for future processing.DataPoints
from each metric. For eachDataPoint
, we define its "group key" using its labels, namespace, timestamp, log group, and log stream. We use this group key to add the metric to its corresponding group ingroupedMetrics
.groupedMetrics
, we iterate through each group and translate it intoCWMetric
. In this stage, we will filter out metrics if there are metric declarations defined and set the dimensions for exported metrics (w/ rolled-up dimensions).CWMetric
into an EMF log and push it to CloudWatch using the appropriate log group and log stream found in the group'sCWMetricMetadata
.Testing:
Tests were added for new functions and tests for modified functions were updated. Additionally, this PR was tested in a sample environment using an NGINX server on EKS. Given the following config (same as in #2):
we get the following cases: