feat(otel): instrument Litestar specific components#4483
feat(otel): instrument Litestar specific components#4483cofin wants to merge 4 commits intofeat/otel-contrib-deprecatefrom
Conversation
Properly wrap guards, CLI, and events with OTEL spans
Properly wrap guards, CLI, and events with OTEL spans
…into feat/otel-updates
github-actions
bot
added
area/dependencies
provinzkraut
left a comment
provinzkraut
left a comment
There was a problem hiding this comment.
Generally a good idea, but I don't think this is the right approach.
Specifically, we shouldn't resort to patching, as it is extremely brittle. The only reason naive instrumentations do this is because frameworks do not provide the necessary hooks. Since we're doing a first party integration though, we can use all the hooks and, if needed, add additional ones.
| _OTEL_AVAILABLE: bool | None = None | ||
| try: # pragma: no cover - optional dependency | ||
| from opentelemetry.trace import TracerProvider as _RuntimeTracerProvider | ||
| except ImportError: # pragma: no cover | ||
| _RuntimeTracerProvider = object # type: ignore[misc,assignment] | ||
|
|
||
| _CUSTOM_TRACER_PROVIDER: _RuntimeTracerProvider | None = None # pyright: ignore | ||
|
|
||
|
|
||
| def _get_tracer(name: str) -> Any: | ||
| from opentelemetry import trace | ||
|
|
||
| tracer_provider = _CUSTOM_TRACER_PROVIDER or trace.get_tracer_provider() | ||
| return tracer_provider.get_tracer(name) | ||
|
|
||
|
|
||
| def _is_otel_available() -> bool: | ||
| """Check if OpenTelemetry is installed and available.""" | ||
| global _OTEL_AVAILABLE | ||
| if _OTEL_AVAILABLE is not None: | ||
| return _OTEL_AVAILABLE | ||
|
|
||
| try: | ||
| import opentelemetry # noqa: F401 | ||
|
|
||
| _OTEL_AVAILABLE = True | ||
| except ImportError: | ||
| _OTEL_AVAILABLE = False | ||
|
|
||
| return _OTEL_AVAILABLE |
There was a problem hiding this comment.
I don't understand this part. Importing the otel module doesn't make any sense if otel isn't available
|
|
||
|
|
||
| @asynccontextmanager | ||
| async def create_span_async(name: str, attributes: dict[str, Any] | None = None) -> AsyncGenerator[Any, None]: |
There was a problem hiding this comment.
What's the purpose of this? It doesn't do anything async as far as I can tell?
| yield None | ||
| return | ||
|
|
||
| from opentelemetry.trace import Status, StatusCode |
There was a problem hiding this comment.
There really shouldn't be an import in a hot path
|
|
||
| try: | ||
| yield span | ||
| except Exception as exc: |
There was a problem hiding this comment.
Is this necessary? I was under the impression that the active span already records exceptions
| result = provider_func(*args, **kwargs) | ||
| if inspect.isawaitable(result): | ||
| result = await result | ||
|
|
||
| # Add result type as attribute | ||
| if span and span.is_recording(): | ||
| span.set_attribute("litestar.dependency.result_type", type(result).__name__) |
There was a problem hiding this comment.
I think this might break for generator dependencies
2 participants
Since we rely on the upstream ASGI open telemetry integration, some of the Litestar specific concepts did not always get wrapped inside the correct span.
This change allows the Open Telemetry plugin to instrument guards, events, and the CLI commands in Litestar.