Move logic to build a Labeled Metric Name to a util file. #30796
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #30796 +/- ##
============================================
- Coverage 71.47% 70.86% -0.61%
- Complexity 0 2990 +2990
============================================
Files 710 1062 +352
Lines 104815 132697 +27882
Branches 0 3230 +3230
============================================
+ Hits 74915 94034 +19119
- Misses 28268 35570 +7302
- Partials 1632 3093 +1461
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
R: @m-trieu |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
JayajP
changed the title
JayajP
force-pushed
the
cache-v2
branch
2 times, most recently
from
ed0dc2e to
e3f3012
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #30796 +/- ##
============================================
+ Coverage 70.95% 71.08% +0.12%
+ Complexity 4470 2987 -1483
============================================
Files 1257 1063 -194
Lines 140917 132815 -8102
Branches 4305 3228 -1077
============================================
- Hits 99989 94411 -5578
+ Misses 37451 35312 -2139
+ Partials 3477 3092 -385
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
JayajP
changed the title
| try { | ||
| Map<String, String> labels = splitter.split(metricNameSplit.get(1)); | ||
| return Optional.of(ParsedMetricName.create(metricNameSplit.get(0), labels)); | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
should we log something here?
There was a problem hiding this comment.
or maybe just move the return statement below into here to show that we only do the latter when an an exception is thrown
|
Java IO precommit failing due to #30941 |
* Extract logic to build labeled MetricNames to a util Class * Address comments
* Extract logic to build labeled MetricNames to a util Class * Address comments
Move logic to build a
Labeled Metric Nameto a util file. Currently all of this logic is inBigQuerySinkMetrics; however, we can make use of these helper classes to add metrics on other sinks.Additionally this CL includes a few optimizations for Labeled Metric Names described below.
Optimizing Building Metric Names:
Currently we build a labeled metric name by populating a new
NavigableMapwith all metric labels, and then iterating over theMapand appending to aStringBuilder. We can optimize the current method with the following:StringBuilderinstead of first populating aNavigableMap(supported with a new wrapper class onStringBuilder,MetricNameBuilder).appendMetricLabels to theStringBuilderinstead of first concatenating the <Label value, Label key> pair src.Optimizing Parsing Metric Names:
Make use of Splittler.MapSplitter to parse labels from a mangled
metric name.Some quick load tests showed that this makes creating/parsing labeled metrics 33%/15% faster respectively.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.