Skip to content
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.

Sign up for GitHub

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

Allow for custom labels on prometheus metrics #393

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow for custom labels on prometheus metrics #393

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44cdd43
feat: allow for custom labels on prometheus metrics
matdurand Sep 14, 2023
cb73514
fix: move to a config struct pattern
matdurand Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions components/metrics/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,28 @@ import (
"github.com/prometheus/client_golang/prometheus"
)

func NewPrometheusMetricsBuilder(prometheusRegistry prometheus.Registerer, namespace string, subsystem string) PrometheusMetricsBuilder {
return PrometheusMetricsBuilder{
type Option func(b *PrometheusMetricsBuilder)

// CustomLabel allow to provide a custom metric label and a function to compute the value
func CustomLabel(label string, fn LabelComputeFn) Option {
return func(b *PrometheusMetricsBuilder) {
b.customLabels = append(b.customLabels, metricLabel{
label: label,
computeFn: fn,
})
}
}

func NewPrometheusMetricsBuilder(prometheusRegistry prometheus.Registerer, namespace string, subsystem string, opts ...Option) PrometheusMetricsBuilder {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @matdurand, sorry about the delay. I wanted to comment last week and just realized I didn't do it. 🤦‍♂️

I have just one comment regarding the API — most Watermill components use a "Config" struct instead of the Option pattern, and I feel it would be a good idea to be consistent, just for the improved developer experience. This means we would need another constructor, something like NewPrometheusMetricsBuilderWithConfig. What do you think?

Copy link
Author

@matdurand matdurand Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @m110, yeah sure, consistency is paramount. I replaced Option with config. I also renamed "custom" for "additional", just because it sounded better imo. Let me know if these changes are enough.

I also fixed an issue do deduplicate if you pass an additionalLabel that is already in the base labels. I had that issue locally in my test project because I needed to override the "publisher_name" label for a special case.

builder := PrometheusMetricsBuilder{
Namespace: namespace,
Subsystem: subsystem,
PrometheusRegistry: prometheusRegistry,
}
for _, opt := range opts {
opt(&builder)
}
return builder
}

// PrometheusMetricsBuilder provides methods to decorate publishers, subscribers and handlers.
Expand All @@ -22,6 +38,8 @@ type PrometheusMetricsBuilder struct {

Namespace string
Subsystem string

customLabels []metricLabel
}

// AddPrometheusRouterMetrics is a convenience function that acts on the message router to add the metrics middleware
Expand All @@ -38,6 +56,7 @@ func (b PrometheusMetricsBuilder) DecoratePublisher(pub message.Publisher) (mess
d := PublisherPrometheusMetricsDecorator{
pub: pub,
publisherName: internal.StructName(pub),
customLabels: b.customLabels,
}

d.publishTimeSeconds, err = b.registerHistogramVec(prometheus.NewHistogramVec(
Expand All @@ -47,7 +66,7 @@ func (b PrometheusMetricsBuilder) DecoratePublisher(pub message.Publisher) (mess
Name: "publish_time_seconds",
Help: "The time that a publishing attempt (success or not) took in seconds",
},
publisherLabelKeys,
appendCustomLabels(publisherLabelKeys, b.customLabels),
))
if err != nil {
return nil, errors.Wrap(err, "could not register publish time metric")
Expand All @@ -61,6 +80,7 @@ func (b PrometheusMetricsBuilder) DecorateSubscriber(sub message.Subscriber) (me
d := &SubscriberPrometheusMetricsDecorator{
closing: make(chan struct{}),
subscriberName: internal.StructName(sub),
customLabels: b.customLabels,
}

d.subscriberMessagesReceivedTotal, err = b.registerCounterVec(prometheus.NewCounterVec(
Expand All @@ -70,7 +90,7 @@ func (b PrometheusMetricsBuilder) DecorateSubscriber(sub message.Subscriber) (me
Name: "subscriber_messages_received_total",
Help: "The total number of messages received by the subscriber",
},
append(subscriberLabelKeys, labelAcked),
appendCustomLabels(append(subscriberLabelKeys, labelAcked), b.customLabels),
))
if err != nil {
return nil, errors.Wrap(err, "could not register time to ack metric")
Expand Down
10 changes: 8 additions & 2 deletions components/metrics/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var (
// HandlerPrometheusMetricsMiddleware is a middleware that captures Prometheus metrics.
type HandlerPrometheusMetricsMiddleware struct {
handlerExecutionTimeSeconds *prometheus.HistogramVec
customLabels []metricLabel
}

// Middleware returns the middleware ready to be used with watermill's Router.
Expand All @@ -45,6 +46,9 @@ func (m HandlerPrometheusMetricsMiddleware) Middleware(h message.HandlerFunc) me
labels := prometheus.Labels{
labelKeyHandlerName: message.HandlerNameFromCtx(ctx),
}
for _, customLabel := range m.customLabels {
labels[customLabel.label] = customLabel.computeFn(ctx)
}

defer func() {
if err != nil {
Expand All @@ -62,7 +66,9 @@ func (m HandlerPrometheusMetricsMiddleware) Middleware(h message.HandlerFunc) me
// NewRouterMiddleware returns new middleware.
func (b PrometheusMetricsBuilder) NewRouterMiddleware() HandlerPrometheusMetricsMiddleware {
var err error
m := HandlerPrometheusMetricsMiddleware{}
m := HandlerPrometheusMetricsMiddleware{
customLabels: b.customLabels,
}

m.handlerExecutionTimeSeconds, err = b.registerHistogramVec(prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Expand All @@ -72,7 +78,7 @@ func (b PrometheusMetricsBuilder) NewRouterMiddleware() HandlerPrometheusMetrics
Help: "The total time elapsed while executing the handler function in seconds",
Buckets: handlerExecutionTimeBuckets,
},
handlerLabelKeys,
appendCustomLabels(handlerLabelKeys, b.customLabels),
))
if err != nil {
panic(errors.Wrap(err, "could not register handler execution time metric"))
Expand Down
14 changes: 14 additions & 0 deletions components/metrics/labels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,17 @@ func labelsFromCtx(ctx context.Context, labels ...string) prometheus.Labels {

return ctxLabels
}

type LabelComputeFn func(msgCtx context.Context) string
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to reflect on that signature. Is it enough to use only the context to get a label's value? Should we also pass the message pointer? For my use case, the additional labels are static, so I basically do this

        metrics.MetricLabel{
		Label: "service",
		ComputeFn: func(ctx context.Context) string {
			return "my_service"
		},
	},

but there might be a use case for getting something from the message itself as a label value.


type metricLabel struct {
label string
computeFn LabelComputeFn
}

func appendCustomLabels(labels []string, customs []metricLabel) []string {
for _, label := range customs {
labels = append(labels, label.label)
}
return labels
}
4 changes: 4 additions & 0 deletions components/metrics/publisher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type PublisherPrometheusMetricsDecorator struct {
pub message.Publisher
publisherName string
publishTimeSeconds *prometheus.HistogramVec
customLabels []metricLabel
}

// Publish updates the relevant publisher metrics and calls the wrapped publisher's Publish.
Expand All @@ -37,6 +38,9 @@ func (m PublisherPrometheusMetricsDecorator) Publish(topic string, messages ...*
if labels[labelKeyHandlerName] == "" {
labels[labelKeyHandlerName] = labelValueNoHandler
}
for _, customLabel := range m.customLabels {
labels[customLabel.label] = customLabel.computeFn(ctx)
}
start := time.Now()

defer func() {
Expand Down
4 changes: 4 additions & 0 deletions components/metrics/subscriber.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type SubscriberPrometheusMetricsDecorator struct {
subscriberName string
subscriberMessagesReceivedTotal *prometheus.CounterVec
closing chan struct{}
customLabels []metricLabel
}

func (s SubscriberPrometheusMetricsDecorator) recordMetrics(msg *message.Message) {
Expand All @@ -33,6 +34,9 @@ func (s SubscriberPrometheusMetricsDecorator) recordMetrics(msg *message.Message
if labels[labelKeyHandlerName] == "" {
labels[labelKeyHandlerName] = labelValueNoHandler
}
for _, customLabel := range s.customLabels {
labels[customLabel.label] = customLabel.computeFn(ctx)
}

go func() {
if subscribeAlreadyObserved(ctx) {
Expand Down