From a960fc496730fc560c2fbfbb52b9a26e8f421b3b Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Wed, 6 Sep 2023 15:30:49 +0300 Subject: [PATCH 01/20] fix: pip install failed on mac --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 93c24a3f..b11db86c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ pre-commit==2.20.0 psutil==5.9.1 pytest==7.1.1 pytest-cov==3.0.0 -pyyaml==6.0 +pyyaml==6.0.1 requests==2.27.1 types-attrs==19.1.0 types-boto==2.49.17 From a063aa7b639a5630577a9f181c48eda8602a2abf Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 13:30:14 +0300 Subject: [PATCH 02/20] feat: do not export empty sqs polling spans --- src/lumigo_opentelemetry/__init__.py | 5 ++- .../botocore/parsers/__init__.py | 19 ++++++++++ .../libs/environment_variables.py | 1 + .../libs/general_utils.py | 13 +++++++ .../resources/span_processor.py | 37 +++++++++++++++++++ 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 src/lumigo_opentelemetry/libs/environment_variables.py create mode 100644 src/lumigo_opentelemetry/resources/span_processor.py diff --git a/src/lumigo_opentelemetry/__init__.py b/src/lumigo_opentelemetry/__init__.py index ae77e231..5c52bb84 100644 --- a/src/lumigo_opentelemetry/__init__.py +++ b/src/lumigo_opentelemetry/__init__.py @@ -89,7 +89,8 @@ def init() -> Dict[str, Any]: from opentelemetry import trace from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter from opentelemetry.sdk.trace import SpanLimits, TracerProvider - from opentelemetry.sdk.trace.export import BatchSpanProcessor + + from lumigo_opentelemetry.resources.span_processor import LumigoSpanProcessor DEFAULT_LUMIGO_ENDPOINT = ( "https://ga-otlp.lumigo-tracer-edge.golumigo.com/v1/traces" @@ -125,7 +126,7 @@ def init() -> Dict[str, Any]: if lumigo_token: tracer_provider.add_span_processor( - BatchSpanProcessor( + LumigoSpanProcessor( OTLPSpanExporter( endpoint=lumigo_endpoint, headers={"Authorization": f"LumigoToken {lumigo_token}"}, diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 9b3d8171..959a9b8b 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -12,6 +12,10 @@ extract_region_from_arn, get_resource_fullname, ) +from lumigo_opentelemetry.resources.span_processor import set_span_no_export +from lumigo_opentelemetry import logger +from lumigo_opentelemetry.libs.general_utils import get_boolean_env_var +from lumigo_opentelemetry.libs.environment_variables import AUTO_FILTER_EMPTY_SQS class AwsParser: @@ -133,6 +137,15 @@ def parse_request( } span.set_attributes(attributes) + @staticmethod + def _should_skip_empty_sqs_polling_response(operation_name: str, result: Dict[Any, Any]) -> bool: + """ + checks the sqs response & returns true if the request receive messages from SQS but no messages were returned + """ + + empty_sqs_poll = operation_name == 'ReceiveMessage' and 'Messages' not in result + return empty_sqs_poll and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) + @staticmethod def parse_response( span: Span, service_name: str, operation_name: str, result: Dict[Any, Any] @@ -145,6 +158,12 @@ def parse_response( {"lumigoData": json.dumps({"trigger": trigger_details})} ) + # Filter out sqs polls with empty response + if SqsParser._should_skip_empty_sqs_polling_response(operation_name, result): + logger.debug('Not tracing empty SQS polling requests ' + f'(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)') + set_span_no_export(span) + class LambdaParser(AwsParser): @staticmethod diff --git a/src/lumigo_opentelemetry/libs/environment_variables.py b/src/lumigo_opentelemetry/libs/environment_variables.py new file mode 100644 index 00000000..a63efc4a --- /dev/null +++ b/src/lumigo_opentelemetry/libs/environment_variables.py @@ -0,0 +1 @@ +AUTO_FILTER_EMPTY_SQS = 'AUTO_FILTER_EMPTY_SQS' diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index 6cfa9f65..903e2514 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -70,3 +70,16 @@ def get_max_size() -> int: os.environ.get(OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, DEFAULT_MAX_ENTRY_SIZE), ) ) + + +def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: + """ + This function return the boolean value of the given environment variable. + If this values doesn't exist, return default. + + @param env_var_name: The env var to get (case-sensitive) + @param default: Default value if env var is not set + @return: The boolean value of the env var + """ + + return os.environ.get(env_var_name, str(default)).lower() == "true" diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py new file mode 100644 index 00000000..0881da9a --- /dev/null +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -0,0 +1,37 @@ +from opentelemetry.trace import Span +from opentelemetry.sdk.trace import ReadableSpan +from opentelemetry.sdk.trace.export import BatchSpanProcessor + +from lumigo_opentelemetry import logger + + +class LumigoSpanProcessor(BatchSpanProcessor): + + def on_end(self, span: ReadableSpan) -> None: + if should_not_export_span(span): + logger.debug('Not exporting span because it has NO_EXPORT=True attribute') + return + + return super().on_end(span) + + +def should_not_export_span(span: ReadableSpan) -> bool: + """ + Given a span, returns an answer if the span should be exported or not. + @param span: A readable span to check + @return: True if the span should not be exported, False otherwise + """ + return span.attributes.get('NO_EXPORT') is True + + +def set_span_no_export(span: Span, no_export: bool = True): + """ + marks the span as a span not intended for export (for example in spans that create a lot of noise and customers + do not want to trace) + + @param span: The span to mark (The span is altered in place) + @param no_export: Should the span be exported or not (default is True, the span will not be exported) + @return: + """ + + span.set_attributes({'NO_EXPORT': no_export}) From 7b8613fd9d14be2c13a31f2495b0eeab8b6deeb7 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 13:31:52 +0300 Subject: [PATCH 03/20] chore: auto format code --- .../instrumentations/botocore/parsers/__init__.py | 12 ++++++++---- .../libs/environment_variables.py | 2 +- src/lumigo_opentelemetry/resources/span_processor.py | 11 +++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 959a9b8b..437f0e9f 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -138,12 +138,14 @@ def parse_request( span.set_attributes(attributes) @staticmethod - def _should_skip_empty_sqs_polling_response(operation_name: str, result: Dict[Any, Any]) -> bool: + def _should_skip_empty_sqs_polling_response( + operation_name: str, result: Dict[Any, Any] + ) -> bool: """ checks the sqs response & returns true if the request receive messages from SQS but no messages were returned """ - empty_sqs_poll = operation_name == 'ReceiveMessage' and 'Messages' not in result + empty_sqs_poll = operation_name == "ReceiveMessage" and "Messages" not in result return empty_sqs_poll and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) @staticmethod @@ -160,8 +162,10 @@ def parse_response( # Filter out sqs polls with empty response if SqsParser._should_skip_empty_sqs_polling_response(operation_name, result): - logger.debug('Not tracing empty SQS polling requests ' - f'(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)') + logger.debug( + "Not tracing empty SQS polling requests " + f"(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)" + ) set_span_no_export(span) diff --git a/src/lumigo_opentelemetry/libs/environment_variables.py b/src/lumigo_opentelemetry/libs/environment_variables.py index a63efc4a..6172c0b6 100644 --- a/src/lumigo_opentelemetry/libs/environment_variables.py +++ b/src/lumigo_opentelemetry/libs/environment_variables.py @@ -1 +1 @@ -AUTO_FILTER_EMPTY_SQS = 'AUTO_FILTER_EMPTY_SQS' +AUTO_FILTER_EMPTY_SQS = "AUTO_FILTER_EMPTY_SQS" diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py index 0881da9a..f804ea49 100644 --- a/src/lumigo_opentelemetry/resources/span_processor.py +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -6,13 +6,12 @@ class LumigoSpanProcessor(BatchSpanProcessor): - def on_end(self, span: ReadableSpan) -> None: if should_not_export_span(span): - logger.debug('Not exporting span because it has NO_EXPORT=True attribute') + logger.debug("Not exporting span because it has NO_EXPORT=True attribute") return - return super().on_end(span) + super().on_end(span) def should_not_export_span(span: ReadableSpan) -> bool: @@ -21,10 +20,10 @@ def should_not_export_span(span: ReadableSpan) -> bool: @param span: A readable span to check @return: True if the span should not be exported, False otherwise """ - return span.attributes.get('NO_EXPORT') is True + return span.attributes.get("NO_EXPORT") is True -def set_span_no_export(span: Span, no_export: bool = True): +def set_span_no_export(span: Span, no_export: bool = True) -> None: """ marks the span as a span not intended for export (for example in spans that create a lot of noise and customers do not want to trace) @@ -34,4 +33,4 @@ def set_span_no_export(span: Span, no_export: bool = True): @return: """ - span.set_attributes({'NO_EXPORT': no_export}) + span.set_attributes({"NO_EXPORT": no_export}) From 8aaa755266802a73b5247f138880badc111a8b22 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Wed, 6 Sep 2023 15:30:49 +0300 Subject: [PATCH 04/20] fix: pip install failed on mac --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 93c24a3f..b11db86c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ pre-commit==2.20.0 psutil==5.9.1 pytest==7.1.1 pytest-cov==3.0.0 -pyyaml==6.0 +pyyaml==6.0.1 requests==2.27.1 types-attrs==19.1.0 types-boto==2.49.17 From 3ab36de546f34ed6fc2a7294a157410aec803d61 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 13:30:14 +0300 Subject: [PATCH 05/20] feat: do not export empty sqs polling spans --- src/lumigo_opentelemetry/__init__.py | 5 ++- .../botocore/parsers/__init__.py | 19 ++++++++++ .../libs/environment_variables.py | 1 + .../libs/general_utils.py | 13 +++++++ .../resources/span_processor.py | 37 +++++++++++++++++++ 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 src/lumigo_opentelemetry/libs/environment_variables.py create mode 100644 src/lumigo_opentelemetry/resources/span_processor.py diff --git a/src/lumigo_opentelemetry/__init__.py b/src/lumigo_opentelemetry/__init__.py index ae77e231..5c52bb84 100644 --- a/src/lumigo_opentelemetry/__init__.py +++ b/src/lumigo_opentelemetry/__init__.py @@ -89,7 +89,8 @@ def init() -> Dict[str, Any]: from opentelemetry import trace from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter from opentelemetry.sdk.trace import SpanLimits, TracerProvider - from opentelemetry.sdk.trace.export import BatchSpanProcessor + + from lumigo_opentelemetry.resources.span_processor import LumigoSpanProcessor DEFAULT_LUMIGO_ENDPOINT = ( "https://ga-otlp.lumigo-tracer-edge.golumigo.com/v1/traces" @@ -125,7 +126,7 @@ def init() -> Dict[str, Any]: if lumigo_token: tracer_provider.add_span_processor( - BatchSpanProcessor( + LumigoSpanProcessor( OTLPSpanExporter( endpoint=lumigo_endpoint, headers={"Authorization": f"LumigoToken {lumigo_token}"}, diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 9b3d8171..959a9b8b 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -12,6 +12,10 @@ extract_region_from_arn, get_resource_fullname, ) +from lumigo_opentelemetry.resources.span_processor import set_span_no_export +from lumigo_opentelemetry import logger +from lumigo_opentelemetry.libs.general_utils import get_boolean_env_var +from lumigo_opentelemetry.libs.environment_variables import AUTO_FILTER_EMPTY_SQS class AwsParser: @@ -133,6 +137,15 @@ def parse_request( } span.set_attributes(attributes) + @staticmethod + def _should_skip_empty_sqs_polling_response(operation_name: str, result: Dict[Any, Any]) -> bool: + """ + checks the sqs response & returns true if the request receive messages from SQS but no messages were returned + """ + + empty_sqs_poll = operation_name == 'ReceiveMessage' and 'Messages' not in result + return empty_sqs_poll and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) + @staticmethod def parse_response( span: Span, service_name: str, operation_name: str, result: Dict[Any, Any] @@ -145,6 +158,12 @@ def parse_response( {"lumigoData": json.dumps({"trigger": trigger_details})} ) + # Filter out sqs polls with empty response + if SqsParser._should_skip_empty_sqs_polling_response(operation_name, result): + logger.debug('Not tracing empty SQS polling requests ' + f'(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)') + set_span_no_export(span) + class LambdaParser(AwsParser): @staticmethod diff --git a/src/lumigo_opentelemetry/libs/environment_variables.py b/src/lumigo_opentelemetry/libs/environment_variables.py new file mode 100644 index 00000000..a63efc4a --- /dev/null +++ b/src/lumigo_opentelemetry/libs/environment_variables.py @@ -0,0 +1 @@ +AUTO_FILTER_EMPTY_SQS = 'AUTO_FILTER_EMPTY_SQS' diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index 6cfa9f65..903e2514 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -70,3 +70,16 @@ def get_max_size() -> int: os.environ.get(OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, DEFAULT_MAX_ENTRY_SIZE), ) ) + + +def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: + """ + This function return the boolean value of the given environment variable. + If this values doesn't exist, return default. + + @param env_var_name: The env var to get (case-sensitive) + @param default: Default value if env var is not set + @return: The boolean value of the env var + """ + + return os.environ.get(env_var_name, str(default)).lower() == "true" diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py new file mode 100644 index 00000000..0881da9a --- /dev/null +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -0,0 +1,37 @@ +from opentelemetry.trace import Span +from opentelemetry.sdk.trace import ReadableSpan +from opentelemetry.sdk.trace.export import BatchSpanProcessor + +from lumigo_opentelemetry import logger + + +class LumigoSpanProcessor(BatchSpanProcessor): + + def on_end(self, span: ReadableSpan) -> None: + if should_not_export_span(span): + logger.debug('Not exporting span because it has NO_EXPORT=True attribute') + return + + return super().on_end(span) + + +def should_not_export_span(span: ReadableSpan) -> bool: + """ + Given a span, returns an answer if the span should be exported or not. + @param span: A readable span to check + @return: True if the span should not be exported, False otherwise + """ + return span.attributes.get('NO_EXPORT') is True + + +def set_span_no_export(span: Span, no_export: bool = True): + """ + marks the span as a span not intended for export (for example in spans that create a lot of noise and customers + do not want to trace) + + @param span: The span to mark (The span is altered in place) + @param no_export: Should the span be exported or not (default is True, the span will not be exported) + @return: + """ + + span.set_attributes({'NO_EXPORT': no_export}) From d69b6f57952d28f0478fc90bd83a56c02b4884c5 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 13:31:52 +0300 Subject: [PATCH 06/20] chore: auto format code --- .../instrumentations/botocore/parsers/__init__.py | 12 ++++++++---- .../libs/environment_variables.py | 2 +- src/lumigo_opentelemetry/resources/span_processor.py | 11 +++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 959a9b8b..437f0e9f 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -138,12 +138,14 @@ def parse_request( span.set_attributes(attributes) @staticmethod - def _should_skip_empty_sqs_polling_response(operation_name: str, result: Dict[Any, Any]) -> bool: + def _should_skip_empty_sqs_polling_response( + operation_name: str, result: Dict[Any, Any] + ) -> bool: """ checks the sqs response & returns true if the request receive messages from SQS but no messages were returned """ - empty_sqs_poll = operation_name == 'ReceiveMessage' and 'Messages' not in result + empty_sqs_poll = operation_name == "ReceiveMessage" and "Messages" not in result return empty_sqs_poll and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) @staticmethod @@ -160,8 +162,10 @@ def parse_response( # Filter out sqs polls with empty response if SqsParser._should_skip_empty_sqs_polling_response(operation_name, result): - logger.debug('Not tracing empty SQS polling requests ' - f'(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)') + logger.debug( + "Not tracing empty SQS polling requests " + f"(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)" + ) set_span_no_export(span) diff --git a/src/lumigo_opentelemetry/libs/environment_variables.py b/src/lumigo_opentelemetry/libs/environment_variables.py index a63efc4a..6172c0b6 100644 --- a/src/lumigo_opentelemetry/libs/environment_variables.py +++ b/src/lumigo_opentelemetry/libs/environment_variables.py @@ -1 +1 @@ -AUTO_FILTER_EMPTY_SQS = 'AUTO_FILTER_EMPTY_SQS' +AUTO_FILTER_EMPTY_SQS = "AUTO_FILTER_EMPTY_SQS" diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py index 0881da9a..f804ea49 100644 --- a/src/lumigo_opentelemetry/resources/span_processor.py +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -6,13 +6,12 @@ class LumigoSpanProcessor(BatchSpanProcessor): - def on_end(self, span: ReadableSpan) -> None: if should_not_export_span(span): - logger.debug('Not exporting span because it has NO_EXPORT=True attribute') + logger.debug("Not exporting span because it has NO_EXPORT=True attribute") return - return super().on_end(span) + super().on_end(span) def should_not_export_span(span: ReadableSpan) -> bool: @@ -21,10 +20,10 @@ def should_not_export_span(span: ReadableSpan) -> bool: @param span: A readable span to check @return: True if the span should not be exported, False otherwise """ - return span.attributes.get('NO_EXPORT') is True + return span.attributes.get("NO_EXPORT") is True -def set_span_no_export(span: Span, no_export: bool = True): +def set_span_no_export(span: Span, no_export: bool = True) -> None: """ marks the span as a span not intended for export (for example in spans that create a lot of noise and customers do not want to trace) @@ -34,4 +33,4 @@ def set_span_no_export(span: Span, no_export: bool = True): @return: """ - span.set_attributes({'NO_EXPORT': no_export}) + span.set_attributes({"NO_EXPORT": no_export}) From 24dbb03188433f851e5297ad90bb70cc03f51fc5 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 13:58:25 +0300 Subject: [PATCH 07/20] test: bool env var retrival edge cases --- .../libs/general_utils.py | 10 +++++- src/test/unit/libs/test_general_utils.py | 33 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index 903e2514..27ee0755 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -82,4 +82,12 @@ def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: @return: The boolean value of the env var """ - return os.environ.get(env_var_name, str(default)).lower() == "true" + env_var_value = os.environ.get(env_var_name, str(default)).lower() + is_truth_value = env_var_value == "true" + is_false_value = env_var_value == "false" + if not is_truth_value and not is_false_value: + logger.debug(f'Invalid boolean value for env var "{env_var_name}", ' + f'defaulting to value "{str(default).lower()}"') + return default + + return is_truth_value diff --git a/src/test/unit/libs/test_general_utils.py b/src/test/unit/libs/test_general_utils.py index db5ebc38..58651425 100644 --- a/src/test/unit/libs/test_general_utils.py +++ b/src/test/unit/libs/test_general_utils.py @@ -1,4 +1,6 @@ -from lumigo_opentelemetry.libs.general_utils import get_max_size +import pytest + +from lumigo_opentelemetry.libs.general_utils import get_max_size, get_boolean_env_var def test_get_max_size_otel_span_attr_limit_is_set(monkeypatch): @@ -19,3 +21,32 @@ def test_get_max_size_both_env_vars_are_set(monkeypatch): def test_get_max_size_get_default_value(): assert get_max_size() == 2048 + + +@pytest.mark.parametrize( + "env_var_value,default,expected_result", + [ + # Normal truth values + ("true", False, True), + ("True", False, True), + ("TRUE", False, True), + + # Normal false values + ("false", True, False), + ("False", True, False), + ("FALSE", True, False), + + # Invalid values, use the default + ("RandomValue", False, False), + ("RandomValue", True, True), + + # Empty values, use the default + (None, False, False), + (None, True, True), + ] +) +def test_get_boolean_env_var(env_var_value, default, expected_result, monkeypatch): + """Try getting a boolean value from env vars, and check all the different options work""" + + monkeypatch.setenv("TEST_VAR", env_var_value) + assert get_boolean_env_var("TEST_VAR", default=default) is expected_result From d48e913b266e8ace8e72da1b622788dd9b9c076f Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 19:42:02 +0300 Subject: [PATCH 08/20] test: new span processor and empty sqs span skipping --- .../botocore/parsers/__init__.py | 14 ++- .../libs/general_utils.py | 6 +- .../resources/span_processor.py | 2 +- .../instrumentations/botocore/test_parsers.py | 104 ++++++++++++++++++ src/test/unit/libs/test_general_utils.py | 5 +- .../unit/resources/test_span_processor.py | 73 ++++++++++++ 6 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 src/test/unit/instrumentations/botocore/test_parsers.py create mode 100644 src/test/unit/resources/test_span_processor.py diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 437f0e9f..c5a60e92 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -145,13 +145,23 @@ def _should_skip_empty_sqs_polling_response( checks the sqs response & returns true if the request receive messages from SQS but no messages were returned """ - empty_sqs_poll = operation_name == "ReceiveMessage" and "Messages" not in result - return empty_sqs_poll and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) + no_messages = not result or not result.get("Messages", None) + sqs_poll = operation_name == "ReceiveMessage" + return ( + sqs_poll + and no_messages + and get_boolean_env_var(AUTO_FILTER_EMPTY_SQS, True) + ) @staticmethod def parse_response( span: Span, service_name: str, operation_name: str, result: Dict[Any, Any] ) -> None: + print("Running SqsParser.parse_response") + print(span) + print(service_name) + print(operation_name) + print(result) trigger_details = parse_triggers( {"service_name": service_name, "operation_name": operation_name, **result} ) diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index 27ee0755..cb5e0b17 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -86,8 +86,10 @@ def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: is_truth_value = env_var_value == "true" is_false_value = env_var_value == "false" if not is_truth_value and not is_false_value: - logger.debug(f'Invalid boolean value for env var "{env_var_name}", ' - f'defaulting to value "{str(default).lower()}"') + logger.debug( + f'Invalid boolean value for env var "{env_var_name}", ' + f'defaulting to value "{str(default).lower()}"' + ) return default return is_truth_value diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py index f804ea49..9749fb1d 100644 --- a/src/lumigo_opentelemetry/resources/span_processor.py +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -20,7 +20,7 @@ def should_not_export_span(span: ReadableSpan) -> bool: @param span: A readable span to check @return: True if the span should not be exported, False otherwise """ - return span.attributes.get("NO_EXPORT") is True + return span.attributes.get("NO_EXPORT", False) is True def set_span_no_export(span: Span, no_export: bool = True) -> None: diff --git a/src/test/unit/instrumentations/botocore/test_parsers.py b/src/test/unit/instrumentations/botocore/test_parsers.py new file mode 100644 index 00000000..9fd565bd --- /dev/null +++ b/src/test/unit/instrumentations/botocore/test_parsers.py @@ -0,0 +1,104 @@ +import pytest +from unittest.mock import Mock, patch + +from lumigo_opentelemetry.instrumentations.botocore.parsers import SqsParser + + +EMPTY_SQS_RESULT_1 = {} +EMPTY_SQS_RESULT_2 = {"Messages": []} +NON_EMPTY_SQS_RESULT = { + "Messages": [{ + "MessageId": "1234", + "Body": "test" + }] +} + + +@pytest.mark.parametrize( + "env_var_value, operation, result, should_skip", + [ + # Check that empty sqs polls are skipped + ("true", "ReceiveMessage", EMPTY_SQS_RESULT_1, True), + ("true", "ReceiveMessage", EMPTY_SQS_RESULT_2, True), + + # Check that non-empty polls are not skipped + ("true", "ReceiveMessage", NON_EMPTY_SQS_RESULT, False), + + # Check that other operations are not skipped + ("true", "DeleteMessage", EMPTY_SQS_RESULT_1, False), + ("true", "DeleteMessageBatch", EMPTY_SQS_RESULT_1, False), + ("true", "SendMessage", EMPTY_SQS_RESULT_1, False), + ("true", "SendMessageBatch", EMPTY_SQS_RESULT_1, False), + ("true", "UnknownOperation", EMPTY_SQS_RESULT_1, False), + ("true", None, EMPTY_SQS_RESULT_1, False), + + # Check that empty sqs polls are not skipped if the env var is set to false + ("false", "ReceiveMessage", EMPTY_SQS_RESULT_1, False), + ("false", "ReceiveMessage", EMPTY_SQS_RESULT_2, False), + + # Check that non-empty polls are not skipped if the env var is set to false + ("false", "ReceiveMessage", NON_EMPTY_SQS_RESULT, False), + + # Check that the default behavior is to skip empty sqs polls + (None, "ReceiveMessage", EMPTY_SQS_RESULT_1, True), + (None, "ReceiveMessage", EMPTY_SQS_RESULT_2, True), + ("UnsupportedEnvVarValue", "ReceiveMessage", EMPTY_SQS_RESULT_2, True), + ] +) +def test_sqs_skip_sqs_response(env_var_value, operation, result, should_skip, monkeypatch): + if env_var_value is not None: + monkeypatch.setenv("AUTO_FILTER_EMPTY_SQS", env_var_value) + + assert SqsParser._should_skip_empty_sqs_polling_response(operation, result) == should_skip + + +@patch("lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response") +def test_parse_sqs_response_skipping_empty_polls_outputs_log(should_skip_mock, caplog): + should_skip_mock.return_value = True + span = Mock(set_attribute=Mock()) + service_name = 'sqs' + operation_name = 'ReceiveMessage' + result = { + 'ResponseMetadata': { + 'RequestId': '54bf0dab-cfab-5fa5-b284-50d83403c94c', + 'HTTPStatusCode': 200, + 'HTTPHeaders': { + 'x-amzn-requestid': '54bf0dab-cfab-5fa5-b284-50d83403c94c', + 'date': 'Thu, 07 Sep 2023 16:25:12 GMT', + 'content-type': 'text/xml', + 'content-length': '240', + 'connection': 'keep-alive' + }, + 'RetryAttempts': 0 + } + } + + SqsParser.parse_response(span, service_name, operation_name, result) + + assert "not tracing empty sqs polling requests" in caplog.text.lower() + + +@patch("lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response") +def test_parse_sqs_response_not_skipping_polls_no_output_log(should_skip_mock, caplog): + should_skip_mock.return_value = False + span = Mock(set_attribute=Mock()) + service_name = 'sqs' + operation_name = 'ReceiveMessage' + result = { + 'ResponseMetadata': { + 'RequestId': '54bf0dab-cfab-5fa5-b284-50d83403c94c', + 'HTTPStatusCode': 200, + 'HTTPHeaders': { + 'x-amzn-requestid': '54bf0dab-cfab-5fa5-b284-50d83403c94c', + 'date': 'Thu, 07 Sep 2023 16:25:12 GMT', + 'content-type': 'text/xml', + 'content-length': '240', + 'connection': 'keep-alive' + }, + 'RetryAttempts': 0 + } + } + + SqsParser.parse_response(span, service_name, operation_name, result) + + assert "not tracing empty sqs polling requests" not in caplog.text.lower() diff --git a/src/test/unit/libs/test_general_utils.py b/src/test/unit/libs/test_general_utils.py index 58651425..e6a9bc1e 100644 --- a/src/test/unit/libs/test_general_utils.py +++ b/src/test/unit/libs/test_general_utils.py @@ -30,20 +30,17 @@ def test_get_max_size_get_default_value(): ("true", False, True), ("True", False, True), ("TRUE", False, True), - # Normal false values ("false", True, False), ("False", True, False), ("FALSE", True, False), - # Invalid values, use the default ("RandomValue", False, False), ("RandomValue", True, True), - # Empty values, use the default (None, False, False), (None, True, True), - ] + ], ) def test_get_boolean_env_var(env_var_value, default, expected_result, monkeypatch): """Try getting a boolean value from env vars, and check all the different options work""" diff --git a/src/test/unit/resources/test_span_processor.py b/src/test/unit/resources/test_span_processor.py new file mode 100644 index 00000000..b426b158 --- /dev/null +++ b/src/test/unit/resources/test_span_processor.py @@ -0,0 +1,73 @@ +from unittest.mock import Mock, patch + +import pytest + +from lumigo_opentelemetry.resources.span_processor import ( + set_span_no_export, + should_not_export_span, + LumigoSpanProcessor, +) + + +def test_set_span_no_export(): + """ + Given a span, check that the span is marked as not exported + """ + + for no_export in [True, False]: + span_mock = Mock(set_attribute=Mock()) + set_span_no_export(span_mock, no_export) + (attributes,) = span_mock.set_attributes.call_args[0] + assert attributes == {"NO_EXPORT": no_export} + + # Check default value + span_mock = Mock(set_attribute=Mock()) + set_span_no_export(span_mock) + (attributes,) = span_mock.set_attributes.call_args[0] + assert attributes == {"NO_EXPORT": True} + + +@pytest.mark.parametrize( + "attributes, should_export", + [ + # Default is to export + ({}, True), + # Use the value if it is set + ({"NO_EXPORT": False}, True), + ({"NO_EXPORT": True}, False), + ], +) +def test_should_not_export_span(attributes, should_export): + readable_span_mock = Mock(attributes=attributes) + + assert should_not_export_span(readable_span_mock) is not should_export + + +@patch("lumigo_opentelemetry.resources.span_processor.should_not_export_span") +@patch("opentelemetry.sdk.trace.export.BatchSpanProcessor.on_end") +def test_lumigo_span_processor_no_export_set( + mocked_super_on_end, mocked_should_not_export_span +): + processor = LumigoSpanProcessor(Mock()) + readable_span_mock = Mock() + mocked_should_not_export_span.return_value = True + processor.on_end(span=readable_span_mock) + + # Check if the parent of processor BatchSpanProcessor.on_end not called + mocked_super_on_end.assert_not_called() + + +@patch("lumigo_opentelemetry.resources.span_processor.should_not_export_span") +@patch("opentelemetry.sdk.trace.export.BatchSpanProcessor.on_end") +def test_lumigo_span_processor_no_export_not_set( + mocked_super_on_end, mocked_should_not_export_span +): + processor = LumigoSpanProcessor(Mock()) + readable_span_mock = Mock() + + # should_not_export is False. i.e. the span should be exported + mocked_should_not_export_span.return_value = False + processor.on_end(span=readable_span_mock) + + # Check if the parent of processor BatchSpanProcessor.on_end was called + mocked_super_on_end.assert_called_once_with(readable_span_mock) From 40c68e81a87b833d8b262e78ce15b08cd5fb07df Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 19:42:29 +0300 Subject: [PATCH 09/20] chore: auto format code --- .../instrumentations/botocore/test_parsers.py | 79 +++++++++---------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/src/test/unit/instrumentations/botocore/test_parsers.py b/src/test/unit/instrumentations/botocore/test_parsers.py index 9fd565bd..4fe79a0b 100644 --- a/src/test/unit/instrumentations/botocore/test_parsers.py +++ b/src/test/unit/instrumentations/botocore/test_parsers.py @@ -6,12 +6,7 @@ EMPTY_SQS_RESULT_1 = {} EMPTY_SQS_RESULT_2 = {"Messages": []} -NON_EMPTY_SQS_RESULT = { - "Messages": [{ - "MessageId": "1234", - "Body": "test" - }] -} +NON_EMPTY_SQS_RESULT = {"Messages": [{"MessageId": "1234", "Body": "test"}]} @pytest.mark.parametrize( @@ -20,10 +15,8 @@ # Check that empty sqs polls are skipped ("true", "ReceiveMessage", EMPTY_SQS_RESULT_1, True), ("true", "ReceiveMessage", EMPTY_SQS_RESULT_2, True), - # Check that non-empty polls are not skipped ("true", "ReceiveMessage", NON_EMPTY_SQS_RESULT, False), - # Check that other operations are not skipped ("true", "DeleteMessage", EMPTY_SQS_RESULT_1, False), ("true", "DeleteMessageBatch", EMPTY_SQS_RESULT_1, False), @@ -31,45 +24,49 @@ ("true", "SendMessageBatch", EMPTY_SQS_RESULT_1, False), ("true", "UnknownOperation", EMPTY_SQS_RESULT_1, False), ("true", None, EMPTY_SQS_RESULT_1, False), - # Check that empty sqs polls are not skipped if the env var is set to false ("false", "ReceiveMessage", EMPTY_SQS_RESULT_1, False), ("false", "ReceiveMessage", EMPTY_SQS_RESULT_2, False), - # Check that non-empty polls are not skipped if the env var is set to false ("false", "ReceiveMessage", NON_EMPTY_SQS_RESULT, False), - # Check that the default behavior is to skip empty sqs polls (None, "ReceiveMessage", EMPTY_SQS_RESULT_1, True), (None, "ReceiveMessage", EMPTY_SQS_RESULT_2, True), ("UnsupportedEnvVarValue", "ReceiveMessage", EMPTY_SQS_RESULT_2, True), - ] + ], ) -def test_sqs_skip_sqs_response(env_var_value, operation, result, should_skip, monkeypatch): +def test_sqs_skip_sqs_response( + env_var_value, operation, result, should_skip, monkeypatch +): if env_var_value is not None: monkeypatch.setenv("AUTO_FILTER_EMPTY_SQS", env_var_value) - assert SqsParser._should_skip_empty_sqs_polling_response(operation, result) == should_skip + assert ( + SqsParser._should_skip_empty_sqs_polling_response(operation, result) + == should_skip + ) -@patch("lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response") +@patch( + "lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response" +) def test_parse_sqs_response_skipping_empty_polls_outputs_log(should_skip_mock, caplog): should_skip_mock.return_value = True span = Mock(set_attribute=Mock()) - service_name = 'sqs' - operation_name = 'ReceiveMessage' + service_name = "sqs" + operation_name = "ReceiveMessage" result = { - 'ResponseMetadata': { - 'RequestId': '54bf0dab-cfab-5fa5-b284-50d83403c94c', - 'HTTPStatusCode': 200, - 'HTTPHeaders': { - 'x-amzn-requestid': '54bf0dab-cfab-5fa5-b284-50d83403c94c', - 'date': 'Thu, 07 Sep 2023 16:25:12 GMT', - 'content-type': 'text/xml', - 'content-length': '240', - 'connection': 'keep-alive' + "ResponseMetadata": { + "RequestId": "54bf0dab-cfab-5fa5-b284-50d83403c94c", + "HTTPStatusCode": 200, + "HTTPHeaders": { + "x-amzn-requestid": "54bf0dab-cfab-5fa5-b284-50d83403c94c", + "date": "Thu, 07 Sep 2023 16:25:12 GMT", + "content-type": "text/xml", + "content-length": "240", + "connection": "keep-alive", }, - 'RetryAttempts': 0 + "RetryAttempts": 0, } } @@ -78,24 +75,26 @@ def test_parse_sqs_response_skipping_empty_polls_outputs_log(should_skip_mock, c assert "not tracing empty sqs polling requests" in caplog.text.lower() -@patch("lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response") +@patch( + "lumigo_opentelemetry.instrumentations.botocore.parsers.SqsParser._should_skip_empty_sqs_polling_response" +) def test_parse_sqs_response_not_skipping_polls_no_output_log(should_skip_mock, caplog): should_skip_mock.return_value = False span = Mock(set_attribute=Mock()) - service_name = 'sqs' - operation_name = 'ReceiveMessage' + service_name = "sqs" + operation_name = "ReceiveMessage" result = { - 'ResponseMetadata': { - 'RequestId': '54bf0dab-cfab-5fa5-b284-50d83403c94c', - 'HTTPStatusCode': 200, - 'HTTPHeaders': { - 'x-amzn-requestid': '54bf0dab-cfab-5fa5-b284-50d83403c94c', - 'date': 'Thu, 07 Sep 2023 16:25:12 GMT', - 'content-type': 'text/xml', - 'content-length': '240', - 'connection': 'keep-alive' + "ResponseMetadata": { + "RequestId": "54bf0dab-cfab-5fa5-b284-50d83403c94c", + "HTTPStatusCode": 200, + "HTTPHeaders": { + "x-amzn-requestid": "54bf0dab-cfab-5fa5-b284-50d83403c94c", + "date": "Thu, 07 Sep 2023 16:25:12 GMT", + "content-type": "text/xml", + "content-length": "240", + "connection": "keep-alive", }, - 'RetryAttempts': 0 + "RetryAttempts": 0, } } From ff1a6a12572c38f0dbb722a6915d666011c70a14 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Thu, 7 Sep 2023 20:28:28 +0300 Subject: [PATCH 10/20] fix: remove debugging prints --- .../instrumentations/botocore/parsers/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index c5a60e92..28150885 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -157,11 +157,6 @@ def _should_skip_empty_sqs_polling_response( def parse_response( span: Span, service_name: str, operation_name: str, result: Dict[Any, Any] ) -> None: - print("Running SqsParser.parse_response") - print(span) - print(service_name) - print(operation_name) - print(result) trigger_details = parse_triggers( {"service_name": service_name, "operation_name": operation_name, **result} ) From 05f11d5523be4884b2bdc60f626e2790bee2a4e0 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 10:15:55 +0300 Subject: [PATCH 11/20] refactor: rename NO_EXPORT attribute to SKIP_EXPORT --- .../botocore/parsers/__init__.py | 4 ++-- .../resources/span_processor.py | 16 +++++++++----- .../unit/resources/test_span_processor.py | 22 +++++++++---------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index 28150885..fb19ce89 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -12,7 +12,7 @@ extract_region_from_arn, get_resource_fullname, ) -from lumigo_opentelemetry.resources.span_processor import set_span_no_export +from lumigo_opentelemetry.resources.span_processor import set_span_skip_export from lumigo_opentelemetry import logger from lumigo_opentelemetry.libs.general_utils import get_boolean_env_var from lumigo_opentelemetry.libs.environment_variables import AUTO_FILTER_EMPTY_SQS @@ -171,7 +171,7 @@ def parse_response( "Not tracing empty SQS polling requests " f"(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)" ) - set_span_no_export(span) + set_span_skip_export(span) class LambdaParser(AwsParser): diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py index 9749fb1d..90fe88ec 100644 --- a/src/lumigo_opentelemetry/resources/span_processor.py +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -5,32 +5,36 @@ from lumigo_opentelemetry import logger +# A span attributes that if is set to True, the span will not be exported +SKIP_EXPORT_SPAN_ATTRIBUTE = "NO_EXPORT" + + class LumigoSpanProcessor(BatchSpanProcessor): def on_end(self, span: ReadableSpan) -> None: - if should_not_export_span(span): + if should_skip_exporting_span(span): logger.debug("Not exporting span because it has NO_EXPORT=True attribute") return super().on_end(span) -def should_not_export_span(span: ReadableSpan) -> bool: +def should_skip_exporting_span(span: ReadableSpan) -> bool: """ Given a span, returns an answer if the span should be exported or not. @param span: A readable span to check @return: True if the span should not be exported, False otherwise """ - return span.attributes.get("NO_EXPORT", False) is True + return span.attributes.get(SKIP_EXPORT_SPAN_ATTRIBUTE, False) is True -def set_span_no_export(span: Span, no_export: bool = True) -> None: +def set_span_skip_export(span: Span, skip_export: bool = True) -> None: """ marks the span as a span not intended for export (for example in spans that create a lot of noise and customers do not want to trace) @param span: The span to mark (The span is altered in place) - @param no_export: Should the span be exported or not (default is True, the span will not be exported) + @param skip_export: Should the span be exported or not (default is True, the span will not be exported) @return: """ - span.set_attributes({"NO_EXPORT": no_export}) + span.set_attributes({SKIP_EXPORT_SPAN_ATTRIBUTE: skip_export}) diff --git a/src/test/unit/resources/test_span_processor.py b/src/test/unit/resources/test_span_processor.py index b426b158..9ea8c9f8 100644 --- a/src/test/unit/resources/test_span_processor.py +++ b/src/test/unit/resources/test_span_processor.py @@ -3,8 +3,8 @@ import pytest from lumigo_opentelemetry.resources.span_processor import ( - set_span_no_export, - should_not_export_span, + set_span_skip_export, + should_skip_exporting_span, LumigoSpanProcessor, ) @@ -16,15 +16,15 @@ def test_set_span_no_export(): for no_export in [True, False]: span_mock = Mock(set_attribute=Mock()) - set_span_no_export(span_mock, no_export) + set_span_skip_export(span_mock, no_export) (attributes,) = span_mock.set_attributes.call_args[0] - assert attributes == {"NO_EXPORT": no_export} + assert attributes == {"SKIP_EXPORT": no_export} # Check default value span_mock = Mock(set_attribute=Mock()) - set_span_no_export(span_mock) + set_span_skip_export(span_mock) (attributes,) = span_mock.set_attributes.call_args[0] - assert attributes == {"NO_EXPORT": True} + assert attributes == {"SKIP_EXPORT": True} @pytest.mark.parametrize( @@ -33,17 +33,17 @@ def test_set_span_no_export(): # Default is to export ({}, True), # Use the value if it is set - ({"NO_EXPORT": False}, True), - ({"NO_EXPORT": True}, False), + ({"SKIP_EXPORT": False}, True), + ({"SKIP_EXPORT": True}, False), ], ) def test_should_not_export_span(attributes, should_export): readable_span_mock = Mock(attributes=attributes) - assert should_not_export_span(readable_span_mock) is not should_export + assert should_skip_exporting_span(readable_span_mock) is not should_export -@patch("lumigo_opentelemetry.resources.span_processor.should_not_export_span") +@patch("lumigo_opentelemetry.resources.span_processor.should_skip_exporting_span") @patch("opentelemetry.sdk.trace.export.BatchSpanProcessor.on_end") def test_lumigo_span_processor_no_export_set( mocked_super_on_end, mocked_should_not_export_span @@ -57,7 +57,7 @@ def test_lumigo_span_processor_no_export_set( mocked_super_on_end.assert_not_called() -@patch("lumigo_opentelemetry.resources.span_processor.should_not_export_span") +@patch("lumigo_opentelemetry.resources.span_processor.should_skip_exporting_span") @patch("opentelemetry.sdk.trace.export.BatchSpanProcessor.on_end") def test_lumigo_span_processor_no_export_not_set( mocked_super_on_end, mocked_should_not_export_span From efda5e941a9630df5061e3630ca2f20b9d1e60e2 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 10:17:15 +0300 Subject: [PATCH 12/20] fix: wrong attribute name used --- src/lumigo_opentelemetry/resources/span_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lumigo_opentelemetry/resources/span_processor.py b/src/lumigo_opentelemetry/resources/span_processor.py index 90fe88ec..72ad226e 100644 --- a/src/lumigo_opentelemetry/resources/span_processor.py +++ b/src/lumigo_opentelemetry/resources/span_processor.py @@ -6,7 +6,7 @@ # A span attributes that if is set to True, the span will not be exported -SKIP_EXPORT_SPAN_ATTRIBUTE = "NO_EXPORT" +SKIP_EXPORT_SPAN_ATTRIBUTE = "SKIP_EXPORT" class LumigoSpanProcessor(BatchSpanProcessor): From 88b4406319699c0edf5970b304f86fc61d860b95 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 10:35:49 +0300 Subject: [PATCH 13/20] test: integration test empty sqs polling span skipping --- src/test/integration/boto3-sqs/app/main.py | 5 +++++ .../boto3-sqs/tests/test_boto3_sqs.py | 21 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/test/integration/boto3-sqs/app/main.py b/src/test/integration/boto3-sqs/app/main.py index 9291d4a4..aa270366 100644 --- a/src/test/integration/boto3-sqs/app/main.py +++ b/src/test/integration/boto3-sqs/app/main.py @@ -13,6 +13,11 @@ def run(): client = boto3.client("sqs", region_name="eu-central-1") queue = client.create_queue(QueueName="test") + # Simulate polling an empty sqs queue + client.receive_message(QueueUrl=queue["QueueUrl"], MaxNumberOfMessages=1) + client.receive_message(QueueUrl=queue["QueueUrl"], MaxNumberOfMessages=1) + client.receive_message(QueueUrl=queue["QueueUrl"], MaxNumberOfMessages=1) + client.send_message( QueueUrl=queue["QueueUrl"], MessageBody="Message_1", diff --git a/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py b/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py index 2a498fa2..50da7f11 100644 --- a/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py +++ b/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py @@ -5,10 +5,13 @@ class TestBoto3SqsSpans(unittest.TestCase): def test_boto3_instrumentation(self): spans_container = SpansContainer.parse_spans_from_file() - self.assertEqual(9, len(spans_container.spans)) + self.assertEqual(12, len(spans_container.spans)) [ create_queue_span, + empty_sqs_poll_1_span, + empty_sqs_poll_2_span, + empty_sqs_poll_3_span, send_message_1_span, send_message_2_span, receive_message_1_span, @@ -21,6 +24,22 @@ def test_boto3_instrumentation(self): after_iterator_break_span, ] = spans_container.spans + # Make sure that all the empty polling spans are marked as skipped + for span in [empty_sqs_poll_1_span, empty_sqs_poll_2_span, empty_sqs_poll_3_span]: + self.assertIsNotNone(span.get("attributes")) + self.assertEqual(span["attributes"].get("SKIP_EXPORT"), True) + + # Make sure that other spans are not marked as skipped + for span in [create_queue_span, + send_message_1_span, + send_message_2_span, + receive_message_1_span, + receive_message_2_span, + consume_message_2_span, + consume_message_1_span, + receive_message_2_span]: + self.assertNotEqual(span.get("attributes", {}).get("SKIP_EXPORT"), True) + self.assertEqual(create_queue_span["name"], "SQS.CreateQueue") self.assertIsNone(create_queue_span["parent_id"]) From 73887b33d8ff0cbd2b428abb0f18d8b48a1b3118 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 10:38:29 +0300 Subject: [PATCH 14/20] chore: fix linting errors --- .../boto3-sqs/tests/test_boto3_sqs.py | 24 ++++++++++++------- src/test/unit/libs/test_general_utils.py | 3 ++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py b/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py index 50da7f11..b3305e61 100644 --- a/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py +++ b/src/test/integration/boto3-sqs/tests/test_boto3_sqs.py @@ -25,19 +25,25 @@ def test_boto3_instrumentation(self): ] = spans_container.spans # Make sure that all the empty polling spans are marked as skipped - for span in [empty_sqs_poll_1_span, empty_sqs_poll_2_span, empty_sqs_poll_3_span]: + for span in [ + empty_sqs_poll_1_span, + empty_sqs_poll_2_span, + empty_sqs_poll_3_span, + ]: self.assertIsNotNone(span.get("attributes")) self.assertEqual(span["attributes"].get("SKIP_EXPORT"), True) # Make sure that other spans are not marked as skipped - for span in [create_queue_span, - send_message_1_span, - send_message_2_span, - receive_message_1_span, - receive_message_2_span, - consume_message_2_span, - consume_message_1_span, - receive_message_2_span]: + for span in [ + create_queue_span, + send_message_1_span, + send_message_2_span, + receive_message_1_span, + receive_message_2_span, + consume_message_2_span, + consume_message_1_span, + receive_message_2_span, + ]: self.assertNotEqual(span.get("attributes", {}).get("SKIP_EXPORT"), True) self.assertEqual(create_queue_span["name"], "SQS.CreateQueue") diff --git a/src/test/unit/libs/test_general_utils.py b/src/test/unit/libs/test_general_utils.py index e6a9bc1e..5aa5b93d 100644 --- a/src/test/unit/libs/test_general_utils.py +++ b/src/test/unit/libs/test_general_utils.py @@ -45,5 +45,6 @@ def test_get_max_size_get_default_value(): def test_get_boolean_env_var(env_var_value, default, expected_result, monkeypatch): """Try getting a boolean value from env vars, and check all the different options work""" - monkeypatch.setenv("TEST_VAR", env_var_value) + if env_var_value is not None: + monkeypatch.setenv("TEST_VAR", env_var_value) assert get_boolean_env_var("TEST_VAR", default=default) is expected_result From be82ddebd955baaf345c019e7781b491e54d5f50 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 10:48:17 +0300 Subject: [PATCH 15/20] fix: change skipping empty sqs poll log line to info --- .../instrumentations/botocore/parsers/__init__.py | 2 +- src/test/unit/instrumentations/botocore/test_parsers.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py index fb19ce89..165d121e 100644 --- a/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py +++ b/src/lumigo_opentelemetry/instrumentations/botocore/parsers/__init__.py @@ -167,7 +167,7 @@ def parse_response( # Filter out sqs polls with empty response if SqsParser._should_skip_empty_sqs_polling_response(operation_name, result): - logger.debug( + logger.info( "Not tracing empty SQS polling requests " f"(override by setting the {AUTO_FILTER_EMPTY_SQS} env var to false)" ) diff --git a/src/test/unit/instrumentations/botocore/test_parsers.py b/src/test/unit/instrumentations/botocore/test_parsers.py index 4fe79a0b..aee01272 100644 --- a/src/test/unit/instrumentations/botocore/test_parsers.py +++ b/src/test/unit/instrumentations/botocore/test_parsers.py @@ -1,3 +1,5 @@ +import logging + import pytest from unittest.mock import Mock, patch @@ -70,9 +72,10 @@ def test_parse_sqs_response_skipping_empty_polls_outputs_log(should_skip_mock, c } } - SqsParser.parse_response(span, service_name, operation_name, result) + with caplog.at_level(logging.INFO): + SqsParser.parse_response(span, service_name, operation_name, result) - assert "not tracing empty sqs polling requests" in caplog.text.lower() + assert "not tracing empty sqs polling requests" in caplog.text.lower() @patch( @@ -101,3 +104,5 @@ def test_parse_sqs_response_not_skipping_polls_no_output_log(should_skip_mock, c SqsParser.parse_response(span, service_name, operation_name, result) assert "not tracing empty sqs polling requests" not in caplog.text.lower() + + # Make sure that there is an info log From 7b963210c8476f0de8c26ea06b346bd46b371d25 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 11:40:15 +0300 Subject: [PATCH 16/20] fix: edge case env var with None value --- src/lumigo_opentelemetry/libs/general_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index cb5e0b17..8711783e 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -82,7 +82,9 @@ def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: @return: The boolean value of the env var """ - env_var_value = os.environ.get(env_var_name, str(default)).lower() + env_var_value = os.environ.get(env_var_name, str(default)) + env_var_value = env_var_value.lower() if isinstance(env_var_value, str) else env_var_value + is_truth_value = env_var_value == "true" is_false_value = env_var_value == "false" if not is_truth_value and not is_false_value: From 65b91137e3f5b56573eb56e5dccff845f4b468d6 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 11:40:46 +0300 Subject: [PATCH 17/20] chore: auto format code --- src/lumigo_opentelemetry/libs/general_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lumigo_opentelemetry/libs/general_utils.py b/src/lumigo_opentelemetry/libs/general_utils.py index 8711783e..1ccf4bb8 100644 --- a/src/lumigo_opentelemetry/libs/general_utils.py +++ b/src/lumigo_opentelemetry/libs/general_utils.py @@ -83,7 +83,9 @@ def get_boolean_env_var(env_var_name: str, default: bool = False) -> bool: """ env_var_value = os.environ.get(env_var_name, str(default)) - env_var_value = env_var_value.lower() if isinstance(env_var_value, str) else env_var_value + env_var_value = ( + env_var_value.lower() if isinstance(env_var_value, str) else env_var_value + ) is_truth_value = env_var_value == "true" is_false_value = env_var_value == "false" From eeb8ce6a0a1f5623746ceabd0c81117e64589600 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 12:34:17 +0300 Subject: [PATCH 18/20] fix: change env var name to have lumigo prefix --- src/lumigo_opentelemetry/libs/environment_variables.py | 2 +- src/test/unit/instrumentations/botocore/test_parsers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lumigo_opentelemetry/libs/environment_variables.py b/src/lumigo_opentelemetry/libs/environment_variables.py index 6172c0b6..d1613e81 100644 --- a/src/lumigo_opentelemetry/libs/environment_variables.py +++ b/src/lumigo_opentelemetry/libs/environment_variables.py @@ -1 +1 @@ -AUTO_FILTER_EMPTY_SQS = "AUTO_FILTER_EMPTY_SQS" +AUTO_FILTER_EMPTY_SQS = "LUMIGO_AUTO_FILTER_EMPTY_SQS" diff --git a/src/test/unit/instrumentations/botocore/test_parsers.py b/src/test/unit/instrumentations/botocore/test_parsers.py index aee01272..913e1d34 100644 --- a/src/test/unit/instrumentations/botocore/test_parsers.py +++ b/src/test/unit/instrumentations/botocore/test_parsers.py @@ -41,7 +41,7 @@ def test_sqs_skip_sqs_response( env_var_value, operation, result, should_skip, monkeypatch ): if env_var_value is not None: - monkeypatch.setenv("AUTO_FILTER_EMPTY_SQS", env_var_value) + monkeypatch.setenv("LUMIGO_AUTO_FILTER_EMPTY_SQS", env_var_value) assert ( SqsParser._should_skip_empty_sqs_polling_response(operation, result) From fbdb95cdf9cd3e25db8496fcf704f63f2d01202e Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 12:34:48 +0300 Subject: [PATCH 19/20] docs: update README.md with filter empty sqs messages feature --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index cca1d8ad..4cdf3ffd 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,7 @@ The `lumigo_opentelemetry` package additionally supports the following configura * We support more granular masking using the following parameters. If not given, the above configuration is the fallback: `LUMIGO_SECRET_MASKING_REGEX_HTTP_REQUEST_BODIES`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_REQUEST_HEADERS`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_RESPONSE_BODIES`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_RESPONSE_HEADERS`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_QUERY_PARAMS`, `LUMIGO_SECRET_MASKING_REGEX_ENVIRONMENT`. * `LUMIGO_SWITCH_OFF=true`: This option disables the Lumigo OpenTelemetry distro entirely; no instrumentation will be injected, no tracing data will be collected. * `LUMIGO_REPORT_DEPENDENCIES=false`: This option disables the built-in dependency reporting to Lumigo SaaS. For more information, refer to the [Automated dependency reporting](#automated-dependency-reporting) section. +* `LUMIGO_AUTO_FILTER_EMPTY_SQS`: This option enables the automatic filtering of empty SQS messages from being sent to Lumigo SaaS. ### Execution Tags @@ -340,6 +341,15 @@ for message in response.get("Messages", []): Without the scope provided by the iterator over `response["Messages"]`, `span_1` would be without a parent span, and that would result in a separate invocation and a separate transaction in Lumigo. +### Filtering out empty SQS messages + +A common pattern in SQS-based applications is to continuously poll an SQS queue for messages, +and to process them as they arrive. +In order not to clutter the Lumigo Dashboard with empty SQS polling messages, the default behavior is to filter them +out from being sent to Lumigo. + +You can change this behavior by setting the environment variable `LUMIGO_AUTO_FILTER_EMPTY_SQS` to `false`. + ## Testing We use [nox](https://pypi.org/project/nox/) for setting up and running our tests. From 302dfb00ab061ae4c32ff1f15709845950b6d194 Mon Sep 17 00:00:00 2001 From: Sagiv Oulu Date: Sun, 10 Sep 2023 14:20:50 +0300 Subject: [PATCH 20/20] docs: detailed possible values for LUMIGO_AUTO_FILTER_EMPTY_SQS --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4cdf3ffd..8c2ee330 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ The `lumigo_opentelemetry` package additionally supports the following configura * We support more granular masking using the following parameters. If not given, the above configuration is the fallback: `LUMIGO_SECRET_MASKING_REGEX_HTTP_REQUEST_BODIES`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_REQUEST_HEADERS`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_RESPONSE_BODIES`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_RESPONSE_HEADERS`, `LUMIGO_SECRET_MASKING_REGEX_HTTP_QUERY_PARAMS`, `LUMIGO_SECRET_MASKING_REGEX_ENVIRONMENT`. * `LUMIGO_SWITCH_OFF=true`: This option disables the Lumigo OpenTelemetry distro entirely; no instrumentation will be injected, no tracing data will be collected. * `LUMIGO_REPORT_DEPENDENCIES=false`: This option disables the built-in dependency reporting to Lumigo SaaS. For more information, refer to the [Automated dependency reporting](#automated-dependency-reporting) section. -* `LUMIGO_AUTO_FILTER_EMPTY_SQS`: This option enables the automatic filtering of empty SQS messages from being sent to Lumigo SaaS. +* `LUMIGO_AUTO_FILTER_EMPTY_SQS`: This option enables the automatic filtering of empty SQS messages from being sent to Lumigo SaaS. For more information, refer to the [Filtering out empty SQS messages](#filtering-out-empty-sqs-messages) section. ### Execution Tags @@ -348,7 +348,11 @@ and to process them as they arrive. In order not to clutter the Lumigo Dashboard with empty SQS polling messages, the default behavior is to filter them out from being sent to Lumigo. -You can change this behavior by setting the environment variable `LUMIGO_AUTO_FILTER_EMPTY_SQS` to `false`. +You can change this behavior by setting the boolean environment variable `LUMIGO_AUTO_FILTER_EMPTY_SQS` to `false`. +The possible variations are: +* `LUMIGO_AUTO_FILTER_EMPTY_SQS=true` filter out empty SQS polling messages +* `LUMIGO_AUTO_FILTER_EMPTY_SQS=false` do not filter out empty SQS polling messages +* No environment variable set (default): filter out empty SQS polling messages ## Testing