Repository navigation
Project Proposal: Add SDK wrapper #3744
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
base: main
Are you sure you want to change the base?
Changes from all commits
2bdeec8
4c2e55a
37bd18c
f8e78a1
5be7c93
4328c95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,7 @@ words: | |
| - Hendrik | ||
| - Hostmetrics | ||
| - Hrabusa | ||
| - Hurtado | ||
| - Instrgen | ||
| - Jaana | ||
| - Jhin | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # OpenTelemetry SDK Wrapper | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest we rename this file (if we decide to use my naming proposal) to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel strongly on the name and am happy to let others weigh in here, will rename the file appropriately once the name is agreed. |
||
|
|
||
| ## Background and description | ||
|
codeboten marked this conversation as resolved.
|
||
|
|
||
| This project started as an investigation to mitigate some issues identified around memory allocations and global interpreter lock (GIL) contention in Python. As a result of testing an implementation that binds [Python to a C++ SDK](https://github.com/open-telemetry/community/issues/3483), a potential reduction in heap allocations by 10-15% and as much as 40% was observed. | ||
|
|
||
| The project proposed here provides the performance benefits of an underlying SDK across Python initially and possibly other languages in the future. A secondary benefit, though not a goal, would be to allow end users and API implementers to experiment with features already supported in the underlying SDK, that may not yet be implemented in a native SDK. | ||
|
|
||
| ### Current challenges | ||
|
|
||
| Reference implementation SDKs in every language offer the flexibility that includes every feature supported by OpenTelemetry. In some implementations, this comes at the cost of: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is another name for native SDKs which is already being used elsewhere in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am having a bit of a hard time understanding what you mean here. What do you mean with an SDK offering "flexibility"? |
||
|
|
||
| * increased resource consumption (memory usage, artifact size) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using OpenTelemetry regardless of the language increases resource consumption. I think what you mean here is that there are certain implementations of OpenTelemetry that significantly increase resource consumption, and those implementations are the ones who can benefit the most from the approach proposed in this project. |
||
| * added complexity | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean here? |
||
|
|
||
| * in Python specifically, the contention with the global interpreter lock which may cause instrumented applications to introduce latency into the application | ||
|
|
||
| This impacts end users by increasing the overhead required to use OpenTelemetry out of the box. | ||
|
|
||
| ### Goals, objectives, and requirements | ||
|
|
||
| The OTel community is currently working across many languages to support reference implementations of all the features in every language. We have always proposed that the API/SDK separation would make it possible for end-users to swap SDKs without having to re-instrument their application/instrumentations code. The proposal for this project is to leverage that model as well as existing OpenTelemetry component to provide alternative SDKs for specific use-cases. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think what you are trying to say here is that the OTel community is currently working to add all the OpenTelemetry features to every native SDK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We have always promised that to our users. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think the proposal for this project is something else. What I think you want to say here is that this project will use the existing separation between API and SDK to allow end users to use alternative SDKs for specific purposes. |
||
|
|
||
| An objective is to prove this approach as part of the project. A requirement of the project is that the performance improvements be worth the investment of time to produce and maintain artifacts moving forward. If the investigations from the project finds that the performance improvements are only applicable to one language or the improvements are negligible, then the deliverables may be changed. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An objective is to prove this approach. A requirement is that the performance improventes are worth the investment. This seems contradictory or at least hard to understand, an objective is to test this approach and a requirement is that the approach is worth the investment? |
||
|
|
||
| A non-goal of this project is to replace existing SDK implementations. There are always different use-cases for end users and a native implementation that offers flexibility is likely to always be needed. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean with flexibility here? I agree that this project is not trying to replace existing SDK implementations, but I think the reason is that there is no certainty that a refitted SDK can satisfy every use case existing SDK implementations can because we have not tested all those use cases yet with a refitted SDK.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance is an optimization. If we are optimizing for higher performance and lower resource consumption, that will be better achieved if this project does not attempt to provide all of the plugins, hooks, and configuration options currently available in the existing SDKs. For example, it may be the case that this implementation only provides OTLP via Proto/HTTP as the sole exporter option, in order to reduce file size and optimize container start up time. I would suggest adding details to this section to clarify that performance is an optimization goal, and this implementation will makes trade-offs to achieve that. The other important form of flexibility is supporting build/deployment environments where our C++ binding mechanism doesn't work or otherwise creates a lot of trouble.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another form of flexibility is "mix-n-match." For example, installing the OTel tracing SDK, but binding to Prometheus for metrics and not installing a logging SDK at all. This implementation could be all-or-nothing, in order to avoid the complexity that comes with offering those options. |
||
|
|
||
| ## Deliverables | ||
|
|
||
| There are a few different approaches available to solve the issue. Phase 1 of the project will provide: | ||
|
|
||
| * a library or artifact that can be used behind an existing language API. The initial proposal is to support at least Python, but support for other languages is not out of scope. | ||
|
codeboten marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should also take look at OBI https://github.com/open-telemetry/opentelemetry-ebpf-instrumentation as one of the possible solutions. It can already be used with Python. I think it is worth mentioning that it can be already used as an SDK for users of OTel Go API (see: https://opentelemetry.io/docs/zero-code/obi/distributed-traces/#instrument-applications-that-use-the-go-trace-api), similar feature is planned for other languages. We already have issues like:
This was initially brought up by @lmolkova 🙏
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely worth investigating! Especially if the goal is to support the same instrumentation we already have, and not to try and create a duplicate set of instrumentation using ebpf targets. |
||
| * benchmarks to compare the existing native SDK with the proposed alternative artifact | ||
| * documentation of the opinions and limitations of what this artifact supports, including decisions made to not support certain protocols or features of the native SDK | ||
|
|
||
|
|
||
| ## Staffing / Help Wanted | ||
|
|
||
| ### Industry outreach (Optional) | ||
|
|
||
| The following vendors are interested in improving this area: | ||
|
|
||
| * Dash0 (@ocelotl) | ||
| * Grafana Labs (@codeboten) | ||
| * Honeycomb (@MikeGoldsmith) | ||
|
|
||
| ### SIG | ||
|
|
||
| New SIG needed with a new name | ||
|
|
||
| ### Required staffing | ||
|
|
||
| #### Project Leads(s) | ||
|
|
||
| * Alex Boten (@codeboten) | ||
| * Diego Hurtado (@ocelotl) | ||
|
|
||
| #### Other Staffing | ||
|
|
||
| Engagement with the backing SDK is expected, level of involvement TBD. | ||
|
|
||
| ### Sponsorship | ||
| See [Project Sponsorship](/project-management.md#project-sponsorship) | ||
|
|
||
| #### TC Sponsor | ||
|
|
||
| Alex Boten (@codeboten) | ||
|
codeboten marked this conversation as resolved.
|
||
| Reiley Yang (@reyang) | ||
|
|
||
| #### GC Liaison | ||
|
|
||
| TODO | ||
|
|
||
| ## Expected Timeline | ||
|
|
||
| * Months 1-3: investigate various approaches to solving the problem and produce benchmarks for each comparing the performance in Python, Ruby, and Node | ||
| * Months 4-12: produce artifacts capable of providing users with alternative implementations | ||
|
|
||
| ## Labels (Optional) | ||
|
|
||
| TODO | ||
|
|
||
| ## GitHub Project (Post-Approval) | ||
|
|
||
| TODO: add project link | ||
|
|
||
| ## SIG Meetings, Roadmap, and Other Info (Post-Approval) | ||
|
|
||
| TODO: add details here | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the idea you have of this project is to "wrap" the C++ SDK with Python bindings, but I think "wrapper" is not specific enough.
Wrapping an SDK is the technique this project will use and wrapping is a technique used all over software for all kinds of purposes.
What is specific to this project is that wrapping an SDK implemented in language B with bindings for language A creates a relationship between SDK A and SDK B: the implementation of SDK A is replaced by SDK B, while the API of A stays exactly as it is for its users.
That relationship will come up again and again, for other components and other languages, so we will need to name the individual cases. If we use the word "wrapper" then the best we can say is "the Python TracerProvider wrapper is feature complete" or "the Node OTLPTraceExporter wrapper is waiting on async exporter support".
Neither of those sentences tells the reader that the Python or Node API for those components is unchanged or that the underlying implementation is not written in Python or Node respectively. The only thing the reader gets is that some components are being wrapped.
On the other hand, with a term of our own, "the Python TracerProvider
new_termis feature complete" and "the Node OTLPTraceExporternew_termis waiting on async exporter support" tell the reader exactly what the previous two sentences could not: the Python and Node APIs for those components are unchanged, and the implementation underneath each one is not written in Python or Node.I think we would benefit from using a new term to describe this project, and I have proposed one here. If we use that proposal, then the title becomes:
Of course, the definition of refit should follow immediately so the reader can understand what we mean by "refit" in the title.
Coining a term is not unusual here. OpenTelemetry has done this before, both to replace a word that was ambiguous and to name something new:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the specific goal of this project is "C++ Bindings for Python" then that should be the name of the project. That will help people looking at the calendar, scanning through the project lists, etc. Neither "refit" or "wrapper" really describe what the specific goal is.