Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace InstrumentationLibrary with InstrumentationScope #405

Merged
merged 3 commits into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,43 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [SDK]
### [API]

#### Added

- [Instrumentation Scope replaces Instrumentation
Library](https://github.com/open-telemetry/opentelemetry-erlang/pull/405) --
If you were using the record directly, please use the function
`opentelemetry:instrumentation_scope/3` or
`opentelemetry:instrumentation_library/3` to create an `instrumentation_scope`
record.

### [SDK]

#### Added

- [Instrumentation Scope replaces Instrumentation
Library](https://github.com/open-telemetry/opentelemetry-erlang/pull/405) --
If you were using the record directly, please use the function
`opentelemetry:instrumentation_scope/3` or
`opentelemetry:instrumentation_library/3` to create an `instrumentation_scope`
record.

### Fixed

- [Allow custom text propagator to be configured via application env](https://github.com/open-telemetry/opentelemetry-erlang/pull/408)

### [Exporter]

#### Added

- [Instrumentation Scope replaces Instrumentation
Library](https://github.com/open-telemetry/opentelemetry-erlang/pull/405) --
If you were using the record directly, please use the function
`opentelemetry:instrumentation_scope/3` or
`opentelemetry:instrumentation_library/3` to create an `instrumentation_scope`
record.

## SDK 1.0.5 - 2022-05-20

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion apps/opentelemetry/include/otel_span.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
%% trace flags lowest bit is 1 but simply not propagated.
is_recording :: boolean() | undefined,

instrumentation_library :: opentelemetry:instrumentation_library() | undefined
instrumentation_scope :: opentelemetry:instrumentation_scope() | undefined
}).

-record(span_limits, {
Expand Down
6 changes: 3 additions & 3 deletions apps/opentelemetry/src/otel_batch_processor.erl
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,10 @@ new_export_table(Name) ->
{write_concurrency, true},
duplicate_bag,
%% OpenTelemetry exporter protos group by the
%% instrumentation_library. So using instrumentation_library
%% instrumentation_scope. So using instrumentation_scope
%% as the key means we can easily lookup all spans for
%% for each instrumentation_library and export together.
{keypos, #span.instrumentation_library}]).
%% for each instrumentation_scope and export together.
{keypos, #span.instrumentation_scope}]).

export_spans(#data{exporter=Exporter,
resource=Resource}) ->
Expand Down
6 changes: 3 additions & 3 deletions apps/opentelemetry/src/otel_simple_processor.erl
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ new_export_table(Name) ->
{write_concurrency, true},
duplicate_bag,
%% OpenTelemetry exporter protos group by the
%% instrumentation_library. So using instrumentation_library
%% instrumentation_scope. So using instrumentation_scope
%% as the key means we can easily lookup all spans for
%% for each instrumentation_library and export together.
{keypos, #span.instrumentation_library}]).
%% for each instrumentation_scope and export together.
{keypos, #span.instrumentation_scope}]).

export_span(Span, #data{exporter=Exporter,
resource=Resource}) ->
Expand Down
6 changes: 3 additions & 3 deletions apps/opentelemetry/src/otel_span_ets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ start_link(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_id_generator:t(),
otel_span:start_opts(), fun(), otel_tracer_server:instrumentation_library())
otel_span:start_opts(), fun(), otel_tracer_server:instrumentation_scope())
-> opentelemetry:span_ctx().
start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationLibrary) ->
start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationScope) ->
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},
Span1 = Span#span{instrumentation_scope=InstrumentationScope},
Span2 = Processors(Ctx, Span1),
_ = storage_insert(Span2),
SpanCtx;
Expand Down
2 changes: 1 addition & 1 deletion apps/opentelemetry/src/otel_tracer.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
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,
instrumentation_scope :: otel_tracer_server:instrumentation_scope() | undefined,
telemetry_library :: otel_tracer_server:telemetry_library() | undefined,
resource :: otel_resource:t() | undefined
}).
Expand Down
4 changes: 2 additions & 2 deletions apps/opentelemetry/src/otel_tracer_default.erl
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ 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, IdGeneratorModule, Opts, Processors, InstrumentationLibrary),
instrumentation_scope=InstrumentationScope}}, Name, Opts) ->
SpanCtx = otel_span_ets:start_span(Ctx, Name, Sampler, IdGeneratorModule, Opts, Processors, InstrumentationScope),
SpanCtx#span_ctx{span_sdk={otel_span_ets, OnEndProcessors}}.

-spec with_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(),
Expand Down
12 changes: 6 additions & 6 deletions apps/opentelemetry/src/otel_tracer_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@
-include("otel_span.hrl").

-type telemetry_library() :: #telemetry_library{}.
-type instrumentation_library() :: #instrumentation_library{}.
-type instrumentation_scope() :: #instrumentation_scope{}.
-export_type([telemetry_library/0,
instrumentation_library/0]).
instrumentation_scope/0]).

-record(state,
{
Expand Down Expand Up @@ -100,15 +100,15 @@ handle_call({get_tracer, Name, Vsn, SchemaUrl}, _From, State=#state{shared_trace
true ->
{reply, {otel_tracer_noop, []}, State};
false ->
InstrumentationLibrary = opentelemetry:instrumentation_library(Name, Vsn, SchemaUrl),
InstrumentationScope = opentelemetry:instrumentation_scope(Name, Vsn, SchemaUrl),
TracerTuple = {Tracer#tracer.module,
Tracer#tracer{instrumentation_library=InstrumentationLibrary}},
Tracer#tracer{instrumentation_scope=InstrumentationScope}},
{reply, TracerTuple, State}
end;
handle_call({get_tracer, InstrumentationLibrary}, _From, State=#state{shared_tracer=Tracer,
handle_call({get_tracer, InstrumentationScope}, _From, State=#state{shared_tracer=Tracer,
deny_list=_DenyList}) ->
{reply, {Tracer#tracer.module,
Tracer#tracer{instrumentation_library=InstrumentationLibrary}}, State};
Tracer#tracer{instrumentation_scope=InstrumentationScope}}, State};
handle_call(force_flush, _From, State=#state{processors=Processors}) ->
Reply = lists:foldl(fun(Processor, Result) ->
case force_flush_(Processor) of
Expand Down
38 changes: 19 additions & 19 deletions apps/opentelemetry/test/opentelemetry_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ all() ->

all_cases() ->
[with_span, macros, child_spans,
update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app,
update_span_data, tracer_instrumentation_scope, tracer_previous_ctx, stop_temporary_app,
reset_after, attach_ctx, default_sampler, non_recording_ets_table,
root_span_sampling_always_on, root_span_sampling_always_off,
record_but_not_sample, record_exception_works, record_exception_with_message_works,
Expand Down Expand Up @@ -103,7 +103,7 @@ init_per_testcase(too_many_attributes, Config) ->
Tid = ets:new(exported_spans, [public, bag]),
otel_batch_processor:set_exporter(otel_exporter_tab, Tid),
[{tid, Tid} | Config];
init_per_testcase(tracer_instrumentation_library, Config) ->
init_per_testcase(tracer_instrumentation_scope, Config) ->
application:set_env(opentelemetry, processors, [{otel_batch_processor, #{scheduled_delay_ms => 1}}]),
{ok, _} = application:ensure_all_started(opentelemetry),
%% adds an exporter for a new table
Expand Down Expand Up @@ -139,34 +139,34 @@ end_per_testcase(_, _Config) ->
ok.

disable_auto_creation(_Config) ->
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_tracer(
{_, #tracer{instrumentation_scope=Library}} = opentelemetry:get_tracer(
tsloughter marked this conversation as resolved.
Show resolved Hide resolved
opentelemetry:get_application(kernel)),
?assertEqual(undefined, Library),
ok.

old_disable_auto_creation(_Config) ->
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_tracer(
{_, #tracer{instrumentation_scope=Library}} = opentelemetry:get_tracer(
opentelemetry:get_application(kernel)),
?assertEqual(undefined, Library),
ok.

application_tracers(_Config) ->
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_tracer(
{_, #tracer{instrumentation_scope=Library}} = opentelemetry:get_tracer(
opentelemetry:get_application(kernel)),
?assertEqual(<<"kernel">>, Library#instrumentation_library.name),
?assertEqual(<<"kernel">>, Library#instrumentation_scope.name),

%% tracers are unique by name/version/schema_url
NewKernelTracer = opentelemetry:get_tracer(kernel, <<"fake-version">>, undefined),
{_, #tracer{instrumentation_library=NewLibrary}} = NewKernelTracer,
?assertEqual(<<"kernel">>, NewLibrary#instrumentation_library.name),
?assertEqual(<<"fake-version">>, NewLibrary#instrumentation_library.version),
?assertEqual(undefined, NewLibrary#instrumentation_library.schema_url),
{_, #tracer{instrumentation_scope=NewLibrary}} = NewKernelTracer,
?assertEqual(<<"kernel">>, NewLibrary#instrumentation_scope.name),
?assertEqual(<<"fake-version">>, NewLibrary#instrumentation_scope.version),
?assertEqual(undefined, NewLibrary#instrumentation_scope.schema_url),

Tracer = opentelemetry:get_tracer(kernel, <<"fake-version">>, <<"http://schema.org/myschema">>),
{_, #tracer{instrumentation_library=NewLibrary1}} = Tracer,
?assertEqual(<<"kernel">>, NewLibrary1#instrumentation_library.name),
?assertEqual(<<"fake-version">>, NewLibrary1#instrumentation_library.version),
?assertEqual(<<"http://schema.org/myschema">>, NewLibrary1#instrumentation_library.schema_url),
{_, #tracer{instrumentation_scope=NewLibrary1}} = Tracer,
?assertEqual(<<"kernel">>, NewLibrary1#instrumentation_scope.name),
?assertEqual(<<"fake-version">>, NewLibrary1#instrumentation_scope.version),
?assertEqual(<<"http://schema.org/myschema">>, NewLibrary1#instrumentation_scope.schema_url),

ok.

Expand Down Expand Up @@ -434,15 +434,15 @@ update_span_data(Config) ->
ok.


tracer_instrumentation_library(Config) ->
tracer_instrumentation_scope(Config) ->
Tid = ?config(tid, Config),

TracerName = tracer1,
TracerVsn = <<"1.0.0">>,
Tracer = {_, #tracer{instrumentation_library=IL}} =
Tracer = {_, #tracer{instrumentation_scope=IL}} =
opentelemetry:get_tracer(TracerName, TracerVsn, "http://schema.org/myschema"),

?assertMatch({instrumentation_library,<<"tracer1">>,<<"1.0.0">>,<<"http://schema.org/myschema">>},
?assertMatch({instrumentation_scope,<<"tracer1">>,<<"1.0.0">>,<<"http://schema.org/myschema">>},
IL),

SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{}),
Expand All @@ -451,8 +451,8 @@ tracer_instrumentation_library(Config) ->

[Span1] = assert_exported(Tid, SpanCtx1),

?assertMatch({instrumentation_library,<<"tracer1">>,<<"1.0.0">>,<<"http://schema.org/myschema">>},
Span1#span.instrumentation_library).
?assertMatch({instrumentation_scope,<<"tracer1">>,<<"1.0.0">>,<<"http://schema.org/myschema">>},
Span1#span.instrumentation_scope).

%% check that ending a span results in the tracer setting the previous tracer context
%% as the current active and not use the parent span ctx of the span being ended --
Expand Down
8 changes: 4 additions & 4 deletions apps/opentelemetry_api/include/opentelemetry.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
-define(OTEL_STATUS_OK, ok).
-define(OTEL_STATUS_ERROR, error).

%% Holds information about the instrumentation library specified when
%% Holds information about the instrumentation scope specified when
%% getting a Tracer from the TracerProvider.
-record(instrumentation_library, {name :: unicode:unicode_binary() | undefined,
version :: unicode:unicode_binary() | undefined,
schema_url :: uri_string:uri_string() | undefined}).
-record(instrumentation_scope, {name :: unicode:unicode_binary() | undefined,
tsloughter marked this conversation as resolved.
Show resolved Hide resolved
version :: unicode:unicode_binary() | undefined,
schema_url :: uri_string:uri_string() | undefined}).

-record(span_ctx, {
%% 128 bit int trace id
Expand Down
18 changes: 12 additions & 6 deletions apps/opentelemetry_api/src/opentelemetry.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
get_text_map_extractor/0,
set_text_map_injector/1,
get_text_map_injector/0,
instrumentation_scope/3,
instrumentation_library/3,
timestamp/0,
timestamp_to_nano/1,
Expand All @@ -60,7 +61,7 @@
-include_lib("kernel/include/logger.hrl").

-export_type([tracer/0,
instrumentation_library/0,
instrumentation_scope/0,
trace_id/0,
span_id/0,
hex_trace_id/0,
Expand All @@ -86,7 +87,7 @@

-type tracer() :: {module(), term()}.

-type instrumentation_library() :: #instrumentation_library{}.
-type instrumentation_scope() :: #instrumentation_scope{}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth maintaining the type alias as well?

-type instrumentation_library() :: #instrumentation_scope{}. or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it. If any user actually uses the library record the least of what they have to deal with is the type spec.


-type trace_id() :: non_neg_integer().
-type span_id() :: non_neg_integer().
Expand Down Expand Up @@ -436,16 +437,21 @@ link_or_false(TraceId, SpanId, Attributes, TraceState) ->
false
end.

instrumentation_library(Name, Vsn, SchemaUrl) ->
instrumentation_scope(Name, Vsn, SchemaUrl) ->
case name_to_binary(Name) of
undefined ->
undefined;
BinaryName ->
#instrumentation_library{name=BinaryName,
version=vsn_to_binary(Vsn),
schema_url=schema_url_to_binary(SchemaUrl)}
#instrumentation_scope{name=BinaryName,
version=vsn_to_binary(Vsn),
schema_url=schema_url_to_binary(SchemaUrl)}
end.

%% this function remains solely to keep backwards compatibility
%% but `instrumentation_scope' should be used instead
instrumentation_library(Name, Vsn, SchemaUrl) ->
instrumentation_scope(Name, Vsn, SchemaUrl).

%% schema_url is option, so set to undefined if its not a string
schema_url_to_binary(SchemaUrl) when is_binary(SchemaUrl) ; is_list(SchemaUrl) ->
unicode:characters_to_binary(SchemaUrl);
Expand Down
2 changes: 1 addition & 1 deletion apps/opentelemetry_experimental/src/otel_meter.hrl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-record(meter, {module :: module(),
instrumentation_library :: otel_tracer_server:instrumentation_library() | undefined,
instrumentation_scope :: otel_tracer_server:instrumentation_scope() | undefined,
telemetry_library :: otel_tracer_server:telemetry_library() | undefined,
resource :: otel_resource:t() | undefined}).
-type meter() :: #meter{}.
Expand Down
4 changes: 2 additions & 2 deletions apps/opentelemetry_experimental/src/otel_meter_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ register_meter(Name, Vsn, #state{meter=Meter,
true ->
opentelemetry_experimental:set_meter(Name, {otel_meter_noop, []});
false ->
InstrumentationLibrary = opentelemetry:instrumentation_library(Name, Vsn, undefined),
InstrumentationScope = opentelemetry:instrumentation_scope(Name, Vsn, undefined),
opentelemetry_experimental:set_meter(Name, {Meter#meter.module,
Meter#meter{instrumentation_library=InstrumentationLibrary}})
Meter#meter{instrumentation_scope=InstrumentationScope}})
end.

%%
Loading