-
Notifications
You must be signed in to change notification settings - Fork 402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Support for external observability providers - Tracer #2030
Comments
Thanks for opening your first issue here! We'll come back to you as soon as we can. |
got dragged in meetings and will reply to you properly tomorrow. It'll be along these lines I posted on Metrics: #2015 (comment)
|
Thank you @heitorlessa
I see how it can be a bit confusing to the customers. I have revised the RFC and tried to make it simple by removing the |
I underestimated how much feedback I needed to write for Metrics RFC so I ran out of time (apologies!) - I've blocked time on Monday afternoon (morning PST) to go through this. At a quick glance, the piece I'm missing in the contract is |
hey @Vandita2020 that's a great start!! I took liberty to address some low hanging fruits and made a list of suggestions similar to what I made to Metrics. There are some minor changes like using the Changes
Asks
|
Hey @heitorlessa, Thanks so much for the review. I have made the changes in the RFC accordingly, also for couple of comments, I have provided come comments/context below.
Currently async methods are handled using
I have corrected it, earlier I was using
For threading, the concept that I used is that the BaseProvider class needs to implement methods to create and manage thread-local storage for each trace. When a new child thread is created, the trace context can be copied to the new thread-local storage, allowing the new thread to continue the trace without interfering with the parent thread’s trace. When the thread finishes, the trace context can be removed from the thread-local storage. |
Great!! As for threading, before you dive in the implementation per se, take a look at how DataDog, NewRelic, and OpenTelemetry Tracers handle threading + asyncio. Reason I ask that is that we wouldn't necessarily need to implement threadlocal or contextvars, because the Provider would be a wrapper on top of the actual implementation (e.g., DataDog Tracer, OpenTelemetry Tracer, etc.). What's missing in the RFC is a section with a comparison on how observability providers out there handle threading. Then call out whether we need additional public methods in the BaseProvider that will be implemented by the actual provider who already handle the threadlocal or contextvars. As this is a complex topic, to recap, our Tracer Observability Provider is a thin wrapper on top of the actual Observability Provider SDK (e.g., DataDog SDK, OpenTelemetry SDK, New Relic SDK, etc.). This helps customers use the same Powertools DX across Providers, and each provider simply implement our interface while bringing their various flavours of SDK already handling this use case. Please do let me know if I can make this any clearer. Tks a lot!! |
Hi @heitorlessa! I'm taking over this issue and trying to catch up on the current process. From what I perceive the current pending item is investigate "How OPTL, new relic,DD handle threading in tracing? - related to issue 2047". Please help me to confirm and remind me what else should I take a look. Thank you! |
Yup, that’s correct!
…On Fri, 5 May 2023 at 21:58, Roger Zhang ***@***.***> wrote:
Hi @heitorlessa <https://github.com/heitorlessa>! I'm taking over this
issue and trying to catch up on the current process. From what I perceive
the current pending item is investigate "How OPTL, new relic,DD handle
threading in tracing? - related to issue 2047". Please help me to confirm
and remind me what else should I take a look.
—
Reply to this email directly, view it on GitHub
<#2030 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBDB3HVQ5X3ARWIJGXTXEVLVPANCNFSM6AAAAAAWDDLERQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hello everyone! I'm updating this issue with some decisions we made regard the Tracer provider NamingWe decide the follow the naming convention of OpenTelemetry tracing. Comparing to the current provider:
signature of
|
@abc.abstractmethod | |
def set_attribute(self, key: str, value: Any, **kwargs) -> None: | |
"""Set an attribute for a span with a key-value pair. | |
Parameters |
We decided to accept Any
value here, but the actual supported data type will be decided in the specific provider
Deprecation of current BaseSegment
and BaseProvider
These two classes will remain for backwards compatibility until Powertools V3
docstring change in the current Subsegment.close
:
We come to the conclusion that using float in epoch seconds in this case is more appropriate. I also created a aws/aws-xray-sdk-python#424 for this in X-Ray python SDK as I believe the docstring here is referencing the X-Ray Python SDK.
Backwards compatibility of X-Ray provider
We support all Backwards compatibility except escape hatch usage directly on X-Ray recorder. For example:
tracer = Tracer()
tracer.provider.capture('subsegment_name')
def myfunc():
# Do something here
We didn't have capture
function in the current BaseProvider
thus this behavior will not be supported in the new provider. For existing function in the current BaseProvider
like in_subsegment
, they will still be supported.
Is this related to an existing feature request or issue?
Issue: #1433
Logger RFC: #2014
Metrics RFC: #2015
Which AWS Lambda Powertools utility does this relate to?
Tracer
Summary
This RFC is one of the three that defines the format when setting up loggers, metrics and traces for better integration with other observability providers.
This RFC is specifically for the Tracer. Currently, we have undocumented BaseProvider for Tracer, but we need to decide more on what minimum features the
BaseProvider
should support. The RFC discusses on the features that could be a part of custom tracer for users to integrate other Observability providers easily.Use case
The use case for this utility would be for developers who want to use other observability providers to trace their application, other than AWS X-Ray.
Proposal
Current tracer experience
The Powertools’ tracer utility is essentially a wrapper for the AWS X-Ray SDK. Some key features of this utility include auto capturing cold start as annotation, auto capturing responses or full exceptions as metadata, and auto-disabling when not running in AWS Lambda environment. Tracer also auto patches supported modules by AWS X-Ray.
JSON output
Tracer proposal
We propose a new parameter to the existing tracer utility that developers can use to specify which observability provider they would like their traces to be pushed to. The below code snippet is a rudimentary look at how this utility can be used and how it will function. Out of the box, we will support DataDog. Other providers TBD
JSON output
Bring your own provider
If you would like to use an observability provider not supported out of the box, or define their own tracer functions, we will define an interface that the customer can implement and pass in to the Tracer class.
Example
The five methods defined above are a combination of methods that already exist in the
BaseProvider
and the ones that are most common in other observability providers.The current
BaseProvider
does support most of the features used in the major observability providers. There are couple of differences I noticed while researching through the other Observability providers.Out of scope
Sending traces from Powertools to the customer's desired observability platform will not be in the scope of this project. The implementation should only support modifying the output of the Tracer so that the customer can push them to their platform of choice.
Potential challenges
We need to determine which platforms we want to support out-of-the-box (apart from Datadog).
Dependencies and Integrations
We will have to integrate with (and thus, have a dependency on) Datadog and any other platforms we decide to support out-of-the-box.
Alternative solutions
No response
Acknowledgment
The text was updated successfully, but these errors were encountered: