`GeneratorExit` sets span status to `ERROR`

[harupy]

Related to #4406. The following code sets the span status to ERROR in opentelemetry-api==1.31.0 and opentelemetry-sdk==1.31.0:

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

provider = TracerProvider()
processor = BatchSpanProcessor(ConsoleSpanExporter())
provider.add_span_processor(processor)

trace.set_tracer_provider(provider)
tracer = trace.get_tracer("my.tracer.name")


def f():
    span = tracer.start_span("test")
    with trace.use_span(span, end_on_exit=True):
        yield


g = f()
next(g)
g.close()

Output:

{
    "name": "test",
    "context": {
        "trace_id": "0xe1dd99a7ddbd4521416321571fa37d42",
        "span_id": "0x3d01a4fbfdad0508",
        "trace_state": "[]"
    },
    "kind": "SpanKind.INTERNAL",
    "parent_id": null,
    "start_time": "2025-03-13T11:47:07.138186Z",
    "end_time": "2025-03-13T11:47:07.138437Z",
    "status": {
        "status_code": "ERROR",
        "description": "GeneratorExit: "
    },
    "attributes": {},
    "events": [
        {
            "name": "exception",
            "timestamp": "2025-03-13T11:47:07.138431Z",
            "attributes": {
                "exception.type": "GeneratorExit",
                "exception.message": "",
                "exception.stacktrace": "Traceback (most recent call last):\n  File \"/Users/harutaka.kawamura/.pyenv/versions/miniconda3-latest/envs/mlflow/lib/python3.9/site-packages/opentelemetry/trace/__init__.py\", line 587, in use_span\n    yield span\n  File \"/Users/harutaka.kawamura/Desktop/repositories/mlflow/b.py\", line 19, in f\n    yield\nGeneratorExit\n",
                "exception.escaped": "False"
            }
        }
    ],
    "links": [],
    "resource": {
        "attributes": {
            "telemetry.sdk.language": "python",
            "telemetry.sdk.name": "opentelemetry",
            "telemetry.sdk.version": "1.31.0",
            "service.name": "unknown_service"
        },
        "schema_url": ""
    }
}

The same code doesn't set the span status to ERROR in opentelemetry-api==1.30.0 and opentelemetry-sdk==1.30.0:

{
    "name": "test",
    "context": {
        "trace_id": "0xc9f3585ec669a8e7c3f3fd64538d582f",
        "span_id": "0x7e5e006dae5a6345",
        "trace_state": "[]"
    },
    "kind": "SpanKind.INTERNAL",
    "parent_id": null,
    "start_time": "2025-03-13T15:19:56.265241Z",
    "end_time": "2025-03-13T15:19:56.265253Z",
    "status": {
        "status_code": "UNSET"
    },
    "attributes": {},
    "events": [],
    "links": [],
    "resource": {
        "attributes": {
            "telemetry.sdk.language": "python",
            "telemetry.sdk.name": "opentelemetry",
            "telemetry.sdk.version": "1.30.0",
            "service.name": "unknown_service"
        },
        "schema_url": ""
    }
}

Is this intended?

[harupy]

https://docs.python.org/3/library/exceptions.html#GeneratorExit

Raised when a generator or coroutine is closed; see generator.close() and coroutine.close(). It directly inherits from BaseException instead of Exception since it is technically not an error.

[harupy]

@tmaxwell-anthropic @emdneto @xrmx Any thoughts on this?

[tmaxwell-anthropic]

Hmmm, I don't know what's the best thing to do here.

I'm relatively new to the OpenTelemetry ecosystem. What are the practical effects of setting status=ERROR? I imagine the main effect is that trace-viewer UIs will show an alert on the span. This could be either good or bad:

• If the BaseException was unexpected, then I imagine the alert will be helpful in understanding what went wrong. For example, imagine if a server received a SIGINT while processing a request, so it raised KeyboardInterrupt. If I was looking at the traces to understand what happened, I think I'd want to see an alert on those spans.
• But if the BaseException is normal/expected, then I imagine the alert will be spurious and unhelpful. For example, if your code is calling generator.close() during normal operation, I imagine the status=ERROR is pretty annoying for you.

One option might be to set status=UNSET for BaseExceptions, but still create an exception event on the span. That way, there's at least some record of the fact that the span didn't exit normally.

Another option might be to special-case GeneratorExit specifically, so if the OTel SDK sees GeneratorExit, then it won't set status=ERROR. I figure almost nobody will want to see alerts for GeneratorExit. But, perhaps it's weird to give special treatment to this one specific exception class? Are there other "benign" exception-classes like GeneratorExit? Maybe the SDK should let users configure which exception-classes shouldn't result in status=ERROR?

(Whatever we decide, let's be sure to keep the behavior consistent between start_span() and start_as_current_span().)

What do other folks think?

[harupy]

Are there other "benign" exception-classes like GeneratorExit? Maybe the SDK should let users configure which exception-classes shouldn't result in status=ERROR?

SystemExit and KeyboardInterrupt also inherit from BaseException.

BaseException
 ├── BaseExceptionGroup
 ├── GeneratorExit
 ├── KeyboardInterrupt
 ├── SystemExit
 └── Exception
      ├── ...

source: https://docs.python.org/3/library/exceptions.html#exception-hierarchy

[xrmx]

Reverting looks the safest thing to do, we can then add more exceptions to the except if we want to catch them

[tmaxwell-anthropic]

we can then add more exceptions to the except if we want to catch them

The original motivation for #4406 was that I had spans which were exiting with asyncio.CancelledError or trio.Cancelled, but the span was being marked as successful. Unfortunately, the OTel SDK can't add those to the except without taking a dependency on asyncio or trio, which seems impractical.

How would you feel about treating GeneratorExit as a special case, and/or making this configurable?

[harupy]

How would you feel about treating GeneratorExit as a special case

This doesn't seem like a bad option because GeneratorExit is not an error and we can still allow asyncio.CancelledError or trio.Cancelled to mark the span as ERROR.