open-telemetry / open-telemetry/opentelemetry-cpp

[EXAMPLE] Use of WithActiveSpan() is confusing

Open
#2,418 6 comments 0 reactions 1 assignee View on GitHub

@marcalff is already working on this.

Since Dec 4, 2023.

Stale triage/accepted
Dominant language
C++
Stars
1.4k
Forks
632
Avg merge
1d 13h
Merged PRs (30d)
75

Description

In the provided http example, the server does the following upon reception of a client request:


{
  StartSpanOptions options;
  options.kind  = SpanKind::kServer; 
  ...
  auto new_context = prop->Extract(carrier, current_context);
  options.parent   = GetSpan(new_context)->GetContext();
  ...
  // start span with parent context extracted from http header
  auto span = get_tracer("http-server")->StartSpan(span_name, {}, options);
  // mark the span as active
  auto scope = get_tracer(‘’)->WithActiveSpan(span);
  ...
  span->End();
  ...
}

I can hardly figure out why the span is created, then explicitly activated. I would expect something like:

{
  ...
  // start span with parent context extracted from http header **and make it the active one**
  auto scope = Scope(get_tracer("http-server")->StartSpan(span_name, {}, options));
  ...
}

Ok, one could argue that there's now no way to explicitly End the span. Ok, but why not simply add an accessor to the underlying Span to openTelemetry::trace::Scope? Something like:

{
  ...
  // start span with parent context extracted from http header **and make it the active one**
  auto scope = Scope(get_tracer("http-server")->StartSpan(span_name, {}, options));
  ...
  scope.Span()->AddEvent(...);
  ...
  // if really required...
  scope.Span()->End();
}

Would that violate a design choice or the Scope paradigm?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.