Add extra-labels to Ring collector

[psalaberria002]

Currently the default labels are [:method :status :statusClass :path].

I would like to have the ability to add extra labels from the request or response.

If this is not the right approach, do you have any other ideas for how to solve it?

Thank you.

[xsc]

@psalaberria002 Thanks for addressing this!

I think there might be a cleaner, more Ring-like way of doing this, namely by using a well known field (e.g. :iapetos/labels) in the request or response map. Then, request labels could be added by middlewares on top, while response labels could be added by middlewares below (or the handler itself). What do you think?

[psalaberria002]

Is this closer to what you were suggesting? It gets labels (specific ns kw) from both the request and the response.

I still don't like the need to define labels when calling ring/initialize. Do you know if this can be done on the fly by some middleware?

[xsc]

@psalaberria002 I don't think you can add labels on-demand since collectors (and thus their labels) have to be registered before they are ever used. I don't see a way of dynamically adding/removing labels in the code or the docs.

As to your implementation: Looking good! Only thing I'm uncertain about is whether :iapetos.collector.ring/labels is maybe a bit verbose/inconvenient to use, so maybe something like :iapetos/labels or a plain :metric-labels might be preferable.

[psalaberria002]

I just wanted to be specific, since these labels are only relevant for ring metrics. Why is :iapetos/labels a better kw? Won't this be confusing? Give me a good reason and I change it ;)

[xsc]

@psalaberria002 Since it's a key within a Ring request or response map I think the "only relevant for ring metrics" part is given by context. Also, :iapetos/labels and :metric-labels are arguably more stable/reusable than tying the key to a specific namespace.

But, maybe the fact that we're arguing about this shows that it's not the correct approach. How do you feel about having an additional :label-fn option instead that takes request and response as parameters and computes the additional metric labels? Cleans up the code a bit and is more flexible than our current approach.

[psalaberria002]

Great idea! Leaving the implementation up to the user sounds more flexible. Will try to have a patch during the day. Thank you for the fast replies.

[psalaberria002]

Any updates on this one?

[xsc]

I removed the default-label-fn and added a dedicated test case. Committed to master in f8d1fa2.

[xsc]

Release Notes

[psalaberria002]

Great! Thank you.