Repository navigation
feat(tracer): add OpenTelemetryProvider implementation (#7003) - #8366
Alwaysgaurav1 wants to merge 3 commits into
Conversation
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
|
No acknowledgement section found. Please make sure you used the template to open a PR and didn't remove the acknowledgment section. Check the template at |
|
iblancasa
left a comment
There was a problem hiding this comment.
The provider abstraction is a useful direction, but I think this needs more work before it is ready to merge.
Issue #7003 and the linked RFC describe a first-class OpenTelemetry experience that can work alongside Lambda autoinstrumentation. This PR currently provides basic child-span creation, but it does not yet define the expected behavior for dependencies, asynchronous functions, error status, metadata handling, or automatic instrumentation.
Could you please address the inline runtime issues and add tests using a real OpenTelemetry SDK/in-memory exporter rather than only mocks? In particular, we should verify async decorators, parent-child context, error status, metadata serialization and size limits and the behavior of patch/patch_all.
It would also help to document how customers select this provider and whether this is intended as a complete OpenTelemetry tracing solution or only a lower-level provider building block.
I know it has been a while since you created the PR and maybe you are not interested anymore in the feature, your priorities changed or something else. If you don't have bandwidth for working on this PR, let me know and I can take it.
| def in_subsegment_async(self, name: str | None = None, **kwargs) -> Generator[BaseSegment, None, None]: | ||
| name = name or "subsegment" | ||
| if self._tracer is not None: | ||
| with self._tracer.start_as_current_span(name) as span: |
There was a problem hiding this comment.
in_subsegment_async is decorated with @contextmanager, which creates a synchronous context manager. Powertools uses this method with async with, so every async @capture_method call will fail at runtime
|
|
||
| def __init__(self, tracer: Any | None = None): | ||
| if tracer is None: | ||
| try: |
There was a problem hiding this comment.
When OpenTelemetry is not installed, this silently creates a no-op provider. Users can deploy successfully but receive no traces.
I guess something should be, at least, printed
| def add_exception( | ||
| self, | ||
| exception: BaseException, | ||
| stack: list[traceback.StackSummary] | None = None, |
There was a problem hiding this comment.
Converting metadata with str(value) loses structured data and produces Python representations rather than stable telemetry values.
It can also create very large span attributes. Please use bounded JSON serialization or record metadata as a bounded span event. The same issue exists in put_metadata
| attr_key = f"{namespace}.{key}" if namespace else key | ||
| span.set_attribute(attr_key, str(value)) | ||
| except ImportError: | ||
| pass |
There was a problem hiding this comment.
Tracer enables automatic patching by default, but patch and patch_all silently do nothing for this provider. This means customers may assume HTTP, AWS SDK, and database calls are instrumented when they are not
| stack: list[traceback.StackSummary] | None = None, | ||
| remote: bool = False, | ||
| ): | ||
| if self.span and hasattr(self.span, "record_exception"): |
There was a problem hiding this comment.
Recording an exception does not set the OpenTelemetry span status to ERROR. Failed handlers and methods may therefore appear successful in tracing backends
|
Hi @Alwaysgaurav1, thanks for putting this together, and sorry it sat here for two months without a proper answer. We're still debating how OpenTelemetry should fit into Tracer: whether it's a provider like this PR does, or something broader that also covers Logger and Metrics, and how it interacts with the GenAI semantic conventions and ADOT. That discussion is still open in #7003, so I'm going to close this PR for now rather than keep it hanging while the design isn't settled. To be clear, this isn't a judgement on the code. When we land on a direction and implement it, I'll make sure you get credit for this work, and you're more than welcome to join the discussion in #7003 in the meantime. Thank you! |
|
Thank you for the update and for the thoughtful explanation! I completely understand holding off until the broader architectural direction (unifying Tracer, Logger, Metrics, and ADOT integration) is established. While reviewing the code and feedback, I also noted a few key areas that we'll want in the final implementation:
I'm happy to follow the discussion on #7003 and contribute once the design direction is finalized. Thanks again! |



Issue number: closes #7003
Summary
Changes
OpenTelemetrySegmentandOpenTelemetryProviderinaws_lambda_powertools.tracing.opentelemetry.OpenTelemetryProviderandOpenTelemetrySegmentfromaws_lambda_powertools.tracing.Tracerdecorator integration intests/unit/test_opentelemetry.py.User experience
Users can now pass an
OpenTelemetryProvidertoTracerto send trace data to OpenTelemetry backends: