Allow a configurable prefix to prepend the metric #10
Conversation
|
|
||
| ```elixir | ||
| config :metrix, | ||
| metric_prefix: "my-prefix." |
There was a problem hiding this comment.
Can we remove the metric_ part of the name here (and throughout) to be consistent w/ how we're handling the global context?
There was a problem hiding this comment.
Yes, it was only for clarity wrt what is being prefixed.
| @@ -22,6 +24,18 @@ defmodule Metrix do | |||
| end | |||
| end | |||
|
|
|||
| defp initial_modifiers do | |||
There was a problem hiding this comment.
What's the purpose of this pass-through function? Seems like we only need one function to get the initial modifiers.
There was a problem hiding this comment.
You're right, I was thinking ahead and got lost.
| @@ -0,0 +1,38 @@ | |||
| defmodule Metrix.Modifiers do | |||
There was a problem hiding this comment.
I like the idea of having a single Agent handle all global state (context and prefix). If you want to do that, I'd be 👍
Wonder if it should be a separate PR so we don't conflate things?
There was a problem hiding this comment.
Makes sense to me.
| end | ||
|
|
||
| defp metric_prefix_modifier(modifiers) do | ||
| case Application.get_env(:metrix, :metric_prefix) do |
There was a problem hiding this comment.
Thinking that having the :metric_prefix (or, now, :prefix) atom in both modules is a bit of a code smell.
Configure it like this: ``` config :metrix, prefix: "my-prefix." ```
Note: I have an alternative approach in another PR coming.
I added a
Modifiersmodule to house the prefix. This could be enhanced with other modifiers or merged into theContextmodule.