DataDog/dd-trace-py

Python Snowflake Driver Team says You're Importing OpenTelemetry wrong

Closed this issue · 9 comments

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!

@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.

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

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.

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

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.

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

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.

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

@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"