-
Notifications
You must be signed in to change notification settings - Fork 666
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
Write Guidelines for Python Packages Extending OTel Python Main Packages #1234
Comments
I think the reasoning behind
What exactly would be in your SDK extension package? Anything besides exporters, trace propagator, resource detectors, and ID generators? Or are you planning to ship your own vendor SDK that has significant modifications i.e. an alternative implementation? FWIW, the google cloud resource detector and trace propagator live in a package (in a separate repo) |
Having the custom propagator, like you mentioned, live in a vendor maintained repo is a totally valid way of hosting it according to results from the Maintainer's meeting in this comment which specifically calls out the Google propagator. However, that comment also says that vendor-specific implementations can be hosted by the OpenTelemetry community for the reasons in the specification. In this case I think this issue may only be talking about what the guidelines for extending the SDK should include, but the question of whether extending the SDK should be possible is already common place practice as mentioned by the specification blurb on propagators and the precedent of the Java and .NET (as of today) OTel repos which all host components which extend the SDK (I'm referring to AWS X-Ray extending the SDK specifically).
That's good insight! Either way the point is that they extend OpenTelemetry in some way or another. If I understand correctly, I think the goal of this issue would be to standardize it so that like you mentioned maybe
I don't believe that our use-case falls under the "alternative implementation" classification. We don't want to re-invent the wheel, we want to treat the "SDK Extension" package as a set of pre-built components that when plugged into and used in conjunction with the existing original OTel SDK, the SDK now does extra hidden behind-the-scenes work to give a user features they want but shouldn't have to worry about. These features the user wants is that their OpenTelemetry traces show up in their vendor specific backend service. But because vendors can be strict in the traces they receive, and the original SDK can't hope to satisfy the requirements of every vendor at the same time, a user should be able to easy import from an SDK extension of the OpenTelemetry main SDK and be set up to trace with the backend service of their choosing quickly and easily. That's why an
Yes, this includes what you mentioned. As an example, AWS X-Ray ID generators is something simple that they can pull in and create a Tracer object with, but without it, tracing to the AWS X-Ray service is completely impossible. The SDK already provides an interface to customize the way IDs are generated, so we want to provide the code necessary to let a user customize it without them having to write it themselves. If they depend on it, they don't have to worry about it going out of date because they just update to the latest version to get whatever they need to continue tracing with their favorite backend. Next would be a propagator, which again the user who traces with AWS X-Ray would want, but which the regular I actually think Exporters, Trace Propagators, Resource Detectors, and ID generators is already a lot and having it all in one package is beneficial to the user experience. But to further the point, we want to add components which extend the Instrumentor to be able to support Lambda uniquely, provide Resource plugins unique to Lambda, and potentially much more in the future. |
Co-authored-by: Mayur Kale <mayurkale@google.com>
This issue was marked stale due to lack of activity. It will be closed in 30 days. |
FYI I just opened a PEP about package repository namespaces and I used your packages/community as a motivating example 🙂 https://discuss.python.org/t/pep-752-package-repository-namespaces/61227 |
If you'd like, you can bring up this topic in the weekly Python SIG meeting. Check the calendar here: https://calendar.google.com/calendar/u/0/embed?src=c_2bf73e3b6b530da4babd444e72b76a6ad893a5c3f43cf40467abc7a9a897f977@group.calendar.google.com&pli=1 We meet Thursdays 9am PST. |
Sure, thanks! |
I missed the meeting this morning because we have an event going on at work this week but I was wondering what your organization thinks about this https://discuss.python.org/t/63192/60:
If your project is still in favor of the proposal, would one of you like to be a co-author to ensure that your situation is properly supported, if that's not already the case? Since the last time we spoke the policy/operational part has been extracted into a separate PEP: |
Following the discussion in #1205 where we identified that the
opentelemetry
(i.e. the opentelemetry-api package) and (soon) theopentelemetry-sdk
packages are setup as Namespace Packages, we decided there was a need for guidelines which explain how subsequent extension packages should extend these base packages.A Namespace Package allows other directories to extend the original namespace package by distributing packages under the same package name.
We already do this today with the
instrumentation
andexporter
packages, which extend the OTel Python API package.e.g.
opentelemetry.instrumentation.*
andopentelemetry.exporter.*
The guidelines should address how packages will extend the OTel Python SDK package and ensure that installing these extension packages don't disrupt the operation of the original package.
Regarding the proposal that OTel Python SDK package extensions should live under
opentelemetry.sdk.extension
, the following can be said:Pros:
instrumentation
andexporter
models we have todayCons:
Inconsistent with other languages who have more complicated package importing
The text was updated successfully, but these errors were encountered: