Remove confusion between namespace and named meter #7
Conversation
This file contains 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
z1c0
reviewed
Jan 9, 2020
Co-Authored-By: Wolfgang Ziegler <[email protected]>
arminru
approved these changes
Jan 9, 2020
Oberon00
reviewed
Jan 13, 2020
arminru
force-pushed
the
named-meter-namespace-clarification-review
branch
from
7e6367c to
f6feff0
Compare
Oberon00
reviewed
Jan 13, 2020
specification/api-metrics-user.md
Outdated
| In case an invalid name (null or empty string) is specified, a working default `Meter` implementation as a fallback is returned | ||
| rather than returning null or throwing an exception. | ||
| A library implementing the OpenTelemetry API *may* also ignore this name and return a default instance for all calls | ||
| if it does not support "named" functionality (e.g. an implementation which is not even observability-related). |
There was a problem hiding this comment.
(e.g. an implementation which is not even observability-related)
What does this mean? @z1c0 do you know?
specification/api-metrics-user.md
Outdated
Comment on lines
25
to
26
| A library implementing the OpenTelemetry API *may* also ignore this name and return a default instance for all calls | ||
| if it does not support "named" functionality (e.g. an implementation which is not even observability-related). |
There was a problem hiding this comment.
What is this trying to say? Just that applications cannot rely on different instances being returned for different names? Then this should be stated as such. Right now this sentence sounds too vague for a spec. State the requirements that implementations of this method must fulfill, or, equivalently, the properties of this method that callers can rely on.
danielkhan
changed the title
dyladan
commented
Jan 13, 2020
discostu105
reviewed
Jan 14, 2020
| developers to instrument their code using counters, gauges, and | ||
| measures. | ||
| Metric instruments are the entry point for application and framework developers to instrument their code using counters, gauges, and measures. | ||
| Metrics are created by calling methods on a `Meter` which is in turn created by a global `MeterFactory`. |
There was a problem hiding this comment.
Didn't we change to a "Registry" term in the tracing world?
There was a problem hiding this comment.
we're updating the otep before the spec
discostu105
approved these changes
Jan 14, 2020
arminru
reviewed
Jan 14, 2020
| @@ -38,7 +36,7 @@ external systems. Metric names conform to the following syntax: | |||
| 4. Subsequent characters must be belong to the alphanumeric characters, '_', '.', and '-'. | |||
|
|
|||
| Metric names belong to a namespace. The `component` label value of the associated `Meter` serves as its namespace, | |||
| allowing the same metric name to be used in multiple libraries of code, unambiguously, within the same application. | |||
| allowing the same metric name to be used in multiple libraries of code, unambiguously, within the same application. | |||
There was a problem hiding this comment.
two spaces at the end of a line create a line break without introducing a new paragraph and were added intentionally
arminru
approved these changes
Jan 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR clarifies some wording which conflated the terms
namespaceandMeter Name. It also adds back the Meter creation section which was originally introduced by @z1c0 in open-telemetry#264 and removed in open-telemetry#250.This addresses issue open-telemetry#391