From e71c853eb4de30c1807638fa056aaf9cb8f8454c Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 12 Nov 2021 15:20:43 -0700 Subject: [PATCH 1/3] make trace/span id generation customizable A behaviour, otel_id_generator, is added which has callbacks for generating trace and span ids, along with the default versions of those functions. The implementation to use is kept in the tracer record. --- apps/opentelemetry/src/otel_id_generator.erl | 54 +++++++++++++++++++ apps/opentelemetry/src/otel_span_ets.erl | 11 ++-- apps/opentelemetry/src/otel_span_utils.erl | 30 +++++------ apps/opentelemetry/src/otel_tracer.hrl | 1 + .../opentelemetry/src/otel_tracer_default.erl | 3 +- apps/opentelemetry/src/otel_tracer_server.erl | 3 ++ .../test/otel_samplers_SUITE.erl | 4 +- apps/opentelemetry_api/src/opentelemetry.erl | 25 +-------- .../src/otel_tracer_noop.erl | 14 ++--- .../test/otel_propagators_SUITE.erl | 21 -------- .../test/opentelemetry_exporter_SUITE.erl | 16 +++--- 11 files changed, 96 insertions(+), 86 deletions(-) create mode 100644 apps/opentelemetry/src/otel_id_generator.erl diff --git a/apps/opentelemetry/src/otel_id_generator.erl b/apps/opentelemetry/src/otel_id_generator.erl new file mode 100644 index 00000000..4ce0e518 --- /dev/null +++ b/apps/opentelemetry/src/otel_id_generator.erl @@ -0,0 +1,54 @@ +%%%------------------------------------------------------------------------ +%% Copyright 2021, OpenTelemetry Authors +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% @doc This module provides the behaviour to implement for custom trace +%% and span id generation and the default implementation of the +%% generators which produces random 128 bit and 64 bit integers for the +%% trace id and span id. +%% @end +%%%----------------------------------------------------------------------- +-module(otel_id_generator). + +-export([generate_trace_id/0, + generate_trace_id/1, + generate_span_id/0, + generate_span_id/1]). + +-callback generate_trace_id() -> opentelemetry:trace_id(). + +-callback generate_span_id() -> opentelemetry:span_id(). + +-type t() :: module(). + +-export_type([t/0]). + +%% @doc Calls a module implementing the `otel_id_generator' behaviour to generate a trace id +-spec generate_trace_id(t()) -> opentelemetry:trace_id(). +generate_trace_id(Module) -> + Module:generate_trace_id(). + +%% @doc Calls a module implementing the `otel_id_generator' behaviour to generate a span id +-spec generate_span_id(t()) -> opentelemetry:span_id(). +generate_span_id(Module) -> + Module:generate_span_id(). + +%% @doc Generates a 128 bit random integer to use as a trace id. +-spec generate_trace_id() -> opentelemetry:trace_id(). +generate_trace_id() -> + rand:uniform(2 bsl 127 - 1). %% 2 shifted left by 127 == 2 ^ 128 + +%% @doc Generates a 64 bit random integer to use as a span id. +-spec generate_span_id() -> opentelemetry:span_id(). +generate_span_id() -> + rand:uniform(2 bsl 63 - 1). %% 2 shifted left by 63 == 2 ^ 64 diff --git a/apps/opentelemetry/src/otel_span_ets.erl b/apps/opentelemetry/src/otel_span_ets.erl index a39a81fc..078e9206 100644 --- a/apps/opentelemetry/src/otel_span_ets.erl +++ b/apps/opentelemetry/src/otel_span_ets.erl @@ -25,7 +25,7 @@ handle_call/3, handle_cast/2]). --export([start_span/6, +-export([start_span/7, end_span/1, end_span/2, get_ctx/1, @@ -46,10 +46,11 @@ start_link(Opts) -> gen_server:start_link(?MODULE, Opts, []). %% @doc Start a span and insert into the active span ets table. --spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_sampler:t(), otel_span:start_opts(), - fun(), otel_tracer_server:instrumentation_library()) -> opentelemetry:span_ctx(). -start_span(Ctx, Name, Sampler, Opts, Processors, InstrumentationLibrary) -> - case otel_span_utils:start_span(Ctx, Name, Sampler, Opts) of +-spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_sampler:t(), otel_id_generator:t(), + otel_span:start_opts(), fun(), otel_tracer_server:instrumentation_library()) + -> opentelemetry:span_ctx(). +start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationLibrary) -> + case otel_span_utils:start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts) of {SpanCtx=#span_ctx{is_recording=true}, Span=#span{}} -> Span1 = Span#span{instrumentation_library=InstrumentationLibrary}, Span2 = Processors(Ctx, Span1), diff --git a/apps/opentelemetry/src/otel_span_utils.erl b/apps/opentelemetry/src/otel_span_utils.erl index 9b3b1888..294b327b 100644 --- a/apps/opentelemetry/src/otel_span_utils.erl +++ b/apps/opentelemetry/src/otel_span_utils.erl @@ -18,7 +18,7 @@ %%%------------------------------------------------------------------------- -module(otel_span_utils). --export([start_span/4, +-export([start_span/5, end_span/1, end_span/2]). @@ -26,17 +26,17 @@ -include("otel_sampler.hrl"). -include("otel_span.hrl"). --spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_sampler:t(), otel_span:start_opts()) - -> {opentelemetry:span_ctx(), opentelemetry:span() | undefined}. -start_span(Ctx, Name, Sampler, Opts) -> +-spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_sampler:t(), otel_id_generator:t(), + otel_span:start_opts()) -> {opentelemetry:span_ctx(), opentelemetry:span() | undefined}. +start_span(Ctx, Name, Sampler, IdGenerator, Opts) -> Attributes = maps:get(attributes, Opts, []), Links = maps:get(links, Opts, []), Kind = maps:get(kind, Opts, ?SPAN_KIND_INTERNAL), StartTime = maps:get(start_time, Opts, opentelemetry:timestamp()), - new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links). + new_span(Ctx, Name, Sampler, IdGenerator, StartTime, Kind, Attributes, Links). -new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links) -> - {NewSpanCtx, ParentSpanId} = new_span_ctx(Ctx), +new_span(Ctx, Name, Sampler, IdGeneratorModule, StartTime, Kind, Attributes, Links) -> + {NewSpanCtx, ParentSpanId} = new_span_ctx(Ctx, IdGeneratorModule), TraceId = NewSpanCtx#span_ctx.trace_id, SpanId = NewSpanCtx#span_ctx.span_id, @@ -60,21 +60,21 @@ new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links) -> is_valid=true, is_recording=IsRecording}, Span}. --spec new_span_ctx(otel_ctx:t()) -> {opentelemetry:span_ctx(), opentelemetry:span_id()}. -new_span_ctx(Ctx) -> +-spec new_span_ctx(otel_ctx:t(), otel_id_generator:t()) -> {opentelemetry:span_ctx(), opentelemetry:span_id()}. +new_span_ctx(Ctx, IdGeneratorModule) -> case otel_tracer:current_span_ctx(Ctx) of undefined -> - {root_span_ctx(), undefined}; + {root_span_ctx(IdGeneratorModule), undefined}; #span_ctx{is_valid=false} -> - {root_span_ctx(), undefined}; + {root_span_ctx(IdGeneratorModule), undefined}; ParentSpanCtx=#span_ctx{span_id=ParentSpanId} -> %% keep the rest of the parent span ctx, simply need to update the span_id - {ParentSpanCtx#span_ctx{span_id=opentelemetry:generate_span_id()}, ParentSpanId} + {ParentSpanCtx#span_ctx{span_id=IdGeneratorModule:generate_span_id()}, ParentSpanId} end. -root_span_ctx() -> - #span_ctx{trace_id=opentelemetry:generate_trace_id(), - span_id=opentelemetry:generate_span_id(), +root_span_ctx(IdGeneratorModule) -> + #span_ctx{trace_id=IdGeneratorModule:generate_trace_id(), + span_id=IdGeneratorModule:generate_span_id(), is_valid=true, trace_flags=0}. diff --git a/apps/opentelemetry/src/otel_tracer.hrl b/apps/opentelemetry/src/otel_tracer.hrl index 7e00392b..0147db1d 100644 --- a/apps/opentelemetry/src/otel_tracer.hrl +++ b/apps/opentelemetry/src/otel_tracer.hrl @@ -9,6 +9,7 @@ on_start_processors :: fun((otel_ctx:t(), opentelemetry:span()) -> opentelemetry:span()), on_end_processors :: fun((opentelemetry:span()) -> boolean() | {error, term()}), sampler :: otel_sampler:t(), + id_generator :: otel_id_generator:t(), instrumentation_library :: otel_tracer_server:instrumentation_library() | undefined, telemetry_library :: otel_tracer_server:telemetry_library() | undefined, resource :: otel_resource:t() | undefined diff --git a/apps/opentelemetry/src/otel_tracer_default.erl b/apps/opentelemetry/src/otel_tracer_default.erl index 4bc9e502..914e3f1d 100644 --- a/apps/opentelemetry/src/otel_tracer_default.erl +++ b/apps/opentelemetry/src/otel_tracer_default.erl @@ -33,8 +33,9 @@ start_span(Ctx, {_, #tracer{on_start_processors=Processors, on_end_processors=OnEndProcessors, sampler=Sampler, + id_generator=IdGeneratorModule, instrumentation_library=InstrumentationLibrary}}, Name, Opts) -> - SpanCtx = otel_span_ets:start_span(Ctx, Name, Sampler, Opts, Processors, InstrumentationLibrary), + SpanCtx = otel_span_ets:start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationLibrary), SpanCtx#span_ctx{span_sdk={otel_span_ets, OnEndProcessors}}. -spec with_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), diff --git a/apps/opentelemetry/src/otel_tracer_server.erl b/apps/opentelemetry/src/otel_tracer_server.erl index 45ed2859..3538983c 100644 --- a/apps/opentelemetry/src/otel_tracer_server.erl +++ b/apps/opentelemetry/src/otel_tracer_server.erl @@ -62,6 +62,8 @@ start_link(Opts) -> init(Opts) -> Resource = otel_resource_detector:get_resource(), + IdGeneratorModule = proplists:get_value(id_generator, Opts, otel_id_generator), + SamplerSpec = proplists:get_value( sampler, Opts, {parent_based, #{root => always_on}} ), @@ -80,6 +82,7 @@ init(Opts) -> sampler=Sampler, on_start_processors=on_start(Processors), on_end_processors=on_end(Processors), + id_generator=IdGeneratorModule, resource=Resource, telemetry_library=TelemetryLibrary}, opentelemetry:set_default_tracer({otel_tracer_default, Tracer}), diff --git a/apps/opentelemetry/test/otel_samplers_SUITE.erl b/apps/opentelemetry/test/otel_samplers_SUITE.erl index 454bf3ff..f8bdcc1e 100644 --- a/apps/opentelemetry/test/otel_samplers_SUITE.erl +++ b/apps/opentelemetry/test/otel_samplers_SUITE.erl @@ -271,7 +271,7 @@ custom_sampler_module(_Config) -> {?DROP, [], []}, Sampler:should_sample( otel_ctx:new(), - opentelemetry:generate_trace_id(), + otel_id_generator:generate_trace_id(), [], SpanName, undefined, @@ -288,7 +288,7 @@ should_sample(_Config) -> otel_samplers:should_sample( Sampler, otel_ctx:new(), - opentelemetry:generate_trace_id(), + otel_id_generator:generate_trace_id(), [], <<"span-name">>, undefined, diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index 239443c1..8b03289a 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -52,9 +52,7 @@ event/3, events/1, status/2, - verify_and_set_term/3, - generate_trace_id/0, - generate_span_id/0]). + verify_and_set_term/3]). -include("opentelemetry.hrl"). -include_lib("kernel/include/logger.hrl"). @@ -349,27 +347,6 @@ status(?OTEL_STATUS_UNSET, _Message) -> status(_, _) -> undefined. -%%-------------------------------------------------------------------- -%% @doc -%% Generates a 128 bit random integer to use as a trace id. -%% @end -%%-------------------------------------------------------------------- --spec generate_trace_id() -> trace_id(). -generate_trace_id() -> - uniform(2 bsl 127 - 1). %% 2 shifted left by 127 == 2 ^ 128 - -%%-------------------------------------------------------------------- -%% @doc -%% Generates a 64 bit random integer to use as a span id. -%% @end -%%-------------------------------------------------------------------- --spec generate_span_id() -> span_id(). -generate_span_id() -> - uniform(2 bsl 63 - 1). %% 2 shifted left by 63 == 2 ^ 64 - -uniform(X) -> - rand:uniform(X). - %% internal functions -spec verify_and_set_term(module() | {module(), term()}, term(), atom()) -> boolean(). diff --git a/apps/opentelemetry_api/src/otel_tracer_noop.erl b/apps/opentelemetry_api/src/otel_tracer_noop.erl index 671293d2..5b27462b 100644 --- a/apps/opentelemetry_api/src/otel_tracer_noop.erl +++ b/apps/opentelemetry_api/src/otel_tracer_noop.erl @@ -30,6 +30,7 @@ update_name/3]). -include("opentelemetry.hrl"). +-include("otel_tracer.hrl"). -define(NOOP_SPAN_CTX, #span_ctx{trace_id=0, span_id=0, @@ -40,20 +41,13 @@ span_sdk=undefined}). -define(NOOP_TRACER_CTX, []). --spec start_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts()) - -> opentelemetry:span_ctx(). +-spec start_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), + otel_span:start_opts()) -> opentelemetry:span_ctx(). start_span(Ctx, _, _SpanName, _) -> %% Spec: Behavior of the API in the absence of an installed SDK case otel_tracer:current_span_ctx(Ctx) of - Parent=#span_ctx{trace_id=TraceId, - is_recording=false} when TraceId =/= 0 -> - %% if the parent is a non-recording Span it MAY return the parent Span - Parent; Parent=#span_ctx{trace_id=TraceId} when TraceId =/= 0 -> - %% API MUST create a non-recording Span with the SpanContext in the parent Context - SpanCtx = Parent#span_ctx{span_id=opentelemetry:generate_span_id(), - is_recording=false}, - SpanCtx; + Parent; _ -> %% If the parent Context contains no valid Span, %% an empty non-recording Span MUST be returned diff --git a/apps/opentelemetry_api/test/otel_propagators_SUITE.erl b/apps/opentelemetry_api/test/otel_propagators_SUITE.erl index 9e8ff764..3cd27773 100644 --- a/apps/opentelemetry_api/test/otel_propagators_SUITE.erl +++ b/apps/opentelemetry_api/test/otel_propagators_SUITE.erl @@ -22,7 +22,6 @@ groups() -> %% Tests of Behavior of the API in the absence of an installed SDK %% https://github.com/open-telemetry/opentelemetry-specification/blob/82865fae64e7b30ee59906d7c4d25a48fe446563/specification/trace/api.md#behavior-of-the-api-in-the-absence-of-an-installed-sdk [{absence_of_an_installed_sdk, [shuffle, parallel], [invalid_span_no_sdk_propagation, - recording_no_sdk_propagation, nonrecording_no_sdk_propagation]}]. init_per_suite(Config) -> @@ -100,26 +99,6 @@ invalid_span_no_sdk_propagation(_Config) -> ok. -recording_no_sdk_propagation(_Config) -> - ct:comment("Test that a start_span called with an valid recording span parent " - "and no SDK results in a new span_id for the child"), - otel_ctx:clear(), - - RecordingSpanCtx = #span_ctx{trace_id=21267647932558653966460912964485513216, - span_id=1152921504606846976, - is_recording=true}, - otel_tracer:set_current_span(RecordingSpanCtx), - ?assertEqual(RecordingSpanCtx, otel_tracer:current_span_ctx()), - ?with_span(<<"span-1">>, #{}, fun(_) -> - %% parent is recording so a new span_id should be used - ?assertNotEqual(RecordingSpanCtx, otel_tracer:current_span_ctx()) - end), - ?assertEqual(RecordingSpanCtx, otel_tracer:current_span_ctx()), - BinaryHeaders = otel_propagator_text_map:inject([]), - ?assertMatch(?EXPECTED_HEADERS, BinaryHeaders), - - ok. - nonrecording_no_sdk_propagation(_Config) -> ct:comment("Test that a start_span called with an valid non-recording span parent " "and no SDK results in the same span_ctx as the child"), diff --git a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl index 30d405c3..12e1f4c5 100644 --- a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl +++ b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl @@ -114,8 +114,8 @@ configuration(_Config) -> ets_instrumentation_info(_Config) -> Tid = ets:new(span_tab, [duplicate_bag, {keypos, #span.instrumentation_library}]), - TraceId = opentelemetry:generate_trace_id(), - SpanId = opentelemetry:generate_span_id(), + TraceId = otel_id_generator:generate_trace_id(), + SpanId = otel_id_generator:generate_span_id(), ParentSpan = #span{name = <<"span-1">>, @@ -137,7 +137,7 @@ ets_instrumentation_info(_Config) -> ChildSpan = #span{name = <<"span-2">>, trace_id = TraceId, - span_id = opentelemetry:generate_span_id(), + span_id = otel_id_generator:generate_span_id(), parent_span_id = SpanId, kind = ?SPAN_KIND_SERVER, start_time = opentelemetry:timestamp(), @@ -165,8 +165,8 @@ ets_instrumentation_info(_Config) -> ok. span_round_trip(_Config) -> - TraceId = opentelemetry:generate_trace_id(), - SpanId = opentelemetry:generate_span_id(), + TraceId = otel_id_generator:generate_trace_id(), + SpanId = otel_id_generator:generate_span_id(), Span = #span{name = <<"span-1">>, @@ -219,8 +219,8 @@ verify_export(Config) -> ?assertMatch(ok, opentelemetry_exporter:export(Tid, otel_resource:create([]), State)), - TraceId = opentelemetry:generate_trace_id(), - SpanId = opentelemetry:generate_span_id(), + TraceId = otel_id_generator:generate_trace_id(), + SpanId = otel_id_generator:generate_span_id(), ParentSpan = #span{name = <<"span-1">>, @@ -242,7 +242,7 @@ verify_export(Config) -> ChildSpan = #span{name = <<"span-2">>, trace_id = TraceId, - span_id = opentelemetry:generate_span_id(), + span_id = otel_id_generator:generate_span_id(), parent_span_id = SpanId, kind = ?SPAN_KIND_SERVER, start_time = opentelemetry:timestamp(), From 6075c25323f1f6fa299a7571dd2f96b8adc3ca41 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sat, 13 Nov 2021 11:37:24 -0700 Subject: [PATCH 2/3] add junit/surefire action to view failures in PR --- .github/workflows/erlang.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/erlang.yml b/.github/workflows/erlang.yml index 982b44d9..656942d7 100644 --- a/.github/workflows/erlang.yml +++ b/.github/workflows/erlang.yml @@ -47,6 +47,13 @@ jobs: run: rebar3 eunit --cover - name: Common Test tests run: rebar3 ct --cover + + - name: Publish Test Report + uses: mikepenz/action-junit-report@v2 + if: always() # always run even if the previous step fails + with: + report_paths: '_build/test/logs/*/junit_report.xml' + - name: XRef run: rebar3 xref - name: Covertool From fa8ae84110eb49af2756e0960962eb301af3dd39 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sat, 13 Nov 2021 11:46:50 -0700 Subject: [PATCH 3/3] pin otel collecto image for tests --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index c7a0b4db..6114658e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ version: "3" services: otel: - image: otel/opentelemetry-collector-contrib-dev + image: otel/opentelemetry-collector-contrib-dev:954d0bbc8e80c88380a801443940c7c91779f928 command: ["--config=/conf/otel-collector-config.yaml"] privileged: true ports: