TD-686: Adopts opentelemetry API (#28)

* Instruments swagger operation handler * Bumps deps; updates proto refs * Tweaks woody timeout testcase * Bumps OTP version * Bumps bouncer & token keeper clients
valitydev · Oct 31, 2023 · 12f5e53 · 12f5e53
1 parent 5aee0f7
commit 12f5e53
Show file tree

Hide file tree

Showing 16 changed files with 178 additions and 68 deletions.
diff --git a/.env b/.env
@@ -2,6 +2,6 @@
 # You SHOULD specify point releases here so that build time and run time Erlang/OTPs
 # are the same. See: https://github.com/erlware/relx/pull/902
 SERVICE_NAME=url-shortener
-OTP_VERSION=24.2.0
+OTP_VERSION=24.3.4
 REBAR_VERSION=3.18
-THRIFT_VERSION=0.14.2.2
+THRIFT_VERSION=0.14.2.3
diff --git a/Makefile b/Makefile
@@ -3,8 +3,6 @@
 # For example, to run with podman put `DOCKER=podman` there.
 -include Makefile.env
 
--include .env
-
 # NOTE
 # Variables specified in `.env` file are used to pick and setup specific
 # component versions, both when building a development image and when running
@@ -18,7 +16,7 @@ DEV_IMAGE_ID = $(file < .image.dev)
 
 DOCKER ?= docker
 DOCKERCOMPOSE ?= docker-compose
-DOCKERCOMPOSE_W_ENV = DEV_IMAGE_TAG=$(DEV_IMAGE_TAG) $(DOCKERCOMPOSE)
+DOCKERCOMPOSE_W_ENV = DEV_IMAGE_TAG=$(DEV_IMAGE_TAG) $(DOCKERCOMPOSE) -f compose.yaml -f compose.tracing.yaml
 REBAR ?= rebar3
 TEST_CONTAINER_NAME ?= testrunner
 
@@ -42,7 +40,7 @@ DOCKER_WC_OPTIONS := -v $(PWD):$(PWD) --workdir $(PWD)
 DOCKER_WC_EXTRA_OPTIONS ?= --rm
 DOCKER_RUN = $(DOCKER) run -t $(DOCKER_WC_OPTIONS) $(DOCKER_WC_EXTRA_OPTIONS)
 
-DOCKERCOMPOSE_RUN = $(DOCKERCOMPOSE_W_ENV) run --rm $(DOCKER_WC_OPTIONS) $(TEST_CONTAINER_NAME)
+DOCKERCOMPOSE_RUN = $(DOCKERCOMPOSE_W_ENV) run --rm $(DOCKER_WC_OPTIONS)
 
 # Utility tasks
 
@@ -53,11 +51,11 @@ wc-%: dev-image
 	$(DOCKER_RUN) $(DEV_IMAGE_TAG) make $*
 
 wdeps-shell: dev-image
-	$(DOCKERCOMPOSE_RUN) su; \
+	$(DOCKERCOMPOSE_RUN) $(TEST_CONTAINER_NAME) su; \
 	$(DOCKERCOMPOSE_W_ENV) down
 
 wdeps-%: dev-image
-	$(DOCKERCOMPOSE_RUN) make $*; \
+	$(DOCKERCOMPOSE_RUN) -T $(TEST_CONTAINER_NAME) make $*; \
 	res=$$?; \
 	$(DOCKERCOMPOSE_W_ENV) down; \
 	exit $$res

diff --git a/apps/shortener/src/shortener.app.src b/apps/shortener/src/shortener.app.src
@@ -19,7 +19,10 @@
         token_keeper_client,
         woody,
         woody_user_identity,
-        erl_health
+        erl_health,
+        opentelemetry_api,
+        opentelemetry_exporter,
+        opentelemetry
     ]},
     {env, []},
     {modules, []},

diff --git a/apps/shortener/src/shortener_bouncer.erl b/apps/shortener/src/shortener_bouncer.erl
@@ -1,6 +1,6 @@
 -module(shortener_bouncer).
 
--include_lib("bouncer_proto/include/bouncer_context_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_thrift.hrl").
 
 -export([gather_context_fragments/4]).
 -export([judge/2]).

diff --git a/apps/shortener/src/shortener_bouncer_context.erl b/apps/shortener/src/shortener_bouncer_context.erl
@@ -1,6 +1,7 @@
 -module(shortener_bouncer_context).
 
--include_lib("bouncer_proto/include/bouncer_context_v1_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_base_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_v1_thrift.hrl").
 
 -type fragment() :: bouncer_client:context_fragment().
 -type acc() :: bouncer_context_helpers:context_fragment().
@@ -45,13 +46,13 @@ build(Prototypes, {Acc0, External}) ->
 
 build(operation, #{id := OperationID} = Params, Acc) ->
     Slug = maps:get(slug, Params, #{}),
-    Acc#bctx_v1_ContextFragment{
-        shortener = #bctx_v1_ContextUrlShortener{
-            op = #bctx_v1_UrlShortenerOperation{
+    Acc#ctx_v1_ContextFragment{
+        shortener = #ctx_v1_ContextUrlShortener{
+            op = #ctx_v1_UrlShortenerOperation{
                 id = operation_id_to_binary(OperationID),
-                shortened_url = #bctx_v1_ShortenedUrl{
+                shortened_url = #ctx_v1_ShortenedUrl{
                     id = maps:get(id, Slug, undefined),
-                    owner = #bouncer_base_Entity{
+                    owner = #base_Entity{
                         id = maps:get(owner, Slug, undefined)
                     }
                 }

diff --git a/apps/shortener/src/shortener_handler.erl b/apps/shortener/src/shortener_handler.erl
@@ -1,5 +1,8 @@
 -module(shortener_handler).
 
+-include_lib("opentelemetry_api/include/otel_tracer.hrl").
+-include_lib("opentelemetry_api/include/opentelemetry.hrl").
+
 %% Swagger handler
 
 -behaviour(swag_server_ushort_logic_handler).
@@ -35,24 +38,33 @@
     swag_server_ushort:handler_opts(_)
 ) ->
     Result :: false | {true, shortener_auth:preauth_context()}.
-authorize_api_key(OperationID, ApiKey, _Context, _HandlerOpts) ->
-    ok = scoper:add_scope('swag.server', #{operation => OperationID}),
+authorize_api_key(OperationID, ApiKey, Context, _HandlerOpts) ->
+    ok = set_otel_context(Context),
     case shortener_auth:preauthorize_api_key(ApiKey) of
-        {ok, Context} ->
-            {true, Context};
+        {ok, Context1} ->
+            {true, Context1};
         {error, Error} ->
             _ = logger:info("API Key preauthorization failed for ~p due to ~p", [OperationID, Error]),
             false
     end.
 
 -spec handle_request(operation_id(), request_data(), request_ctx(), any()) ->
     {ok | error, swag_server_ushort:response()}.
-handle_request(OperationID, Req, SwagContext0, _Opts) ->
+handle_request(OperationID, Req, SwagContext, Opts) ->
+    SpanName = <<"server ", (atom_to_binary(OperationID))/binary>>,
+    ?with_span(SpanName, #{kind => ?SPAN_KIND_SERVER}, fun(_SpanCtx) ->
+        scoper:scope('swag.server', #{operation => OperationID}, fun() ->
+            handle_request_(OperationID, Req, SwagContext, Opts)
+        end)
+    end).
+
+handle_request_(OperationID, Req, SwagContext0, _Opts) ->
     try
         WoodyContext0 = create_woody_ctx(Req),
         SwagContext1 = authenticate_api_key(SwagContext0, WoodyContext0),
         AuthContext = get_auth_context(SwagContext1),
         WoodyContext1 = put_user_identity(WoodyContext0, AuthContext),
+        ok = set_context_meta(AuthContext),
         Slug = prefetch_slug(Req, WoodyContext1),
         case shortener_auth:authorize_operation(make_prototypes(OperationID, Slug), SwagContext1, WoodyContext1) of
             allowed ->
@@ -67,8 +79,6 @@ handle_request(OperationID, Req, SwagContext0, _Opts) ->
             {error, {401, #{}, undefined}};
         error:{woody_error, {Source, Class, Details}} ->
             {error, handle_woody_error(Source, Class, Details)}
-    after
-        ok = scoper:remove_scope()
     end.
 
 -spec map_error(error_type(), validation_error()) -> error_message().
@@ -127,6 +137,14 @@ collect_user_identity(AuthContext) ->
         email => shortener_auth:get_user_email(AuthContext)
     }).
 
+set_context_meta(AuthContext) ->
+    Meta = #{
+        metadata => #{
+            'user-identity' => collect_user_identity(AuthContext)
+        }
+    },
+    scoper:add_meta(Meta).
+
 handle_woody_error(_Source, result_unexpected, _Details) ->
     {500, #{}, <<>>};
 handle_woody_error(_Source, resource_unavailable, _Details) ->
@@ -255,6 +273,14 @@ get_source_url_whitelist() ->
     % upon initialization
     maps:get(source_url_whitelist, genlib_app:env(shortener, api), []).
 
+set_otel_context(#{cowboy_req := Req}) ->
+    Headers = cowboy_req:headers(Req),
+    %% Implicitly puts OTEL context into process dictionary.
+    %% Since cowboy does not reuse process for other requests, we don't care
+    %% about cleaning it up.
+    _OtelCtx = otel_propagator_text_map:extract(maps:to_list(Headers)),
+    ok.
+
 %%
 
 -type state() :: undefined.

diff --git a/apps/shortener/test/shortener_auth_SUITE.erl b/apps/shortener/test/shortener_auth_SUITE.erl
@@ -1,8 +1,8 @@
 -module(shortener_auth_SUITE).
 
--include_lib("bouncer_proto/include/bouncer_decisions_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_decision_thrift.hrl").
 -include_lib("bouncer_proto/include/bouncer_base_thrift.hrl").
--include_lib("bouncer_proto/include/bouncer_context_v1_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_v1_thrift.hrl").
 
 -include_lib("shortener_token_keeper_data.hrl").
 -include_lib("shortener_bouncer_data.hrl").
@@ -300,19 +300,19 @@ append_request_id(Params = #{header := Headers}) ->
 format_ts(Ts) ->
     genlib_rfc3339:format(Ts, second).
 
-get_operation_id(#bctx_v1_ContextFragment{
-    shortener = #bctx_v1_ContextUrlShortener{op = #bctx_v1_UrlShortenerOperation{id = OperationID}}
+get_operation_id(#ctx_v1_ContextFragment{
+    shortener = #ctx_v1_ContextUrlShortener{op = #ctx_v1_UrlShortenerOperation{id = OperationID}}
 }) ->
     OperationID.
 
 get_owner_info(Context) ->
     {get_owner_id(Context), get_user_id(Context)}.
 
-get_owner_id(#bctx_v1_ContextFragment{
-    shortener = #bctx_v1_ContextUrlShortener{op = #bctx_v1_UrlShortenerOperation{shortened_url = Url}}
+get_owner_id(#ctx_v1_ContextFragment{
+    shortener = #ctx_v1_ContextUrlShortener{op = #ctx_v1_UrlShortenerOperation{shortened_url = Url}}
 }) ->
-    #bctx_v1_ShortenedUrl{owner = #bouncer_base_Entity{id = OwnerID}} = Url,
+    #ctx_v1_ShortenedUrl{owner = #base_Entity{id = OwnerID}} = Url,
     OwnerID.
 
-get_user_id(#bctx_v1_ContextFragment{user = #bctx_v1_User{id = UserID}}) ->
+get_user_id(#ctx_v1_ContextFragment{user = #ctx_v1_User{id = UserID}}) ->
     UserID.
diff --git a/apps/shortener/test/shortener_bouncer_data.hrl b/apps/shortener/test/shortener_bouncer_data.hrl
@@ -1,14 +1,15 @@
 -ifndef(shortener_bouncer_data_included__).
 -define(shortener_bouncer_data_included__, ok).
 
--include_lib("bouncer_proto/include/bouncer_decisions_thrift.hrl").
--include_lib("bouncer_proto/include/bouncer_context_v1_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_decision_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_v1_thrift.hrl").
 
 -define(TEST_USER_REALM, <<"external">>).
 -define(TEST_RULESET_ID, <<"service/authz/api">>).
 
--define(JUDGEMENT(Resolution), #bdcs_Judgement{resolution = Resolution}).
--define(ALLOWED, {allowed, #bdcs_ResolutionAllowed{}}).
--define(FORBIDDEN, {forbidden, #bdcs_ResolutionForbidden{}}).
+-define(JUDGEMENT(Resolution), #decision_Judgement{resolution = Resolution}).
+-define(ALLOWED, {allowed, #decision_ResolutionAllowed{}}).
+-define(FORBIDDEN, {forbidden, #decision_ResolutionForbidden{}}).
 
 -endif.
diff --git a/apps/shortener/test/shortener_ct_helper_bouncer.erl b/apps/shortener/test/shortener_ct_helper_bouncer.erl
@@ -23,7 +23,7 @@ mock_client(SupOrConfig) ->
             [
                 {
                     org_management,
-                    {orgmgmt_auth_context_provider_thrift, 'AuthContextProvider'},
+                    {orgmgmt_authctx_provider_thrift, 'AuthContextProvider'},
                     fun('GetUserContext', {UserID}) ->
                         {encoded_fragment, Fragment} = bouncer_client:bake_context_fragment(
                             bouncer_context_helpers:make_user_fragment(#{
@@ -47,7 +47,7 @@ mock_arbiter(JudgeFun, SupOrConfig) ->
             [
                 {
                     bouncer,
-                    {bouncer_decisions_thrift, 'Arbiter'},
+                    {bouncer_decision_thrift, 'Arbiter'},
                     fun('Judge', {?TEST_RULESET_ID, Context}) ->
                         Fragments = decode_context(Context),
                         Combined = combine_fragments(Fragments),
@@ -59,11 +59,11 @@ mock_arbiter(JudgeFun, SupOrConfig) ->
         )
     ).
 
-decode_context(#bdcs_Context{fragments = Fragments}) ->
+decode_context(#decision_Context{fragments = Fragments}) ->
     maps:map(fun(_, Fragment) -> decode_fragment(Fragment) end, Fragments).
 
-decode_fragment(#bctx_ContextFragment{type = v1_thrift_binary, content = Content}) ->
-    Type = {struct, struct, {bouncer_context_v1_thrift, 'ContextFragment'}},
+decode_fragment(#ctx_ContextFragment{type = v1_thrift_binary, content = Content}) ->
+    Type = {struct, struct, {bouncer_ctx_v1_thrift, 'ContextFragment'}},
     Codec = thrift_strict_binary_codec:new(Content),
     {ok, Fragment, _} = thrift_strict_binary_codec:read(Codec, Type),
     Fragment.
@@ -80,7 +80,7 @@ combine_fragments(Fragments) ->
     [Fragment | Rest] = maps:values(Fragments),
     lists:foldl(fun combine_fragments/2, Fragment, Rest).
 
-combine_fragments(Fragment1 = #bctx_v1_ContextFragment{}, Fragment2 = #bctx_v1_ContextFragment{}) ->
+combine_fragments(Fragment1 = #ctx_v1_ContextFragment{}, Fragment2 = #ctx_v1_ContextFragment{}) ->
     combine_records(Fragment1, Fragment2).
 
 combine_records(Record1, Record2) ->

diff --git a/apps/shortener/test/shortener_ct_helper_token_keeper.erl b/apps/shortener/test/shortener_ct_helper_token_keeper.erl
@@ -1,7 +1,7 @@
 -module(shortener_ct_helper_token_keeper).
 
 -include_lib("token_keeper_proto/include/tk_token_keeper_thrift.hrl").
--include_lib("token_keeper_proto/include/tk_context_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_thrift.hrl").
 -include_lib("shortener_token_keeper_data.hrl").
 
 -define(TOKEN_ID, <<"LETMEIN">>).
@@ -99,13 +99,13 @@ create_bouncer_context(AuthParams, UserParams) ->
 %%
 
 encode_context(Context) ->
-    #bctx_ContextFragment{
+    #ctx_ContextFragment{
         type = v1_thrift_binary,
         content = encode_context_content(Context)
     }.
 
 encode_context_content(Context) ->
-    Type = {struct, struct, {bouncer_context_v1_thrift, 'ContextFragment'}},
+    Type = {struct, struct, {bouncer_ctx_v1_thrift, 'ContextFragment'}},
     Codec = thrift_strict_binary_codec:new(),
     case thrift_strict_binary_codec:write(Codec, Type, Context) of
         {ok, Codec1} ->

diff --git a/apps/shortener/test/shortener_general_SUITE.erl b/apps/shortener/test/shortener_general_SUITE.erl
@@ -1,7 +1,7 @@
 -module(shortener_general_SUITE).
 
--include_lib("bouncer_proto/include/bouncer_decisions_thrift.hrl").
--include_lib("bouncer_proto/include/bouncer_context_v1_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_decision_thrift.hrl").
+-include_lib("bouncer_proto/include/bouncer_ctx_v1_thrift.hrl").
 
 -export([init/1]).
 
@@ -312,7 +312,7 @@ woody_timeout_test(C) ->
     Params = construct_params(SourceUrl),
     {Time, {error, {invalid_response_code, 503}}} =
         timer:tc(fun() ->
-            shorten_url(Params, C2)
+            shorten_url(Params, 30 * 1000, C2)
         end),
     true = (Time >= 3000000),
     genlib_app:stop_unload_applications(Apps).
@@ -343,9 +343,13 @@ set_api_auth_token(Token, C) ->
 %%
 
 shorten_url(ShortenedUrlParams, C) ->
+    shorten_url(ShortenedUrlParams, 5000, C).
+
+shorten_url(ShortenedUrlParams, RecvTimeout, C) ->
     swag_client_ushort_shortener_api:shorten_url(
         ?config(api_endpoint, C),
-        append_common_params(#{body => ShortenedUrlParams}, C)
+        append_common_params(#{body => ShortenedUrlParams}, C),
+        [{recv_timeout, RecvTimeout}]
     ).
 
 delete_shortened_url(ID, C) ->

diff --git a/compose.tracing.yaml b/compose.tracing.yaml
@@ -0,0 +1,27 @@
+services:
+  testrunner:
+    depends_on:
+      jaeger:
+        condition: service_healthy
+    environment:
+      - OTEL_SERVICE_NAME=shortener
+      - OTEL_TRACES_EXPORTER=otlp
+      - OTEL_TRACES_SAMPLER=parentbased_always_on
+      - OTEL_EXPORTER_OTLP_PROTOCOL=http_protobuf
+      - OTEL_EXPORTER_OTLP_ENDPOINT=http://jaeger:4318
+
+  jaeger:
+    image: jaegertracing/all-in-one:1.47
+    environment:
+      - COLLECTOR_OTLP_ENABLED=true
+    healthcheck:
+      test: "/go/bin/all-in-one-linux status"
+      interval: 2s
+      timeout: 1s
+      retries: 20
+    ports:
+      - 4317:4317 # OTLP gRPC receiver
+      - 4318:4318 # OTLP http receiver
+      - 5778:5778
+      - 14250:14250
+      - 16686:16686
diff --git a/docker-compose.yml → compose.yaml b/docker-compose.yml → compose.yaml
@@ -20,15 +20,13 @@ services:
     command: /sbin/init
 
   machinegun:
-    image: docker.io/rbkmoney/machinegun:c05a8c18cd4f7966d70b6ad84cac9429cdfe37ae
-    ports:
-      - "8022"
+    image: ghcr.io/valitydev/machinegun:sha-5c0db56
     command: /opt/machinegun/bin/machinegun foreground
     volumes:
       - ./test/machinegun/config.yaml:/opt/machinegun/etc/config.yaml
       - ./test/machinegun/cookie:/opt/machinegun/etc/cookie
     healthcheck:
-      test: curl http://localhost:8022/health
+      test: "/opt/machinegun/bin/machinegun ping"
       interval: 5s
       timeout: 1s
       retries: 20