Skip to content

Commit

Permalink
remove sampler option to start_span and with_span
Browse files Browse the repository at this point in the history
  • Loading branch information
Tristan Sloughter committed Oct 21, 2021
1 parent fab724a commit c30748c
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 33 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Removed

- The `sampler` option to `start_span` and `with_span` was removed.

## [API 1.0.0-rc.3.2] - 2021-10-13

##### Fixed
Expand Down
8 changes: 4 additions & 4 deletions apps/opentelemetry/src/otel_span_ets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
handle_call/3,
handle_cast/2]).

-export([start_span/5,
-export([start_span/6,
end_span/1,
end_span/2,
get_ctx/1,
Expand All @@ -46,10 +46,10 @@ 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_span:start_opts(),
-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, Opts, Processors, InstrumentationLibrary) ->
case otel_span_utils:start_span(Ctx, Name, Opts) of
start_span(Ctx, Name, Sampler, Opts, Processors, InstrumentationLibrary) ->
case otel_span_utils:start_span(Ctx, Name, Sampler, Opts) of
{SpanCtx=#span_ctx{is_recording=true}, Span=#span{}} ->
Span1 = Span#span{instrumentation_library=InstrumentationLibrary},
Span2 = Processors(Ctx, Span1),
Expand Down
7 changes: 3 additions & 4 deletions apps/opentelemetry/src/otel_span_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,20 @@
%%%-------------------------------------------------------------------------
-module(otel_span_utils).

-export([start_span/3,
-export([start_span/4,
end_span/1,
end_span/2]).

-include_lib("opentelemetry_api/include/opentelemetry.hrl").
-include("otel_sampler.hrl").
-include("otel_span.hrl").

-spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_span:start_opts())
-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, Opts) ->
start_span(Ctx, Name, Sampler, Opts) ->
Attributes = maps:get(attributes, Opts, []),
Links = maps:get(links, Opts, []),
Kind = maps:get(kind, Opts, ?SPAN_KIND_INTERNAL),
Sampler = maps:get(sampler, Opts),
StartTime = maps:get(start_time, Opts, opentelemetry:timestamp()),
new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links).

Expand Down
15 changes: 5 additions & 10 deletions apps/opentelemetry/src/otel_tracer_default.erl
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,13 @@
%% @doc Starts an inactive Span and returns its SpanCtx.
-spec start_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(),
otel_span:start_opts()) -> opentelemetry:span_ctx().
start_span(Ctx, Tracer={_, #tracer{on_start_processors=Processors,
on_end_processors=OnEndProcessors,
instrumentation_library=InstrumentationLibrary}}, Name, Opts) ->
Opts1 = maybe_set_sampler(Tracer, Opts),
SpanCtx = otel_span_ets:start_span(Ctx, Name, Opts1, Processors, InstrumentationLibrary),
start_span(Ctx, {_, #tracer{on_start_processors=Processors,
on_end_processors=OnEndProcessors,
sampler=Sampler,
instrumentation_library=InstrumentationLibrary}}, Name, Opts) ->
SpanCtx = otel_span_ets:start_span(Ctx, Name, Sampler, Opts, Processors, InstrumentationLibrary),
SpanCtx#span_ctx{span_sdk={otel_span_ets, OnEndProcessors}}.

maybe_set_sampler(_Tracer, Opts) when is_map_key(sampler, Opts) ->
Opts;
maybe_set_sampler({_, #tracer{sampler=Sampler}}, Opts) ->
Opts#{sampler => Sampler}.

-spec with_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(),
otel_span:start_opts(), otel_tracer:traced_fun(T)) -> T.
with_span(Ctx, Tracer, SpanName, Opts, Fun) ->
Expand Down
22 changes: 13 additions & 9 deletions apps/opentelemetry/test/opentelemetry_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -438,25 +438,27 @@ default_sampler(_Config) ->
ok.

non_recording_ets_table(_Config) ->
Tracer = opentelemetry:get_tracer(),
Tracer={TracerModule, TracerConfig} = opentelemetry:get_tracer(),

SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{}),
?assertMatch(true, SpanCtx1#span_ctx.is_recording),

AlwaysOff = otel_sampler:new(always_off),
SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{sampler => AlwaysOff}),
Tracer1 = {TracerModule, TracerConfig#tracer{sampler=AlwaysOff}},
SpanCtx2 = otel_tracer:start_span(Tracer1, <<"span-2">>, #{}),
?assertMatch(false, SpanCtx2#span_ctx.is_recording),

%% verify that ETS table only contains the recording span <<"span-1">>
?assertMatch([#span{name = <<"span-1">>}], ets:tab2list(?SPAN_TAB)),
ok.

root_span_sampling_always_off(_Config) ->
Tracer = opentelemetry:get_tracer(),
Tracer={TracerModule, TracerConfig} = opentelemetry:get_tracer(),

Sampler = otel_sampler:new(always_off),
Tracer1 = {TracerModule, TracerConfig#tracer{sampler=Sampler}},

SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}),
SpanCtx1 = otel_tracer:start_span(Tracer1, <<"span-1">>, #{}),
?assertMatch(false, SpanCtx1#span_ctx.is_recording),
?assertMatch(0, SpanCtx1#span_ctx.trace_flags),

Expand All @@ -468,11 +470,12 @@ root_span_sampling_always_off(_Config) ->
ok.

root_span_sampling_always_on(_Config) ->
Tracer = opentelemetry:get_tracer(),
Tracer={TracerModule, TracerConfig} = opentelemetry:get_tracer(),

Sampler = otel_sampler:new(always_on),
Tracer1 = {TracerModule, TracerConfig#tracer{sampler=Sampler}},

SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}),
SpanCtx1 = otel_tracer:start_span(Tracer1, <<"span-1">>, #{}),
?assertMatch(true, SpanCtx1#span_ctx.is_recording),
?assertMatch(1, SpanCtx1#span_ctx.trace_flags),

Expand All @@ -491,16 +494,17 @@ record_but_not_sample(Config) ->
Sampler = otel_sampler:new({static_sampler, #{<<"span-record-and-sample">> => ?RECORD_AND_SAMPLE,
<<"span-record">> => ?RECORD_ONLY}}),

Tracer = opentelemetry:get_tracer(),
{Module, Tracer0} = opentelemetry:get_tracer(),
Tracer = {Module, Tracer0#tracer{sampler=Sampler}},

SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-record-and-sample">>, #{sampler => Sampler}),
SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-record-and-sample">>, #{}),
?assertEqual(true, SpanCtx1#span_ctx.is_recording),
?assertEqual(1, SpanCtx1#span_ctx.trace_flags),

?set_current_span(SpanCtx1),
?assertMatch(SpanCtx1, ?current_span_ctx),

SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-record">>, #{sampler => Sampler}),
SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-record">>, #{}),
?assertEqual(true, SpanCtx2#span_ctx.is_recording),
?assertEqual(0, SpanCtx2#span_ctx.trace_flags),

Expand Down
2 changes: 0 additions & 2 deletions apps/opentelemetry_api/lib/open_telemetry/tracer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ defmodule OpenTelemetry.Tracer do

@type start_opts() :: %{
optional(:attributes) => OpenTelemetry.attributes(),
# TODO sampler should is an opaque type defined in the implementation
optional(:sampler) => term(),
optional(:links) => OpenTelemetry.links(),
optional(:is_recording) => boolean(),
optional(:start_time) => :opentelemetry.timestamp(),
Expand Down
1 change: 0 additions & 1 deletion apps/opentelemetry_api/src/otel_span.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
-define(is_recording(SpanCtx), SpanCtx =/= undefined andalso SpanCtx#span_ctx.is_recording =:= true).

-type start_opts() :: #{attributes => opentelemetry:attributes(),
sampler => term(),
links => opentelemetry:links(),
is_recording => boolean(),
start_time => opentelemetry:timestamp(),
Expand Down
1 change: 0 additions & 1 deletion apps/opentelemetry_api/test/open_telemetry_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ defmodule OpenTelemetryTest do
kind: :internal,
links: [],
start_time: :opentelemetry.timestamp()
# sampler
} do
:ok
end
Expand Down
9 changes: 7 additions & 2 deletions test/otel_tests.exs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ defmodule OtelTests do
@fields Record.extract(:span, from_lib: "opentelemetry/include/otel_span.hrl")
Record.defrecordp(:span, @fields)

@fields Record.extract(:tracer, from_lib: "opentelemetry/src/otel_tracer.hrl")
Record.defrecordp(:tracer, @fields)

@fields Record.extract(:span_ctx, from_lib: "opentelemetry_api/include/opentelemetry.hrl")
Record.defrecordp(:span_ctx, @fields)

Expand All @@ -34,11 +37,13 @@ defmodule OtelTests do
test "child span should not be sampled if root span is not sampled" do
:otel_batch_processor.set_exporter(:otel_exporter_pid, self())
OpenTelemetry.register_tracer(:test_tracer, "0.1.0")
t={module, t_config} = :opentelemetry.get_tracer()

sampler = :otel_sampler.new(:always_off)
t_always_off = {module, tracer(t_config, sampler: sampler)}

Tracer.with_span "span-1", %{sampler: sampler} do
Tracer.with_span "span-2" do
:otel_tracer.with_span t_always_off, "span-1", %{}, fn(_) ->
:otel_tracer.with_span t, "span-2", %{}, fn(_) ->
Tracer.set_attribute("foo", "bar")
end
end
Expand Down

0 comments on commit c30748c

Please sign in to comment.