open-telemetry/opentelemetry-erlang

dmy-gh

@dmy-gh

I had to do:

current_ctx = Ctx.attach(ctx)
...
Span.end_span(child)
Ctx.detach(current_ctx)

In a similar example with a GenServer.call/3 instead of a Task.

Without it new traces where ok but the first one stacked part of the new ones

It's way easier with the with_span macro

Tristan Sloughter

@tsloughter

the with_span macro does the attaching, ending and detaching

also, with_span can take a context as the first argument. so in some cases you don't need to attach the context, just do Tracer.with_span ctx, name, %{}, do ...

dmy-gh

@dmy-gh

Do you have a use case example for the with_span with a context as the first argument?

In what does it differ from a newly created span that one would want as a child or a linked span?

Tristan Sloughter

@tsloughter

in the async example you have instead of attaching ctx you could pass it to with_span

dmy-gh

@dmy-gh

That's what I wanted to do when I was passing the context to the parent attribute

Switching to this version

Well, it still needs to start the span and set it current but at least it prevents from having to end it and detach it

The link attribute will be more clean :thumbsup:

dmy-gh

@dmy-gh

PR merge, nice :thumbsup:

Even though I now have a link instead of :undefined when calling OpenTelemetry.link/1, I still get an error during the span creation
What I tried differs from the repository test (open-telemetry/opentelemetry-erlang@529039d) as it only uses the with_span macro:

  def link do
    Tracer.with_span "parent" do
      parent = Tracer.current_span_ctx()

      task =
        Task.async(fn ->
          Tracer.with_span "child", %{links: [OpenTelemetry.link(parent)]} do
            :hello
          end
        end)

      Task.await(task)
    end
  end

dmy-gh

@dmy-gh

OpenTelemetry.link(parent) returns {:link, 140973076598678694095226025386297353991, 1148588555666536500, [], :undefined}

The stack trace ends with:

  {:opentelemetry_exporter_trace_service_pb, :is_empty_string, [:undefined],
   [
     file: '/test/deps/opentelemetry_exporter/apps/opentelemetry_exporter/src/opentelemetry_exporter_trace_service_pb.erl',
     line: 928
   ]},

Tristan Sloughter

@tsloughter

oh, issue is in the exporter expecting a string. hm

Tristan Sloughter

@tsloughter

ah, tracestate is a string in the proto

ok, so its supposed to be W3C TraceSttate encoded before being used in the proto

I can fix that

oh, exporter already does convert to a string

are you using the git branch of the exporter or the hexpackage?

dmy-gh

@dmy-gh

I'm using the same commit ref for opentelemetry, opentelemetry_api and opentelemetry_exporter

dmy-gh

@dmy-gh

%{
  ...,
  "opentelemetry": {:git, "https://github.com/open-telemetry/opentelemetry-erlang.git", "fc25e7a7adb0ab217f1c2616e580e49c4cea541f", [sparse: "apps/opentelemetry"]},
  "opentelemetry_api": {:git, "https://github.com/open-telemetry/opentelemetry-erlang.git", "fc25e7a7adb0ab217f1c2616e580e49c4cea541f", [sparse: "apps/opentelemetry_api"]},
  "opentelemetry_exporter": {:git, "https://github.com/open-telemetry/opentelemetry-erlang.git", "fc25e7a7adb0ab217f1c2616e580e49c4cea541f", [sparse: "apps/opentelemetry_exporter"]},
}

Tristan Sloughter

@tsloughter

yup, found the bug. its in the link encoding in the exporter

will get a fix out

Tristan Sloughter

@tsloughter

open-telemetry/opentelemetry-erlang#198

dmy-gh

@dmy-gh

open-telemetry/opentelemetry-erlang#198

Thank you :pray:

dmy-gh

@dmy-gh

It works :slight_smile:

Tristan Sloughter

@tsloughter

cool

dmy-gh

@dmy-gh

:parent and :links represent different concepts, why :parent has been removed?

Tristan Sloughter

@tsloughter

removing the parent option is more a simplification. the parent can still be set manually by passing a context as the first argument that has the span you want as parent to be the current span in that context

the parent option would mean yet another way the parent could be set and doesn't add much

dmy-gh

@dmy-gh

Ok, that's what you explained yesterday

Also it seems to produce a correct trace, it makes Dialyzer complain

Tristan Sloughter

@tsloughter

what makes dialyzer complain?

dmy-gh

@dmy-gh

  def parent do
    Tracer.with_span "parent" do
      parent_ctx = Ctx.get_current()

      task =
        Task.async(fn ->
          Tracer.with_span parent_ctx, "child", %{} do
            Tracer.with_span "sub child" do
              Process.sleep(2_000)
            end

            Process.sleep(1_000)
          end
        end)

      Task.await(task)
    end
  end

Produces:

  InstrumentationLibrarySpans #0
 InstrumentationLibrary  
 Span #0
     Trace ID       : 3590104f363edf2915ab7dea675346f7
     Parent ID      : 
     ID             : d1b85fe9bb49f1a7
     Name           : parent
     Kind           : SPAN_KIND_INTERNAL
     Start time     : 2021-02-02 16:32:06.285669427 +0000 UTC
     End time       : 2021-02-02 16:32:09.28805425 +0000 UTC
     Status code    : STATUS_CODE_UNSET
     Status message : 
 Span #1
     Trace ID       : 3590104f363edf2915ab7dea675346f7
     Parent ID      : d1b85fe9bb49f1a7
     ID             : 7d04026e36f93266
     Name           : child
     Kind           : SPAN_KIND_INTERNAL
     Start time     : 2021-02-02 16:32:06.2857683 +0000 UTC
     End time       : 2021-02-02 16:32:09.287856878 +0000 UTC
     Status code    : STATUS_CODE_UNSET
     Status message : 
 Span #2
     Trace ID       : 3590104f363edf2915ab7dea675346f7
     Parent ID      : 7d04026e36f93266
     ID             : 7e3223982ba56037
     Name           : sub child
     Kind           : SPAN_KIND_INTERNAL
     Start time     : 2021-02-02 16:32:06.285813566 +0000 UTC
     End time       : 2021-02-02 16:32:08.286844333 +0000 UTC
     Status code    : STATUS_CODE_UNSET
     Status message :

Which seems ok

Is Ctx.get_current() what should be the first argument?

dmy-gh

@dmy-gh

https://github.com/open-telemetry/opentelemetry-erlang/blob/main/apps/opentelemetry_api/src/otel_tracer.erl#L71-L72

The third and fourth args are inverted int the spec

Tristan Sloughter

@tsloughter

yea, the first argument would be the ctx. using the one returned by get_current() would be the same as not passinga ctx tho, since it is the one from the pdict

oh, thnaks, I see

annoying that CI dialyzer passed

dmy-gh

@dmy-gh

yea, the first argument would be the ctx. using the one returned by get_current() would be the same as not passinga ctx tho, since it is the one from the pdict

If it was in the same process which is not the case here

Tristan Sloughter

@tsloughter

right

dmy-gh

@dmy-gh

What would you advise to create span in a GenServer?
As the context referential is per process, passing the parent context as we did for the Task seems to work but with_span macro attaches it back at the end so any call to this GenServer with sub part containing a with_span will get the last set context as a parent

Tristan Sloughter

@tsloughter

if you pass context as an argument to with_span then it'll go back to the previous context which was undefined

dmy-gh

@dmy-gh

Doesn't this mean that it sets the parent context as the new current context at the end here: https://github.com/open-telemetry/opentelemetry-erlang/blob/613f2d1086a7f2e36757ef545a65540664492ac2/apps/opentelemetry/src/otel_tracer_default.erl#L59 ?

Tristan Sloughter

@tsloughter

the parent context, as in the Ctx variable passed in (or read in from the pdict if the user doesn't pass one), yes

and that should be detach but it doesn't really have an effect on the outcome so I keep forgetting to fix that

Where communities thrive

People

Repo info

Activity