-
Notifications
You must be signed in to change notification settings - Fork 471
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
Refactor/metric exporters non plural #2255
Refactor/metric exporters non plural #2255
Conversation
Make metrics more consistent with other signals
Make metrics more consistent with other signals
@lalitb - PR that addresses your comment #2241 (comment) about MetricsExporter |
c4518e0
to
2532bca
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2255 +/- ##
=====================================
Coverage 79.4% 79.4%
=====================================
Files 121 121
Lines 20962 20962
=====================================
Hits 16656 16656
Misses 4306 4306 ☔ View full report in Codecov by Sentry. |
2532bca
to
c1d2f0f
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.
LGTM.
We need to consolidate the changelog entries and make a separate migration guide, as there are quite a few changes for the next release.
related to comment: #2221 (comment)
Changes
De-pluralize MetricExporter types:
MetricsExporter{Builder}
->MetricExporter{Builder}
InMemoryMetricsExporter{Builder}
->InMemoryMetricExporter{Builder}
PushMetricsExporter
->PushMetricExporter
As I was making these changes I noticed that there is also
InMemoryLogsExporter
andLogExporter
. Will open another PR to address this.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes