new: MetricNameDescriptor overload option
#126
Conversation
incertum
force-pushed
the
new/FQMetricDescriptor
branch
from
3cab56b to
0ab5efd
Compare
|
Fixes #127 FYI: Proposing to update docs separately at the end in new PRs. |
incertum
changed the title
| self.helpText = helpText | ||
| } | ||
|
|
||
| public var FQMetricName: String { |
There was a problem hiding this comment.
Two things:
- This is already nested in a type called
FQso we can drop the prefix` - We usually don't capability variables or method names in Swift
| public var FQMetricName: String { | |
| public var metricName: String { |
There was a problem hiding this comment.
name seemed simplest given metricName is already taken.
incertum
force-pushed
the
new/FQMetricDescriptor
branch
from
0ab5efd to
acef547
Compare
| /// - Parameter descriptor: An ``FQMetricDescriptor`` that provides the fully qualified name for the metric. | ||
| /// - Returns: A ``Counter`` that is registered with this ``PrometheusCollectorRegistry`` | ||
| public func makeCounter(descriptor: FQMetricDescriptor) -> Counter { | ||
| return self.makeCounter(name: descriptor.name) |
There was a problem hiding this comment.
In a follow up PR will adjust this to enable printing the HELP line if applicable for all cases.
incertum
changed the title
incertum
force-pushed
the
new/FQMetricDescriptor
branch
from
acef547 to
5580757
Compare
| /// - Parameter unitName: An optional suffix describing the metric's unit (e.g., `_seconds`, `_total`). | ||
| /// - Parameter helpText: Optional descriptive text for the metric. | ||
| /// - Returns: A new ``FQMetricDescriptor`` instance with the specified components. | ||
| public struct FQMetricDescriptor { |
There was a problem hiding this comment.
It seems like FQ is not a term of art in the prometheus space. Can we rather name this FullyQualifiedMetricNameDescriptor. We try to use as little abbreviations as possible.
There was a problem hiding this comment.
Hah, seems I missed to send my comment huh -- yeah, agreed we should not be doing FQ/FQN. We could also just skip the FullyQualified part and call it some thing else.
It doesn't seem like we have MetricDescriptor either, we could just call it that even
There was a problem hiding this comment.
How about MetricNameDescriptor?
incertum
force-pushed
the
new/FQMetricDescriptor
branch
from
5580757 to
322da1d
Compare
Adds a new way to create metrics using a MetricNameDescriptor, which makes it easier to build metric names programmatically. The changes are fully backward compatible. Signed-off-by: Melissa Kilby <[email protected]>
incertum
force-pushed
the
new/FQMetricDescriptor
branch
from
322da1d to
1532037
Compare
incertum
changed the title
MetricNameDescriptor overload option
This change adds a new way to create metrics using an
FQMetricDescriptor, which makes it easier to build metric names programmatically. The changes are fully backward compatible.Also, while the descriptor has an optional helpText property, it doesn't generate the corresponding HELP line in the output yet. I believe it would be best to handle this in a separate, follow-up PR, which I'm happy to create.
CC @fabianfett, @ktoso