Repository: snowflakedb/snowflake-connector-python

Issue:
SNOW-1763555: OpenTelemetry integration utterly breaks driver if using DataDog
### Python version

N/A

### Operating system and processor architecture

N/A

### Installed packages

```python
ddtrace
opentelemetry-api
```


### What did you do?

I used the driver w/ Datadog APM, which installs the opentelemetry packages that are used as the check to see if we can inject trace context.

With DataDog, this gets happens on every query, and it fails to run.

This is because the only exception caught is ModuleNotFoundError, and opentracing is throwing a ValueError which isn't getting caught.

(please forgive any typos, I copied the backtrace from a screenshot)

```python
ValueError: Propagator tracecontext not found. It is either misspelled or not installed.
  File "/var/task/lambda_function.py", line 116, in lambda_handler
    process_scan(
  File "/var/task/store.py", line 92, in process_scan
    upload_file_to_snowflake(cursor, secrets, file_name=sv_file, table=snowflake_table)
  File "/var/task/store.py", line 55, in upload_file_to_snowflake
    cursor.execute (
  File "/var/task/snowflake/connector/cursor.py", line 984, in execute
    ret = self._execute_helper(query, **kwargs)
  File "/var/task/snowflake/connector/cursor.py", line 695, in _execute_helper
    ret = self._connection.cmd_query(
  File "/var/task/snowflake/connector/connection.py", line 1350, in cd_query
    ret = self. rest. request(
  File "/var/task/snowflake/connector/network.py", line 483, in request
    from opentelemetry.propagate import inject
  File "<frozen importlib._bootstrap»", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap»", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap»", Line 690, in _load_unlocked
  File "/var/task/ddtrace/internal/module.py", line 295, in _exec_module
    self.loader.exec_module (module)
  File "/var/task/opentelemetry/propagate/__init__.py", line 149, in ‹module>
    raise ValueError(
```


### What did you expect to see?

I would expect that opentelemetry integration would be done via this- https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation

and not via a hack in the actual library that breaks the driver for anyone that has opentelemetry installed but not in-use.

I hope that this issue results in that change being reverted, and the original requestor of the feature gets this done properly in the open-telemetry project.

### Can you set logging to DEBUG and collect the logs?

```bash
We already have it at DEBUG and that's the problem.
```

Hints from issue comments:
@lattwood: This was directly caused by #1989@lattwood: Aside- what kind of commit title is `test`?

<img width="1067" alt="image" src="https://github.com/user-attachments/assets/df707602-0cb6-476e-a0e7-02f813c54c19">
@lattwood: `User has to import opentelemetry and configure it to have this. This code executes only if the import succeeds which requires user to import and configure opentelemetry.`

specifically this is the erroneous assumption, there are many ways the import can fail.@schammah: affected by this as well
any estimate to resolve?@fbexiga: Seems like a straightforward issue to fix... Can we get some eyes on this one?
@sfc-gh-mkeller @sfc-gh-yuwang @sfc-gh-aling @sfc-gh-sghosh @sfc-gh-yixie @sfc-gh-aalam@sfc-gh-dszmolka: thanks folks for drawing attention to this, we'll take a look.@sfc-gh-bdrutu: Thanks for your bug report, this is very valuable input. Unfortunately, after further investigation this proves to be an issue specific to DataDog because by just including a clean `opentelemetry-api` dependency does not reproduce this problem, which means that what they include is not the "full" opentelemetry-api (see the fact that in opentelemetry-api the tracecontext is available see [here](https://github.com/open-telemetry/opentelemetry-python/blob/a1191854ca69a8111e0d7b303b2c2c49245b83f8/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py#L25) and [here](https://github.com/open-telemetry/opentelemetry-python/blob/a1191854ca69a8111e0d7b303b2c2c49245b83f8/opentelemetry-api/pyproject.toml#L49) and [here](https://github.com/open-telemetry/opentelemetry-python/blob/a1191854ca69a8111e0d7b303b2c2c49245b83f8/opentelemetry-api/src/opentelemetry/propagate/__init__.py#L129)(here you can see the exception being thrown). 

This is still a problem that Snowflake can fix by catching all the exceptions there, though that is not the right way of writing code, since you cannot have a catch all exception every time you call any API that should not return an exception, but Snowflake will do it anyway since tracing should not break your data aplication.@lattwood: @sfc-gh-bdrutu Based on what you've said, I've taken the liberty of opening an issue on the ddtrace-py repository- https://github.com/DataDog/dd-trace-py/issues/11407@bogdandrutu: > I would expect that opentelemetry integration would be done via this- https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation
>
> and not via a hack in the actual library that breaks the driver for anyone that has opentelemetry installed but not in-use.
>
> I hope that this issue results in that change being reverted, and the original requestor of the feature gets this done properly in the open-telemetry project.

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.@PaulBormanTR: I'm facing this as well. Specifically, incompatible with https://github.com/DataDog/datadog-lambda-python/releases/tag/v6.99.0
Was this introduced by https://github.com/snowflakedb/snowflake-connector-python/commit/0d005199204a90082ec22b63008bb6e44500ec44? @lattwood: @bogdandrutu you literally have the same bio on your two accounts.

![Image](https://github.com/user-attachments/assets/66298bf9-6c75-4d2d-9463-d5ad9f71b03a)
![Image](https://github.com/user-attachments/assets/7eddfbd9-7820-4a5e-a47f-f87eb77aa097)@sfc-gh-mkeller: Since I was unable to reproduce the issue I'd really like someone with the issue to test drive the change I proposed in #2106 .
Please don't forget to let us know if it worked properly!@sfc-gh-dszmolka: merging the PR autoclosed this issue, however it needs a release first, so reopened until release is out.
Please folks, if you're able, test the fix (by installing the driver from `main`) and let us know how it worked for you. Thank you in advance !@sfc-gh-dszmolka: released in PythonConnector v3.12.4@lattwood: https://github.com/open-telemetry/opentelemetry-python/blob/415c94fb9834ce38459e58d5ee192a98494c4c76/opentelemetry-api/src/opentelemetry/context/__init__.py#L42-L64

We're getting a ton of log spam due to this.

```
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
```

Which it appears is due to datadog setting up a different context provider for if someone enables their opentelemetry integration.

Please implement opentelemetry in a way that it can be disabled with an environment variable, or put the functionality into a separate package.@sfc-gh-bdrutu: Hi @lattwood 

You should talk to the DD folks about this problem since our usage is a legit and correct usage of the opentelemetry-api.

> Which it appears is due to datadog setting up a different context provider for if someone enables their opentelemetry integration.

Talk to them maybe they shouldn't do that, or they should correctly do it.

> Please implement opentelemetry in a way that it can be disabled with an environment variable, or put the functionality into a separate package.

We are not "implementing" opentelemetry, we are only propagate the context to Snowflake and as mentioned our usage is correct and is DD causing your issue.

Solution patch:
diff --git a/DESCRIPTION.md b/DESCRIPTION.md
index 482c60239..9513fa71f 100644
--- a/DESCRIPTION.md
+++ b/DESCRIPTION.md
@@ -10,6 +10,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
 
 - v3.12.4(TBD)
   - Fixed a bug where multipart uploads to Azure would be missing their MD5 hashes.
+  - Fixed a bug where OpenTelemetry header injection would sometimes cause Exceptions to be thrown.
 
 - v3.12.3(October 25,2024)
   - Improved the error message for SSL-related issues to provide clearer guidance when an SSL error occurs.
diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py
index a00cc6588..18b28e061 100644
--- a/src/snowflake/connector/network.py
+++ b/src/snowflake/connector/network.py
@@ -481,11 +481,19 @@ def request(
             HTTP_HEADER_USER_AGENT: PYTHON_CONNECTOR_USER_AGENT,
         }
         try:
-            from opentelemetry.propagate import inject
+            # SNOW-1763555: inject OpenTelemetry headers if available specifically in WC3 format
+            #  into our request headers in case tracing is enabled. This should make sure that
+            #  our requests are accounted for properly if OpenTelemetry is used by users.
+            from opentelemetry.trace.propagation.tracecontext import (
+                TraceContextTextMapPropagator,
+            )
 
-            inject(headers)
-        except ModuleNotFoundError as e:
-            logger.debug(f"Opentelemtry otel injection failed because of: {e}")
+            TraceContextTextMapPropagator().inject(headers)
+        except Exception:
+            logger.debug(
+                "Opentelemtry otel injection failed",
+                exc_info=True,
+            )
         if self._connection.service_name:
             headers[HTTP_HEADER_SERVICE_NAME] = self._connection.service_name
         if method == "post":


Test patch:
diff --git a/test/unit/test_connection.py b/test/unit/test_connection.py
index 89398fd86..d3c0c3259 100644
--- a/test/unit/test_connection.py
+++ b/test/unit/test_connection.py
@@ -572,3 +572,19 @@ def test_ssl_error_hint(caplog):
         exc.value, OperationalError
     )
     assert "SSL error" in caplog.text and _CONNECTIVITY_ERR_MSG in caplog.text
+
+
+def test_otel_error_message(caplog, mock_post_requests):
+    """This test assumes that OpenTelemetry is not installed when tests are running."""
+    with mock.patch("snowflake.connector.network.SnowflakeRestful._post_request"):
+        with caplog.at_level(logging.DEBUG):
+            with fake_connector():
+                ...
+    assert caplog.records
+    important_records = [
+        record
+        for record in caplog.records
+        if "Opentelemtry otel injection failed" in record.message
+    ]
+    assert len(important_records) == 1
+    assert important_records[0].exc_text is not None