From 9037d24b0ccaa9e6c32610c44d0218e0c0431772 Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 10:40:26 -0800 Subject: [PATCH 1/6] update how HTTP headers are requested from OpenTelemetry --- src/snowflake/connector/network.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py index a00cc6588..97201b1c4 100644 --- a/src/snowflake/connector/network.py +++ b/src/snowflake/connector/network.py @@ -481,9 +481,12 @@ 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.propagate import TraceContextTextMapPropagator - inject(headers) + TraceContextTextMapPropagator().inject(headers) except ModuleNotFoundError as e: logger.debug(f"Opentelemtry otel injection failed because of: {e}") if self._connection.service_name: From e8f5d83dde160ecdc7950d14f55c12e9e1423b62 Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 10:53:15 -0800 Subject: [PATCH 2/6] changelog entry --- DESCRIPTION.md | 1 + 1 file changed, 1 insertion(+) 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. From 58b108bb7409a5586a466a761a639ca690e72d09 Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 11:00:05 -0800 Subject: [PATCH 3/6] widen Exception handling --- src/snowflake/connector/network.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py index 97201b1c4..94a2ee993 100644 --- a/src/snowflake/connector/network.py +++ b/src/snowflake/connector/network.py @@ -487,8 +487,11 @@ def request( from opentelemetry.propagate import TraceContextTextMapPropagator TraceContextTextMapPropagator().inject(headers) - except ModuleNotFoundError as e: - logger.debug(f"Opentelemtry otel injection failed because of: {e}") + 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": From ad43b134ba7cb0dfd6cbd8f48c97a59467cf3d9e Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 11:32:37 -0800 Subject: [PATCH 4/6] Update src/snowflake/connector/network.py Co-authored-by: Bogdan Drutu --- src/snowflake/connector/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py index 94a2ee993..de4d503dc 100644 --- a/src/snowflake/connector/network.py +++ b/src/snowflake/connector/network.py @@ -484,7 +484,7 @@ def request( # 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.propagate import TraceContextTextMapPropagator + from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator TraceContextTextMapPropagator().inject(headers) except Exception: From df09326b16465cfb6d1f4c48136ef5e455e23f1b Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 13:18:18 -0800 Subject: [PATCH 5/6] adding new unit test --- test/unit/test_connection.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 From bd956353fdd6d9476e714d5b94fe00dc3bf2a07c Mon Sep 17 00:00:00 2001 From: Mark Keller Date: Thu, 14 Nov 2024 13:29:23 -0800 Subject: [PATCH 6/6] fix linting issue --- src/snowflake/connector/network.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/snowflake/connector/network.py b/src/snowflake/connector/network.py index de4d503dc..18b28e061 100644 --- a/src/snowflake/connector/network.py +++ b/src/snowflake/connector/network.py @@ -484,7 +484,9 @@ def request( # 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 + from opentelemetry.trace.propagation.tracecontext import ( + TraceContextTextMapPropagator, + ) TraceContextTextMapPropagator().inject(headers) except Exception: