open-telemetry / open-telemetry/opentelemetry-python

Errors are being raised in API and SDK

Open
#2,148 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
2.6k
Forks
1k
Avg merge
4d 15h
Merged PRs (30d)
19

Description

Our most basic example is this one:

from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import (
    BatchSpanProcessor,
    ConsoleSpanExporter,
)

trace.set_tracer_provider(TracerProvider())
trace.get_tracer_provider().add_span_processor(
    BatchSpanProcessor(ConsoleSpanExporter())
)
tracer = trace.get_tracer(__name__)
with tracer.start_as_current_span("foo"):
    print("Hello world!")

If we remove the argument "foo" in this line:

with tracer.start_as_current_span("foo"):

then running this example fails with this error:

Traceback (most recent call last):
  File "/home/ocelotl/github/ocelotl/opentelemetry-python/docs/examples/basic_tracer/basic_trace.py", line 27, in <module>
    with tracer.start_as_current_span():
  File "/home/ocelotl/.pyenv/versions/3.9.5/lib/python3.9/contextlib.py", line 244, in helper
    return _GeneratorContextManager(func, args, kwds)
  File "/home/ocelotl/.pyenv/versions/3.9.5/lib/python3.9/contextlib.py", line 87, in __init__
    self.gen = func(*args, **kwds)
TypeError: start_as_current_span() missing 1 required positional argument: 'name'

This should not happen, the spec states here:

API methods MUST NOT throw unhandled exceptions when used incorrectly by end users.

Our SDK methods are also unsafe, we even raise exceptions directly. Even if we did not raise an exception there, and instead we logged an error we must end up returning a No Op object, as required by the spec (which we don't do):

Whenever API call returns values that is expected to be non-null value - in case of error in processing logic - SDK MUST return a "no-op" or any other "default" object that was (ideally) pre-allocated and readily available. This way API call sites will not crash on attempts to access methods and properties of a null objects.

That means that just wrapping the code of every method of every class or every function in a try except and logging an error is not enough. We need a way to define these NoOps and return them when necessary.

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.

Research direction

Start with the error-handling principles linked in the issue, then inspect docs/examples/basic_tracer/basic_trace.py and opentelemetry-sdk/src/opentelemetry/sdk/trace/init.py. Determine the scope of the API and SDK no-op behavior and how incorrect calls should be handled. Done means the described example and affected API or SDK calls follow the specification without unhandled exceptions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, observability-sre
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.