Updates handle_event/4 to extract latency metrics from a map #21
Conversation
…g (ie. constaints errors)
|
Thanks! |
|
Maybe you also have some ecto3 tests to share :-)? |
| @@ -178,6 +178,15 @@ defmodule Prometheus.EctoInstrumenter do | |||
| end | |||
| end | |||
|
|
|||
| def handle_event(_event, latency, metadata, _config) when is_map(latency) do | |||
| latency | |||
| |> Map.put(:decode_time, Map.get(latency, :decode_time, 0)) | |||
There was a problem hiding this comment.
It can be done with Map.put_new(:decode_time, 0).
Please note that existing and my proposal both does handle when the key exists with nil value - in that case, it will remain as nil.
There was a problem hiding this comment.
That's definitely cleaner with put_new. Do we want to cover the case when it would be nil ? From what I can see the key is either present with a value or absent.
There was a problem hiding this comment.
I don't know what will be passed in it - probably better to check the doc of code producing this.
However, I'd like to ask a question in different angle - why do we need to backfill missing :decode_time as 0? Wouldn`t it skew the stat, if it happens a lot?
I think it's better to pass as-is, and log/1 or other functions need to take care of missing:decode_time.
| def handle_event(_event, latency, metadata, _config) when is_map(latency) do | ||
| latency | ||
| |> Map.put(:decode_time, Map.get(latency, :decode_time, 0)) | ||
| |> Enum.reduce(metadata, fn {key, value}, metadata -> |
There was a problem hiding this comment.
What about using Map.merge/2? Then we need to change a little bit, as the current code use latency (first arg) over metadata map for duplicates, while Map.merge/2 uses the second arg.
metadata
|> Map.merge(Map.put_lazy(latency, :decode_time, 0))
|> log()
|
Thanks for merging this. I agree, this needs some test coverage. I'll try to get a PR in when I get a chance and include some of the suggestions from @chulkilee |
Fixes #19
The incoming latency parameter is a Map and this fix extracts the required metrics and adds them to the metadata map.