Skip to content
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

Callback on context change #3915

Open
xrmx opened this issue May 10, 2024 · 2 comments
Open

Callback on context change #3915

xrmx opened this issue May 10, 2024 · 2 comments

Comments

@xrmx
Copy link
Contributor

xrmx commented May 10, 2024

Why you need this feature?

Elastic is proposing a donation of our continuous profiler (open-telemetry/community#1918). In order to do correlation of profiling data with traces collected by language sdks we need to provide to the profiler current thread trace id, span id and root span id.

Describe the solution you'd like

Following java implementation I thought an hook on the context api may work, I've done a quick POC here:

https://gist.github.com/xrmx/fc81e710e7a860ca229839b10e780dfd

On the technical side I'm unsure if the optional hook callback should be configured via an environment variable or via an entrypoint.

Describe alternatives you've considered

An alternative if the context is the right place to hook would be to provide a custom context storage implementation but that would not be generic or upstreamable.

Additional context

In the java sdk they have https://javadoc.io/doc/io.opentelemetry/opentelemetry-context/latest/io/opentelemetry/context/ContextStorage.html#addWrapper(java.util.function.Function)

@lzchen
Copy link
Contributor

lzchen commented May 13, 2024

I think this is a cool feature to have but I will reiterate the concerns that I had during the previous SIG meeting. I think we have reservations against adding functionality/changes to the api in general if it is not defined/stable in the spec. Since Java SDK has this functionality, we maybe can explore the possibility of having this as an experimental feature that users can use to their discretion.

An alternative if the context is the right place to hook would be to provide a custom context storage implementation but that would not be generic or upstreamable.

I think that has been our recommendation in general for anyone who wants custom, specific behavior out of the api/sdk that is not currently supported in the default implementation. Yes the solutions are not generic or upstreamable but I believe that is the point: we do not want to so easily include custom implementations/contributions and mark them with the OpenTelemetry Python stamp of approval. Similar to instrumentations, we welcome contributions but we want to avoid contributors dropping their features/instrumentations but the burden of supportability rests on the maintainers/active members of the community once the contributor no longer has bandwidth to support it. However this is a case-by-case basis and I think this feature warrants some discussion and am curious as to what others think about it's usefulness in general.

@xrmx
Copy link
Contributor Author

xrmx commented May 14, 2024

If it wasn't clear here I'm just proposing a generic way to hook into context change, not anything specific to the profiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants