dferstay commented on a change in pull request #669:
URL: https://github.com/apache/pulsar-client-go/pull/669#discussion_r753735421
##########
File path: pulsar/internal/metrics.go
##########
@@ -114,183 +115,243 @@ func NewMetricsProvider(metricsCardinality int,
userDefinedLabels map[string]str
metrics := &Metrics{
metricsLevel: metricsCardinality,
messagesPublished:
prometheus.NewCounterVec(prometheus.CounterOpts{
+ Namespace: userDefinedMetricsNamespace,
Review comment:
@cckellogg ,
> If the namespace and subsystem are empty strings will that have any effect
on the metric name?
No it will not. In pulsar/internal/metrics.go we currently create
prometheus Opts structs with zero-value (`""`) Namespace and Subsystem fields:
https://github.com/apache/pulsar-client-go/blob/master/pulsar/internal/metrics.go#L116
> Also, for my knowledge does setting the subsystem and namespace change the
metric name? For example, this `pulsar_client_messages_published` =>
`namespace_subsystem_pulsar_client_messages_published`?
Yes, setting the namespace and/or subsystem changes the fully-qualified name
of the metric; your example is correct.
> Curious, what are some advantages of this approach over others ones like
custom labels or providing a custom prefix string to apply to the metric names?
Providing a custom prefix string would have an equivalent effect.
Custom labels could be used for disambiguation as well, though consumption
through PromQL would have to specify the appropriate label values.
> Are the namespace and subsystem going to be deprecated in later versions?
Oof, good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
