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

Python Snowflake Driver Team says You're Importing OpenTelemetry wrong #11407

Closed
lattwood opened this issue Nov 14, 2024 · 9 comments
Closed
Assignees

Comments

@lattwood
Copy link

Python Snowflake Driver Team says you're including opentelemetry-api wrong.

I think they're the ones that are wrong, by building opentelemetry into their connector, instead of getting their instrumentation landed here: https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation

and in an ideal world, in this repo as well.

Would ya'll mind sorting this out?

snowflakedb/snowflake-connector-python#2084

Thanks!

@bogdandrutu
Copy link

bogdandrutu commented Nov 14, 2024

@lattwood I don't think Snowflake is wrong. Based on my experience with OpenTelemetry in different environments, OpenTelemetry was designed with flexibility in mind, supporting scenarios where telemetry is directly embedded within libraries, rather than requiring separate instrumentation libraries.

@lattwood
Copy link
Author

lattwood commented Nov 14, 2024

of course you don't, you work for Snowflake, and you're posting from your personal github account rather than @sfc-gh-bdrutu.

Image
Image

@keller00
Copy link

I think the point wasn't to appear as not a Snowflake employee. I think Bogdan used his personal account because he was making a post outside of normal work hours.

@lattwood
Copy link
Author

@keller00 then why refer to "Snowflake" rather than "I/we"? It's disingenuous at best and intentionally misleading at worst.

@bogdandrutu
Copy link

bogdandrutu commented Nov 14, 2024

of course you don't, you work for Snowflake, and you're posting from your personal github account rather than @sfc-gh-bdrutu.

@lattwood I am doing this because @bogdandrutu may know something about OpenTelemetry, in case you did not trust what @sfc-gh-bdrutu said.

Also https://github.com/bogdandrutu says that I do work for Snowflake, so you don't need to make guesses and correlations.

@mabdinur
Copy link
Contributor

The ddtrace library does not do anything unorthodox. We simply implement the opentelemetry context api and use the default implementation for all other components. ddtrace installs the opentelemetry-api but only configures support if DD_TRACE_OTEL_ENABLED=true is set (which honestly doesn't matter for this bug).

I think this issue should be resolved in the snowflake-connector-python. The fix looks good to me: snowflakedb/snowflake-connector-python#2106.

@mabdinur mabdinur self-assigned this Nov 14, 2024
@bogdandrutu
Copy link

bogdandrutu commented Nov 14, 2024

@mabdinur

Your "unorthodox" part is that you remove (somehow, not an expert in Python) the opentelemetry_propagator from the entry-points, which makes inject to throw an exception, but if someone uses directly the opentelemetry-api that does not throw.

Indeed the fix in snowflake-connector-python will work for this case, but DataDog should not make the inject to throw if it doesn't throw when used as a direct dependency.

@mabdinur
Copy link
Contributor

ddtrace only replaces the default opentelemetry_context entrypoint. All other entrypoints (ex: propagator entrypoint) should remain unaffected.

The ValueError will be raised if OTEL_PROPAGATORS environment variable is set and contains a value that is not baggage or tracecontext. This is expected

@lattwood
Copy link
Author

lattwood commented Dec 4, 2024

@mabdinur so now we're seeing another fun error.

Failed to load context: contextvars_context, fallback to contextvars_context
Traceback (most recent call last):
  File "/var/task/opentelemetry/context/__init__.py", line 43, in _load_runtime_context
    return next(  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^
StopIteration

Due to this code- https://github.com/open-telemetry/opentelemetry-python/blob/415c94fb9834ce38459e58d5ee192a98494c4c76/opentelemetry-api/src/opentelemetry/context/__init__.py#L40-L64

I think OTEL_PYTHON_CONTEXT environment variable needs to be set to ddcontextvars_context?

ddcontextvars_context = "ddtrace.internal.opentelemetry.context:DDRuntimeContext"

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

4 participants