From aa0713959bb4b8f9d25162005573e35959002456 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 22 Apr 2024 15:39:18 +0300 Subject: [PATCH 1/4] TD-788: Adds 'hackney_host_request_time' histogram metric --- src/woody_event_handler_otel.erl | 8 ++++---- src/woody_hackney_prometheus.erl | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/woody_event_handler_otel.erl b/src/woody_event_handler_otel.erl index 4307dac..63cfd31 100644 --- a/src/woody_event_handler_otel.erl +++ b/src/woody_event_handler_otel.erl @@ -25,14 +25,14 @@ %% Client events handle_event(Event, RpcID, _Meta = #{url := Url}, _Opts) when ?IS_CLIENT_INTERNAL(Event) -> - with_span(otel_ctx:get_current(), mk_ref(RpcID), fun(SpanCtx) -> - _ = otel_span:add_event(SpanCtx, atom_to_binary(Event), #{url => Url}) + with_existing_span(otel_ctx:get_current(), mk_ref(RpcID), fun(SpanCtx) -> + _ = otel_span:add_event(SpanCtx, woody_util:normalize_event_name(Event), #{url => Url}) end); %% Internal error handling handle_event(?EV_INTERNAL_ERROR, RpcID, Meta = #{error := Error, class := Class, reason := Reason}, _Opts) -> Stacktrace = maps:get(stack, Meta, []), Details = io_lib:format("~ts: ~ts", [Error, Reason]), - with_span(otel_ctx:get_current(), mk_ref(RpcID), fun(SpanCtx) -> + with_existing_span(otel_ctx:get_current(), mk_ref(RpcID), fun(SpanCtx) -> _ = otel_span:record_exception(SpanCtx, genlib:define(Class, error), Details, Stacktrace, #{}), otel_maybe_cleanup(Meta, SpanCtx) end); @@ -68,7 +68,7 @@ span_end(Ctx, Key, OnBeforeEnd) -> ok end. -with_span(Ctx, Key, F) -> +with_existing_span(Ctx, Key, F) -> SpanCtx = woody_util:span_stack_get(Key, Ctx, otel_tracer:current_span_ctx(Ctx)), _ = F(SpanCtx), ok. diff --git a/src/woody_hackney_prometheus.erl b/src/woody_hackney_prometheus.erl index 310a381..d02248c 100644 --- a/src/woody_hackney_prometheus.erl +++ b/src/woody_hackney_prometheus.erl @@ -26,6 +26,7 @@ -define(NB_REQUESTS, hackney_nb_requests). -define(TOTAL_REQUESTS, hackney_total_requests). -define(HOST_NB_REQUESTS, hackney_host_nb_requests). +-define(HOST_REQUEST_TIME, hackney_host_request_time). %% milliseconds -define(HOST_CONNECT_TIMEOUT, hackney_host_connect_timeout). -define(HOST_CONNECT_ERROR, hackney_host_connect_error). -define(HOST_NEW_CONNECTION, hackney_host_new_connection). @@ -50,6 +51,14 @@ new(counter, [hackney, nb_requests]) -> {labels, [host]}, {help, "Number of running requests."} ]), + true = prometheus_histogram:declare([ + {name, ?HOST_REQUEST_TIME}, + {registry, registry()}, + {labels, [host]}, + {buckets, duration_buckets()}, + {duration_unit, seconds}, + {help, "Request time."} + ]), [ true = prometheus_counter:declare([ {name, Name}, @@ -117,6 +126,9 @@ decrement_counter(_Name, _Value) -> -spec update_histogram(name(), fun(() -> ok) | number()) -> ok. update_histogram(_Name, Fun) when is_function(Fun, 0) -> Fun(); +update_histogram([hackney, Host, request_time], Value) -> + _ = prometheus_histogram:observe(registry(), ?HOST_REQUEST_TIME, [Host], Value), + ok; update_histogram(_Name, _Value) -> ok. @@ -132,3 +144,20 @@ update_meter(_Name, _Value) -> registry() -> default. + +duration_buckets() -> + %% Seconds + [ + 0.001, + 0.005, + 0.010, + 0.025, + 0.050, + 0.100, + 0.250, + 0.500, + 1, + 2.5, + 5, + 10 + ]. From 3d17f4cd8865b14418d234106febf6105934049c Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 22 Apr 2024 15:40:13 +0300 Subject: [PATCH 2/4] Adds missing func --- src/woody_util.erl | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/woody_util.erl b/src/woody_util.erl index 7c7e5fc..5686fed 100644 --- a/src/woody_util.erl +++ b/src/woody_util.erl @@ -16,6 +16,7 @@ -export([span_stack_put/3]). -export([span_stack_get/3]). -export([span_stack_pop/2]). +-export([normalize_event_name/1]). %% %% Internal API @@ -95,3 +96,37 @@ span_stack_pop(Key, Context) -> Context1 = otel_ctx:set_value(Context, ?OTEL_SPANS_STACK, Stack1), {ok, SpanCtx, ParentSpanCtx, Context1} end. + +%% TODO Normalize according spec: +%% https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/events.md +%% +%% Specifically this: +%% +%% Semantic conventions MUST limit names to printable Basic Latin +%% characters (more precisely to U+0021 .. U+007E subset of Unicode +%% code points). It is recommended to further limit names to the +%% following Unicode code points: Latin alphabet, Numeric, Underscore, +%% Dot (as namespace delimiter). +-define(EVENT_NAME_FORBIDDEN_CHARS, [<<" ">>]). + +-spec normalize_event_name(binary() | atom()) -> opentelemetry:event_name(). +normalize_event_name(Event) when is_atom(Event) -> + normalize_event_name(atom_to_binary(Event, unicode)); +normalize_event_name(Event) when is_binary(Event) -> + binary:replace(Event, ?EVENT_NAME_FORBIDDEN_CHARS, <<$_>>, [global]). + +-ifdef(TEST). + +-include_lib("eunit/include/eunit.hrl"). + +-spec test() -> _. + +-spec normalize_event_name_test_() -> _. +normalize_event_name_test_() -> + [ + ?_assertEqual(<<"client_resolve_begin">>, normalize_event_name('client resolve begin')), + ?_assertEqual(<<"client_resolve/begin">>, normalize_event_name('client resolve/begin')), + ?_assertEqual(<<"client.resolve">>, normalize_event_name('client.resolve')) + ]. + +-endif. From edc582de12c03135f8e5677b4a32b9e91dda8c0e Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 22 Apr 2024 15:56:41 +0300 Subject: [PATCH 3/4] Bumps CI workflow --- .github/workflows/erlang-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/erlang-checks.yml b/.github/workflows/erlang-checks.yml index f29549f..262e2e9 100644 --- a/.github/workflows/erlang-checks.yml +++ b/.github/workflows/erlang-checks.yml @@ -30,7 +30,7 @@ jobs: run: name: Run checks needs: setup - uses: valitydev/erlang-workflows/.github/workflows/erlang-parallel-build.yml@v1.0.10 + uses: valitydev/erlang-workflows/.github/workflows/erlang-parallel-build.yml@v1.0.14 with: otp-version: ${{ needs.setup.outputs.otp-version }} rebar-version: ${{ needs.setup.outputs.rebar-version }} From 4198596d2d98510432364e73135849786c914f85 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Mon, 22 Apr 2024 16:43:03 +0300 Subject: [PATCH 4/4] Refactors request time buckets --- src/woody_hackney_prometheus.erl | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/woody_hackney_prometheus.erl b/src/woody_hackney_prometheus.erl index a8ffaef..99e6374 100644 --- a/src/woody_hackney_prometheus.erl +++ b/src/woody_hackney_prometheus.erl @@ -26,7 +26,7 @@ -define(NB_REQUESTS, hackney_nb_requests). -define(TOTAL_REQUESTS, hackney_total_requests). -define(HOST_NB_REQUESTS, hackney_host_nb_requests). --define(HOST_REQUEST_TIME, hackney_host_request_time). %% milliseconds +-define(HOST_REQUEST_TIME, hackney_host_request_time). -define(HOST_CONNECT_TIMEOUT, hackney_host_connect_timeout). -define(HOST_CONNECT_ERROR, hackney_host_connect_error). -define(HOST_NEW_CONNECTION, hackney_host_new_connection). @@ -55,8 +55,7 @@ new(counter, [hackney, nb_requests]) -> {name, ?HOST_REQUEST_TIME}, {registry, registry()}, {labels, [host]}, - {buckets, duration_buckets()}, - {duration_unit, seconds}, + {buckets, request_time_buckets_ms()}, {help, "Request time."} ]), [ @@ -145,19 +144,15 @@ update_meter(_Name, _Value) -> registry() -> default. -duration_buckets() -> - %% Seconds +request_time_buckets_ms() -> [ - 0.001, - 0.005, - 0.010, - 0.025, - 0.050, - 0.100, - 0.250, - 0.500, - 1, - 2.5, 5, - 10 + 10, + 25, + 50, + 100, + 250, + 500, + 1000, + 10000 ].