refactor span modules and add span impl setup callback #7

tsloughter · 2019-08-17T16:43:58Z

No description provided.

tsloughter · 2019-08-18T14:18:43Z

I've updated this to configure the span and context implementations through the setup of ot_tracer_sdk.

One other change I'm considering is rename ot_span and ot_ctxtoot_span_apiandot_ctx_api` respectively to signal they are just apis and not implementations.

hauleth

We probably should add Elvis to the CI pipeline

src/opentelemetry.app.src

hauleth · 2019-08-19T20:50:13Z

src/opentelemetry_app.erl

+    %% if the span impl needs to have a process supervised it must be
+    %% setup after the supervision tree has started.
+    {ot_tracer, TracerOpts} = lists:keyfind(ot_tracer, 1, Opts),
+    ot_tracer_sdk:setup(TracerOpts),


Shouldn't this be ran before starting supervisor itself?

It can't if it is to be allowed to have supervision by the application.

Hm, though the return for the tracer setup function could be a list of children to start...

Yeah, I would do that... use the configuration to detemine whether to start children of the top-level sup.

Yea, issue was you should be able to enable tracers at runtime. But I can add a function that handles that for the user allowing the setup function to still return specs.

hauleth · 2019-08-19T20:50:40Z

src/ot_ctx.erl

-
-spec with_value(term(), term(), fun()) -> ok.
-with_value(Key, Value, Fun) ->
-    ?ctx:with_value(Key, Value, Fun).


Are we removing that functions once again?

Yes, I talk about it in the issue I created. If the context and span are configurable outside the scope of the tracer it is confusing. The tracer is what gets configured to use specific context and span implementations.

src/ot_span_ets.erl

src/ot_span_utils.erl

seancribbs · 2019-08-20T19:34:48Z

src/ot_span_ets.erl

@@ -68,6 +75,34 @@ finish_span(#span_ctx{span_id=SpanId,
 finish_span(_) ->
    ok.

+-spec get_ctx(opentelemetry:span()) -> opentelemetry:span_ctx().
+get_ctx(_Span) ->
+    #span_ctx{}.


I'm guessing these are just stubs?

Could the stubs return anything significant?
I'm looking at the utils module and all that data seems like it should be available anyway?

tsloughter · 2019-08-20T19:53:04Z

I think I need to rename some modules.

The reason there is any _sdk module is that technically there should be both an opentelemetry_api and opentelemetry_sdk application. Instead of doing that my plan was to have them both in the same application but identified through the suffix.

But it definitely causes confusion for anyone looking at the code that isn't coming directly from the OTel spec -- and maybe even causes confusion in that case.

Instead I'll add to the readme an explanation of how the OTel concepts of API and SDK map to our codebase and for example ot_tracer_sdk will become ot_tracer_default.

tsloughter · 2019-08-21T02:02:53Z

@hauleth @seancribbs I made a number of updates. I think it is getting closer...

ferd

I think for the most part this makes sense, I see the point of changing to persistent terms. the one big confusion I have is regarding the majority of the behaviour being stubbed out. I'm not too sure what's that about.

ferd · 2019-09-06T02:12:25Z

src/ot_span_ets.erl

@@ -68,6 +75,34 @@ finish_span(#span_ctx{span_id=SpanId,
 finish_span(_) ->
    ok.

+-spec get_ctx(opentelemetry:span()) -> opentelemetry:span_ctx().
+get_ctx(_Span) ->
+    #span_ctx{}.


Could the stubs return anything significant?
I'm looking at the utils module and all that data seems like it should be available anyway?

ferd · 2019-09-06T02:17:21Z

src/ot_tracer_default.erl

+    lists:filtermap(fun({ConfigKey, PersistentKey}) ->
+                        {Module, Args} = maps:get(ConfigKey, Opts),
+                        persistent_term:put(PersistentKey, Module),
+                        case erlang:function_exported(Module, start_link, 1) of


Risks of the module not being loaded? I guess it doesn't matter for releases.

Still, it can be problematic during development.

src/ot_span_utils.erl

tsloughter · 2019-09-06T17:37:28Z

@ferd hm, what behaviour is "stubbed out"? Do you mean like ot_span_ets and other placeholders?

ferd · 2019-09-06T18:23:20Z

@ferd hm, what behaviour is "stubbed out"? Do you mean like ot_span_ets and other placeholders?

Yeah. They have like the create/stop span functions defined, the rest is just ignoring all arguments and returning ok.

tsloughter · 2019-09-06T18:55:02Z

Yea, just placeholders to have the behaviour callbacks covered but haven't yet implemented the logic.

tsloughter · 2019-09-11T14:55:21Z

Ugh, it seems to not be in sync with circleci which now has all jobs in the config green.

ferd

Approving this. I'm still a bit weirded out by the stubs and there's one potential/non-ideal thing regarding runtime, but those are not blockers to the other improvements down the pipeline.

tsloughter · 2019-09-11T15:44:00Z

Closing and reopening because circle and github won't sync properly.

* opentelemetry metrics API This patch includes a behaviour for implementing the Meter SDK and API modules for calling the meter for creating and recording measurements. * Update src/ot_meter.erl Co-Authored-By: Arkadiusz Gil <arkgil@users.noreply.github.com> * cleanup of metrics api based on comments on PR * add Meter Provider * remove gauge * add meter macros and registration * add elixir meter api module * add register_application_* functions to OpenTelemetry module * add observer api * add elixir modules for counter, measure and observer api * add meter to bound instrument return to simplify api * updates to observer api * remove label set function * more simplification and some docs * fix meter lookup * add counter and measure specific macros * fix get_meter typespec and add miissing callbacks to noop meter * fix provider return values to booleans for registering traer/meter Co-authored-by: Arkadiusz Gil <arkgil@users.noreply.github.com>

* Initial commit * api initial commit ot_tracer: behavour that an otel tracer must implement ot_span: behaviour an otel span must implement otel: friendly user api on top of ot_tracer and ot_span * ot_span: add type specs on functions * add functions for creating records and align with latest otel protos * update readme to describe use of api application * updates for fred's PR review * separate context propagation otep * add with_value function to run function with scoped context * add http propagation for baggage, correlation and trace * remove callbacks from otel module that were accidentally copeid from ot_span * fix type specs after dialyzing opentelemetry * fix baggage and correlations to/from http propagation * add verification checks that tracer and context implement behaviours * add optional config value to context manager like the tracer * add some additional type specs * make propagator funs fully qualified important for being able to run upgrades. without fully qualified functions the funs will eventually become badfuns after the third module reload. * update span start_opts to include attributes, sampling hint and start_time (#6) * Initial Elixir API (#3) * start on the elixir API * add .gitignore, address feedback * update span start_opts to include attributes, sampling hint and start_time (#6) * use VERSION file for application version * read version from version file Co-authored-by: Tristan Sloughter <t@crashfast.com> * remove baggage. it was removed from otep-66 (#9) * remove resource from span record (#12) * add CODEOWNERS file (#15) * Registering named tracers (#16) * add tracer provider interface for named tracers * update readme with use about named tracer * fix register_tracer type spec * move span record to sdk * add additional details about what register_tracer does * new registeration, plus add set_span/2, end_span/2 and replace otel with macros in tracer.hrl (#17) * support registering an application's tracer * add set_span/2, end_span/2 and replace otel with macros in tracer.hrl * update readme to use register_application_tracer/1 * Set Erlang Approvers as CODEOWNERS (#19) * Elixir API based on the Erlang macro api and application registered tracers (#18) * add set_span/2, end_span/2 and replace otel with macros in tracer.hrl * Elixir API macros to match the Erlang macros * include erlang docs in elixir docs (#20) * include erlang docs in elixir docs * make erlang_docs function in mix.exs more readable * make edocs before mix docs and add hex metadata * make use of is_recording in ot_span and update events field to latest spec (#21) * use is_recording to skip possibly expensive span ops * update Event type to otel beta spec * missed a couple of is_recording and events changes (#22) * remove old is_recording_events callback * fix add_events callback type spec to new events record type * fixing up mix.lock and use of event (#23) * update elixir opentelemetry api to replace time events with events * update mix lock file * expand the docs in open_telemtry.ex, the readme and add CONTRIBUTING.md (#24) * add elixir docs to OpenTelemetry module * add register_application_tracer function to OpenTelemetry module and elixir to readme * add readme section about including the SDK * add contributing doc * add docs to span.ex and tracer.ex * chore: cleanup Elixir code - Macros use `:bind_quoted` where possible - Remove needless aliases in tests * Add create_span and new end_span/2 (#27) * Share configuration between Rebar3 and Mix (#11) * ft: share configuration between Rebar3 and Mix This should reduce amount of places where the configuration mismatch can happen. Current solution uses Rebar3 configuration as a source of truth. TBD: - support for Rebar3 profile sharing * fix(docs): build pages for the Erlang files * chore: update mix.lock * a bit of cleanup (#29) * remove unused context with_value function * fix is_recording macro in tracer header * opentelemetry metrics API (#7) * opentelemetry metrics API This patch includes a behaviour for implementing the Meter SDK and API modules for calling the meter for creating and recording measurements. * Update src/ot_meter.erl Co-Authored-By: Arkadiusz Gil <arkgil@users.noreply.github.com> * cleanup of metrics api based on comments on PR * add Meter Provider * remove gauge * add meter macros and registration * add elixir meter api module * add register_application_* functions to OpenTelemetry module * add observer api * add elixir modules for counter, measure and observer api * add meter to bound instrument return to simplify api * updates to observer api * remove label set function * more simplification and some docs * fix meter lookup * add counter and measure specific macros * fix get_meter typespec and add miissing callbacks to noop meter * fix provider return values to booleans for registering traer/meter Co-authored-by: Arkadiusz Gil <arkgil@users.noreply.github.com> * take default argument in ot_ctx:ctx and some simple bug fixes (#38) * Add callbacks for getting the Resource from a tracer provider (#40) * handle codechange in ot_tracer_provider * add callback to get resource from tracer provider * fix spec for provider resource fetching as resource is defined in the SDK * bump VERSION file to 0.3.0 (#41) * add include dir to files published to hex (#42) * removing wts dependency (#43) * add hex metadata * remove use of wts. get time_offset at export instead of at creation * use system time in nano seconds for an event time * version bump to 0.3.1 (#44) * add otel exporter to list and add lists for integrations (#45) * update readme to use set_attribute not the old add_attribute (#50) * drop attributes and events that aren't a list (#51) * Repair typespec (#46) I'm pretty sure you didn't intend to document [`register_application_tracer/1`][rat1] as: > Current time in UNIX Epoch time, nanoseconds since 00:00:00 UTC on > 1 January 1970. ... but that's how it turned out, perhaps due to a slip-up in 54ce3ad. [rat1]: https://hexdocs.pm/opentelemetry_api/0.3.1/OpenTelemetry.html#register_application_tracer/1 Co-authored-by: Tristan Sloughter <t@crashfast.com> * fix bug where there is no current span ctx and update_name is called (#52) * replace create_span with start_inactive_span (#53) * Add accessors to deconstruct Span (#54) * Add accessors to deconstruct Span When trying to link traces across separate systems (either linked via HTTP calls or a message queue), having direct access to the TraceID and the SpanID make it easier to serialize this properly and recreate a Link on the receiving end. This PR adds support for deconstructing a Span Context to get access to these values. I've added a couple of tests but they aren't all passing, so could use a hand getting the test for establishing links to parent trace in shape. * Rework CT test * Use the right type on type specs * Removed test that should be part of an SDK instead * Fix issues/48 (#56) - OpenTelemetry removing `/"` and replacing this with `"` as it creates confusion. - Updated the example doc for the new API updates. Modified `OpenTelemetry.Span` Following changes were done. - Changes related to documentation example `add_event` - Updated `add_event/1` to `add_event/2` to accept `event_name` and `event_attributes`. * Update readme (#59) * version bump to 0.3.2 (#55) * Fixing the string formatting acrosst the project. (#62) Changes to be committed: modified: lib/open_telemetry/measure.ex modified: lib/open_telemetry/meter.ex modified: lib/open_telemetry/observer.ex modified: lib/open_telemetry/tracer.ex * Update span add_event docs variable name (#64) Seems like the variables on lines 9,10 (`ecto_attributes`, and `ecto_event`) are the same variable, but named differently? * Unspecified is not a valid span kind (#67) * move context implementation to api app (#66) * moving to github actions to use the same as the rest of OpenTelemetry org (#68) * fix sampler typespec to term() since sampler is in SDK app * move to github actions for CI * support passing just a span_ctx to link functions (#69) * support passing just a span_ctx to link functions * use covertool and codecov instead of coveralls * fix elixir otp version in main.yml * fix link/2 to take undefined atom and fix typespecs * correct docs for timestamp functions (#70) * Add http status helper (#78) * Add http status helper * Update module doc * Fix message fallback * Use macro * Update headers typing (#79) Co-authored-by: Tristan Sloughter <t@crashfast.com> * Add atom support (#80) * Remove CircleCI (#81) Github actions appears to be the CI system now * Metrics api 0.4.0 (#71) * remove uses of 'labelset' which was removed from the spec * initial work on updated metrics api 0.4.0 * rename instrument function to definition and add elixir api * prefix meter macros with ot_ * Update lib/open_telemetry/sum_observer.ex Co-authored-by: Tino Breddin <tolbrino@users.noreply.github.com> * Update lib/open_telemetry/updown_sum_observer.ex Co-authored-by: Tino Breddin <tolbrino@users.noreply.github.com> * add some docs to ot_instrument Co-authored-by: Tino Breddin <tolbrino@users.noreply.github.com> * ctx api update and add optional explicit ctx argument to start span functions (#83) * rename correlations to baggage * update context api * make context to an optional explicit argument to start_span * rename ot_ctx:ctx() type to ot_ctx:t() to be consistent * feat: move opentelemetry app under apps * fix: remove CircleCI configuration * fix(cover): upload coverage report for API * fix: run CT in CI as well as EUnit * chore(cover): properly mark each coverage report This should result with better reports as soon as coverage reports start to parse. Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> Co-authored-by: Tristan Sloughter <t@crashfast.com> Co-authored-by: Zach Daniel <zachary.s.daniel@gmail.com> Co-authored-by: Greg Mefford <greg@gregmefford.com> Co-authored-by: Arkadiusz Gil <arkgil@users.noreply.github.com> Co-authored-by: Garth Kidd <garth@garthk.com> Co-authored-by: Leandro Ostera <leandro@ostera.io> Co-authored-by: Yatender Singh <yatender.nitk@gmail.com> Co-authored-by: Marc Delagrammatikas <delagrammatikas@gmail.com> Co-authored-by: Bryan Naegele <bryannaegele@users.noreply.github.com> Co-authored-by: Dave Lucia <davelucianyc@gmail.com> Co-authored-by: Tino Breddin <tolbrino@users.noreply.github.com>

…ele-patch-1 Format message body as string for non-200's

…atch-procesor Refactor otel batch procesor

tsloughter mentioned this pull request Aug 17, 2019

Tracer, span and context implementations and how to configure them #8

Closed

tsloughter requested a review from hauleth August 19, 2019 15:12

hauleth reviewed Aug 19, 2019

View reviewed changes

tsloughter force-pushed the override-overload branch from 0e7ca2b to bf1b8fc Compare August 19, 2019 23:22

seancribbs reviewed Aug 20, 2019

View reviewed changes

tsloughter force-pushed the override-overload branch from cc771c0 to 89d610d Compare August 21, 2019 02:01

tsloughter force-pushed the override-overload branch from accba5f to 1dfe015 Compare August 21, 2019 14:57

tsloughter mentioned this pull request Aug 24, 2019

implement seq_trace based context #9

Merged

tsloughter requested a review from hauleth August 27, 2019 21:22

tsloughter mentioned this pull request Aug 31, 2019

samplers #10

Merged

ferd reviewed Sep 6, 2019

View reviewed changes

src/ot_span_utils.erl Show resolved Hide resolved

tsloughter requested a review from deadtrickster as a code owner September 11, 2019 14:35

tsloughter added 5 commits September 11, 2019 08:37

refactor span modules and add span impl setup callback

df46493

configure span and ctx implementations through ot_tracer_sdk

bbbb18e

rename default tracer and move setup to before root sup

6e7eafb

add to the readme a description of the API/SDK split

1a90830

add test of overriding the default tracer

c1e5fa0

tsloughter requested a review from ferd September 11, 2019 14:39

tsloughter force-pushed the override-overload branch from cd2bc06 to 854ec72 Compare September 11, 2019 14:50

tsloughter force-pushed the override-overload branch from 4567180 to 41cf282 Compare September 11, 2019 15:08

remove otp21 from circle until I fix the orb cache values

b4affb6

tsloughter force-pushed the override-overload branch from 41cf282 to b4affb6 Compare September 11, 2019 15:20

ferd approved these changes Sep 11, 2019

View reviewed changes

just check compilation on otp 21

1cfa419

tsloughter closed this Sep 11, 2019

hauleth pushed a commit to hauleth/opentelemetry-erlang that referenced this pull request Sep 30, 2020

Merge pull request open-telemetry#7 from opentelemetry-beam/bryannaeg…

5048a78

…ele-patch-1 Format message body as string for non-200's

SergeTupchiy added a commit to SergeTupchiy/opentelemetry-erlang that referenced this pull request Nov 20, 2023

Merge pull request open-telemetry#7 from SergeTupchiy/refactor-otel-b…

4a86254

…atch-procesor Refactor otel batch procesor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor span modules and add span impl setup callback #7

refactor span modules and add span impl setup callback #7

tsloughter commented Aug 17, 2019

tsloughter commented Aug 18, 2019

hauleth left a comment

hauleth Aug 19, 2019

tsloughter Aug 19, 2019

tsloughter Aug 19, 2019

seancribbs Aug 20, 2019

tsloughter Aug 20, 2019

hauleth Aug 19, 2019

tsloughter Aug 19, 2019

seancribbs Aug 20, 2019

tsloughter Aug 21, 2019

ferd Sep 6, 2019

tsloughter commented Aug 20, 2019

tsloughter commented Aug 21, 2019

ferd left a comment

ferd Sep 6, 2019

ferd Sep 6, 2019

hauleth Sep 11, 2019

tsloughter commented Sep 6, 2019

ferd commented Sep 6, 2019

tsloughter commented Sep 6, 2019

tsloughter commented Sep 11, 2019

ferd left a comment

tsloughter commented Sep 11, 2019

refactor span modules and add span impl setup callback #7

refactor span modules and add span impl setup callback #7

Conversation

tsloughter commented Aug 17, 2019

tsloughter commented Aug 18, 2019

hauleth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsloughter commented Aug 20, 2019

tsloughter commented Aug 21, 2019

ferd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsloughter commented Sep 6, 2019

ferd commented Sep 6, 2019

tsloughter commented Sep 6, 2019

tsloughter commented Sep 11, 2019

ferd left a comment

Choose a reason for hiding this comment

tsloughter commented Sep 11, 2019