From c30748c0625b5576e10b7ae162f4eaca95e872f8 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 20 Oct 2021 17:53:49 -0600 Subject: [PATCH] remove sampler option to start_span and with_span --- CHANGELOG.md | 6 +++++ apps/opentelemetry/src/otel_span_ets.erl | 8 +++---- apps/opentelemetry/src/otel_span_utils.erl | 7 +++--- .../opentelemetry/src/otel_tracer_default.erl | 15 +++++-------- .../test/opentelemetry_SUITE.erl | 22 +++++++++++-------- .../lib/open_telemetry/tracer.ex | 2 -- apps/opentelemetry_api/src/otel_span.erl | 1 - .../test/open_telemetry_test.exs | 1 - test/otel_tests.exs | 9 ++++++-- 9 files changed, 38 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d252b38c2..c7d3a946a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/apps/opentelemetry/src/otel_span_ets.erl b/apps/opentelemetry/src/otel_span_ets.erl index 144c42927..a39a81fc3 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/5, +-export([start_span/6, end_span/1, end_span/2, get_ctx/1, @@ -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), diff --git a/apps/opentelemetry/src/otel_span_utils.erl b/apps/opentelemetry/src/otel_span_utils.erl index 915e2bca0..9b3b18887 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/3, +-export([start_span/4, end_span/1, end_span/2]). @@ -26,13 +26,12 @@ -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). diff --git a/apps/opentelemetry/src/otel_tracer_default.erl b/apps/opentelemetry/src/otel_tracer_default.erl index 9ae3fb138..4bc9e5023 100644 --- a/apps/opentelemetry/src/otel_tracer_default.erl +++ b/apps/opentelemetry/src/otel_tracer_default.erl @@ -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) -> diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 19b420681..fb1e16c54 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -438,13 +438,14 @@ 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">> @@ -452,11 +453,12 @@ non_recording_ets_table(_Config) -> 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), @@ -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), @@ -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), diff --git a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex index 5ab61438a..529d450a3 100644 --- a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex +++ b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex @@ -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(), diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index b90410528..9388db0af 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -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(), diff --git a/apps/opentelemetry_api/test/open_telemetry_test.exs b/apps/opentelemetry_api/test/open_telemetry_test.exs index ed13dc974..ad4c2df27 100644 --- a/apps/opentelemetry_api/test/open_telemetry_test.exs +++ b/apps/opentelemetry_api/test/open_telemetry_test.exs @@ -62,7 +62,6 @@ defmodule OpenTelemetryTest do kind: :internal, links: [], start_time: :opentelemetry.timestamp() - # sampler } do :ok end diff --git a/test/otel_tests.exs b/test/otel_tests.exs index d697b07a9..cf98f8076 100644 --- a/test/otel_tests.exs +++ b/test/otel_tests.exs @@ -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) @@ -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