Add notes about MetricExporter naming#2286
Merged
tigrannajaryan merged 5 commits intoopen-telemetry:mainfrom Feb 15, 2022
Merged
Add notes about MetricExporter naming#2286tigrannajaryan merged 5 commits intoopen-telemetry:mainfrom
tigrannajaryan merged 5 commits intoopen-telemetry:mainfrom
Conversation
reyang
added
area:sdk
jsuereth
approved these changes
Jan 29, 2022
cijothomas
approved these changes
Jan 29, 2022
carlosalberto
approved these changes
Jan 30, 2022
MrAlias
approved these changes
Feb 1, 2022
jmacd
approved these changes
Feb 7, 2022
Contributor
jmacd
left a comment
jmacd
left a comment
There was a problem hiding this comment.
Agree that MetricReader/PushMetricExporter are acceptable choices
Member
Author
|
@bogdandrutu 👋 friendly pinging assignee. we have 6 approvals, would you mind landing this one? thank you! |
rauno56
approved these changes
Feb 15, 2022
Member
|
Enough approvals, more than 2 days since last comment/change. Merging. |
carlosalberto
pushed a commit
to carlosalberto/opentelemetry-specification
that referenced
this pull request
Oct 31, 2024
If the implementors choose to implement pull metric exporters modeled as a metric reader, the only variant for metric exporter interface is push metric exporter. While the pull metric exporter is modeled as a "metric reader", the better naming for the genric "metric exporter" interface may be "push metric exporter". Just open this PR to see if there are similar naming confusions with other SDK implementors. If so, I'd believe the metric exporter and metric reader section need a re-structure in a follow-up PR since how the pull metric exporter is modeled is important to the naming of the metric exporter interface. Related: - open-telemetry/opentelemetry-js#2707 - open-telemetry/opentelemetry-js#2589 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
If the implementors choose to implement pull metric exporters modeled as a metric reader, the only variant for metric exporter interface is push metric exporter. While the pull metric exporter is modeled as a "metric reader", the better naming for the genric "metric exporter" interface may be "push metric exporter".
Just open this PR to see if there are similar naming confusions with other SDK implementors. If so, I'd believe the metric exporter and metric reader section need a re-structure in a follow-up PR since how the pull metric exporter is modeled is important to the naming of the metric exporter interface.
Related: