From 4c96222ed77e96eebd5e39fd5482637e96c3978a Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 19 Sep 2024 14:27:01 -0400 Subject: [PATCH 01/12] chore(telemetry): unskip all tests --- tests/telemetry/test_writer.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 73cdc98c00e..28c8d7704d7 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -182,7 +182,6 @@ def test_app_started_event(telemetry_writer, test_agent_session, mock_time): ("DD_APPSEC_SCA_ENABLED", "0", "false"), ], ) -@pytest.mark.skip(reason="FIXME: This test needs to be updated.") def test_app_started_event_configuration_override( test_agent_session, run_python_code_in_subprocess, tmpdir, env_var, value, expected_value ): @@ -196,6 +195,12 @@ def test_app_started_event_configuration_override( logging.basicConfig() import ddtrace.auto + +# By default telemetry collection is enabled after 10 seconds, so we either need to +# to sleep for 10 seconds or manually call _app_started() to generate the app started event. +# This delay allows us to collect start up erorrs and dynamic configurations +import ddtrace +ddtrace.internal.telemetry.telemetry_writer._app_started() """ env = os.environ.copy() @@ -253,10 +258,8 @@ def test_app_started_event_configuration_override( env["DD_SPAN_SAMPLING_RULES_FILE"] = str(file) env["DD_TRACE_PARTIAL_FLUSH_ENABLED"] = "false" env["DD_TRACE_PARTIAL_FLUSH_MIN_SPANS"] = "3" - env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" _, stderr, status, _ = run_python_code_in_subprocess(code, env=env) - assert status == 0, stderr app_started_events = test_agent_session.get_events("app-started") @@ -380,19 +383,19 @@ def test_update_dependencies_event_when_disabled(telemetry_writer, test_agent_se assert event["request_type"] != "app-dependencies-loaded" -@pytest.mark.skip(reason="FIXME: This test does not generate a dependencies event") def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_session, mock_time): # Fetch modules to reset the state of seen modules modules.get_newly_imported_modules() + telemetry_writer.reset_queues() + assert telemetry_writer._imported_dependencies == {} + del sys.modules["httpretty"] + import httpretty # noqa F401 - import string - - new_deps = [string.__name__] - telemetry_writer._app_dependencies_loaded_event(new_deps) # force a flush telemetry_writer.periodic(force_flush=True) events = test_agent_session.get_events("app-dependencies-loaded") assert len(events) == 1 + assert events[0]["payload"]["dependencies"] == [{"name": "httpretty", "version": "1.0.5"}] def test_update_dependencies_event_not_duplicated(telemetry_writer, test_agent_session, mock_time): From f940031746a4b7eccc3710a6a22748b86b9e68be Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 20 Sep 2024 11:55:24 -0400 Subject: [PATCH 02/12] Update test_writer.py Co-authored-by: erikayasuda <153395705+erikayasuda@users.noreply.github.com> --- tests/telemetry/test_writer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 28c8d7704d7..9ae6e7aa1b3 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -198,7 +198,7 @@ def test_app_started_event_configuration_override( # By default telemetry collection is enabled after 10 seconds, so we either need to # to sleep for 10 seconds or manually call _app_started() to generate the app started event. -# This delay allows us to collect start up erorrs and dynamic configurations +# This delay allows us to collect start up errors and dynamic configurations import ddtrace ddtrace.internal.telemetry.telemetry_writer._app_started() """ From 09d9b07d03d56b61981800aa854493c25ec3af2a Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 20 Sep 2024 13:17:04 -0400 Subject: [PATCH 03/12] avoid checkign the number of requests, multiple heartbeats can be sent --- tests/telemetry/test_writer.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 9ae6e7aa1b3..5f608c1e162 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -536,8 +536,6 @@ def test_send_failing_request(mock_status, telemetry_writer): telemetry_writer._client.url, mock_status, ) - # ensure one failing request was sent - assert len(httpretty.latest_requests()) == 1 def test_app_heartbeat_event_periodic(mock_time, telemetry_writer, test_agent_session): From 9489e5ecc2129f7e1d684ba46a362afde575f86b Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 20 Sep 2024 13:48:44 -0400 Subject: [PATCH 04/12] remove flaky duplicate test --- tests/tracer/test_processors.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/tracer/test_processors.py b/tests/tracer/test_processors.py index a2587223a52..37ed707c63f 100644 --- a/tests/tracer/test_processors.py +++ b/tests/tracer/test_processors.py @@ -371,21 +371,6 @@ def test_span_creation_metrics(): ) -def test_span_creation_metrics_disabled_telemetry(): - """Test that telemetry metrics are not queued when telemetry is disabled""" - aggr = SpanAggregator( - partial_flush_enabled=False, partial_flush_min_spans=0, trace_processors=[], writer=DummyWriter() - ) - - with override_global_config(dict(_telemetry_enabled=False)): - with mock.patch("ddtrace.internal.telemetry.telemetry_writer.add_count_metric") as mock_tm: - for _ in range(300): - span = Span("span", on_finish=[aggr.on_span_finish]) - aggr.on_span_start(span) - span.finish() - mock_tm.assert_not_called() - - def test_changing_tracer_sampler_changes_tracesamplingprocessor_sampler(): """Changing the tracer sampler should change the sampling processor's sampler""" tracer = Tracer() From 048bfe268e7777fb7709368b8b1dc810aee13b7c Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Sep 2024 11:32:16 -0400 Subject: [PATCH 05/12] Apply suggestions from code review --- tests/telemetry/test_writer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 5f608c1e162..bf2ab445d8d 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -395,7 +395,8 @@ def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_sessi telemetry_writer.periodic(force_flush=True) events = test_agent_session.get_events("app-dependencies-loaded") assert len(events) == 1 - assert events[0]["payload"]["dependencies"] == [{"name": "httpretty", "version": "1.0.5"}] + deps = [dep["name"] for event in events for dep in event["payload"]["dependencies"]]] + assert "httpretty" in deps def test_update_dependencies_event_not_duplicated(telemetry_writer, test_agent_session, mock_time): From 25492da5c742955049d1e586fb662076cb156282 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Sep 2024 11:32:36 -0400 Subject: [PATCH 06/12] Apply suggestions from code review --- tests/telemetry/test_writer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index bf2ab445d8d..13c0fbbc760 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -394,7 +394,6 @@ def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_sessi # force a flush telemetry_writer.periodic(force_flush=True) events = test_agent_session.get_events("app-dependencies-loaded") - assert len(events) == 1 deps = [dep["name"] for event in events for dep in event["payload"]["dependencies"]]] assert "httpretty" in deps From fb6b7101b3e56f6b7ecd2d763bee5d3e9daf6a4e Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Sep 2024 12:04:15 -0400 Subject: [PATCH 07/12] clean up 2 --- tests/telemetry/test_writer.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 13c0fbbc760..56aff281e9d 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -385,7 +385,8 @@ def test_update_dependencies_event_when_disabled(telemetry_writer, test_agent_se def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_session, mock_time): # Fetch modules to reset the state of seen modules - modules.get_newly_imported_modules() + modules.uninstall_import_hook() + modules.install_import_hook() telemetry_writer.reset_queues() assert telemetry_writer._imported_dependencies == {} del sys.modules["httpretty"] @@ -394,7 +395,7 @@ def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_sessi # force a flush telemetry_writer.periodic(force_flush=True) events = test_agent_session.get_events("app-dependencies-loaded") - deps = [dep["name"] for event in events for dep in event["payload"]["dependencies"]]] + deps = [dep["name"] for event in events for dep in event["payload"].get("dependencies", [])] assert "httpretty" in deps From b0c84f56bfed3f570823b56610892769cedc0432 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 26 Sep 2024 16:26:52 -0400 Subject: [PATCH 08/12] use subprocess --- tests/telemetry/test_writer.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 56aff281e9d..99142617059 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -383,19 +383,18 @@ def test_update_dependencies_event_when_disabled(telemetry_writer, test_agent_se assert event["request_type"] != "app-dependencies-loaded" -def test_update_dependencies_event_not_stdlib(telemetry_writer, test_agent_session, mock_time): - # Fetch modules to reset the state of seen modules - modules.uninstall_import_hook() - modules.install_import_hook() - telemetry_writer.reset_queues() - assert telemetry_writer._imported_dependencies == {} - del sys.modules["httpretty"] - import httpretty # noqa F401 +def test_update_dependencies_event_not_stdlib(test_agent_session, ddtrace_run_python_code_in_subprocess): + env = os.environ.copy() + # app-started events are sent 10 seconds after ddtrace imported, this configuration overrides this + # behavior to force the app-started event to be queued immediately + env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" - # force a flush - telemetry_writer.periodic(force_flush=True) + # Import httppretty after ddtrace is imported, this ensures that the module is sent in a dependencies event + _, stderr, status, _ = ddtrace_run_python_code_in_subprocess("import ddtrace; import httpretty", env=env) + assert status == 0, stderr events = test_agent_session.get_events("app-dependencies-loaded") - deps = [dep["name"] for event in events for dep in event["payload"].get("dependencies", [])] + assert len(events) >= 1 + deps = [dep["name"] for event in events for dep in event["payload"]["dependencies"]] assert "httpretty" in deps From a63de2864c9c666f72201af53a17bbd7d98489ee Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 27 Sep 2024 23:20:22 -0400 Subject: [PATCH 09/12] telemetry events are not sent on first tracer flush (anymore) --- tests/telemetry/test_telemetry.py | 42 +++---------------------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/tests/telemetry/test_telemetry.py b/tests/telemetry/test_telemetry.py index 28cb4453466..520568cff91 100644 --- a/tests/telemetry/test_telemetry.py +++ b/tests/telemetry/test_telemetry.py @@ -21,34 +21,6 @@ def test_enable(test_agent_session, run_python_code_in_subprocess): assert stderr == b"" -@pytest.mark.snapshot -def test_telemetry_enabled_on_first_tracer_flush(test_agent_session, ddtrace_run_python_code_in_subprocess): - """assert telemetry events are generated after the first trace is flushed to the agent""" - - # Submit a trace to the agent in a subprocess - code = """ -from ddtrace import tracer - -span = tracer.trace("test-telemetry") -span.finish() - """ - env = os.environ.copy() - env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" - _, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env) - assert status == 0, stderr - assert stderr == b"" - # Ensure telemetry events were sent to the agent (snapshot ensures one trace was generated) - # Note event order is reversed e.g. event[0] is actually the last event - events = test_agent_session.get_events() - - assert len(events) == 5 - assert events[0]["request_type"] == "app-closing" - assert events[1]["request_type"] == "app-dependencies-loaded" - assert events[2]["request_type"] == "app-integrations-change" - assert events[3]["request_type"] == "generate-metrics" - assert events[4]["request_type"] == "app-started" - - def test_enable_fork(test_agent_session, run_python_code_in_subprocess): """assert app-started/app-closing events are only sent in parent process""" code = """ @@ -183,9 +155,6 @@ def test_app_started_error_handled_exception(test_agent_session, run_python_code from ddtrace import tracer from ddtrace.filters import TraceFilter -from ddtrace.settings import _config - -_config._telemetry_dependency_collection = False class FailingFilture(TraceFilter): def process_trace(self, trace): @@ -197,10 +166,10 @@ def process_trace(self, trace): } ) -# generate and encode span +# generate and encode span to trigger sampling failure tracer.trace("hello").finish() -# force app_started call instead of waiting for periodic() +# force app_started event (instead of waiting for 10 seconds) from ddtrace.internal.telemetry import telemetry_writer telemetry_writer._app_started() """ @@ -273,9 +242,6 @@ def test_handled_integration_error(test_agent_session, run_python_code_in_subpro from ddtrace import patch, tracer patch(raise_errors=False, sqlite3=True) - -# Create a span to start the telemetry writer -tracer.trace("hi").finish() """ env = os.environ.copy() @@ -373,7 +339,7 @@ def test_app_started_with_install_metrics(test_agent_session, run_python_code_in } ) # Generate a trace to trigger app-started event - _, stderr, status, _ = run_python_code_in_subprocess("import ddtrace; ddtrace.tracer.trace('s1').finish()", env=env) + _, stderr, status, _ = run_python_code_in_subprocess("import ddtrace", env=env) assert status == 0, stderr app_started_event = test_agent_session.get_events("app-started") @@ -392,8 +358,6 @@ def test_instrumentation_telemetry_disabled(test_agent_session, run_python_code_ code = """ from ddtrace import tracer -# Create a span to start the telemetry writer -tracer.trace("hi").finish() # We want to import the telemetry module even when telemetry is disabled. import sys From 18a2a6b8535ddeadd6f2912216fc13984c0f19dc Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 27 Sep 2024 23:33:00 -0400 Subject: [PATCH 10/12] run deps test in subprocess --- tests/telemetry/test_data.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/telemetry/test_data.py b/tests/telemetry/test_data.py index 0b6cf354233..7c607287d26 100644 --- a/tests/telemetry/test_data.py +++ b/tests/telemetry/test_data.py @@ -13,7 +13,6 @@ from ddtrace.internal.telemetry.data import get_application from ddtrace.internal.telemetry.data import get_host_info from ddtrace.internal.telemetry.data import get_hostname -from ddtrace.internal.telemetry.data import update_imported_dependencies def test_get_application(): @@ -173,7 +172,10 @@ def test_get_container_id_when_container_does_not_exists(): assert _get_container_id() == "" +@pytest.mark.subprocess def test_update_imported_dependencies_both_empty(): + from ddtrace.internal.telemetry.data import update_imported_dependencies + already_imported = {} new_modules = [] res = update_imported_dependencies(already_imported, new_modules) @@ -182,9 +184,12 @@ def test_update_imported_dependencies_both_empty(): assert new_modules == [] +@pytest.mark.subprocess def test_update_imported_dependencies(): import xmltodict + from ddtrace.internal.telemetry.data import update_imported_dependencies + already_imported = {} res = update_imported_dependencies(already_imported, [xmltodict.__name__]) assert len(res) == 1 From cd7e94f7086791d71664bc30c9d281892d91a431 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Sat, 28 Sep 2024 00:06:33 -0400 Subject: [PATCH 11/12] improve reliability of telemetry event/metrics counts --- tests/conftest.py | 9 ++++ tests/telemetry/test_telemetry.py | 34 ++++++--------- tests/telemetry/test_telemetry_metrics_e2e.py | 42 ++++++------------- tests/telemetry/test_writer.py | 9 +--- 4 files changed, 36 insertions(+), 58 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 27e53842411..4f85a4afc5d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -558,6 +558,15 @@ def get_events(self, event_type=None, filter_heartbeats=True): requests = self.get_requests(event_type, filter_heartbeats) return [req["body"] for req in requests] + def get_metrics(self, name): + metrics = [] + for event in self.get_events("generate-metrics"): + for series in event["payload"]["series"]: + if series["metric"] == name: + metrics.append(series) + metrics.sort(key=lambda x: (x["metric"], x["tags"]), reverse=False) + return metrics + @pytest.fixture def test_agent_session(telemetry_writer, request): diff --git a/tests/telemetry/test_telemetry.py b/tests/telemetry/test_telemetry.py index 520568cff91..509017203d7 100644 --- a/tests/telemetry/test_telemetry.py +++ b/tests/telemetry/test_telemetry.py @@ -260,19 +260,12 @@ def test_handled_integration_error(test_agent_session, run_python_code_in_subpro ) # Get metric containing the integration error - integration_error = {} - metric_events = test_agent_session.get_events("generate-metrics") - for event in metric_events: - for metric in event["payload"]["series"]: - if metric["metric"] == "integration_errors": - integration_error = metric - break - + integration_error = test_agent_session.get_metrics("integration_errors") # assert the integration metric has the correct type, count, and tags - assert integration_error - assert integration_error["type"] == "count" - assert integration_error["points"][0][1] == 1 - assert integration_error["tags"] == ["integration_name:sqlite3", "error_type:attributeerror"] + assert len(integration_error) == 1 + assert integration_error[0]["type"] == "count" + assert integration_error[0]["points"][0][1] == 1 + assert integration_error[0]["tags"] == ["integration_name:sqlite3", "error_type:attributeerror"] def test_unhandled_integration_error(test_agent_session, ddtrace_run_python_code_in_subprocess): @@ -316,16 +309,13 @@ def test_unhandled_integration_error(test_agent_session, ddtrace_run_python_code assert "ddtrace/contrib/internal/flask/patch.py:" in flask_integration["error"] assert "not enough values to unpack (expected 2, got 0)" in flask_integration["error"] - metric_events = [event for event in events if event["request_type"] == "generate-metrics"] - - assert len(metric_events) == 1 - assert metric_events[0]["payload"]["namespace"] == "tracers" - assert len(metric_events[0]["payload"]["series"]) == 1 - assert metric_events[0]["payload"]["series"][0]["metric"] == "integration_errors" - assert metric_events[0]["payload"]["series"][0]["type"] == "count" - assert len(metric_events[0]["payload"]["series"][0]["points"]) == 1 - assert metric_events[0]["payload"]["series"][0]["points"][0][1] == 1 - assert metric_events[0]["payload"]["series"][0]["tags"] == ["integration_name:flask", "error_type:valueerror"] + error_metrics = test_agent_session.get_metrics("integration_errors") + assert len(error_metrics) == 1 + error_metric = error_metrics[0] + assert error_metric["type"] == "count" + assert len(error_metric["points"]) == 1 + assert error_metric["points"][0][1] == 1 + assert error_metric["tags"] == ["integration_name:flask", "error_type:valueerror"] def test_app_started_with_install_metrics(test_agent_session, run_python_code_in_subprocess): diff --git a/tests/telemetry/test_telemetry_metrics_e2e.py b/tests/telemetry/test_telemetry_metrics_e2e.py index 6e8b833e310..77b10c508ef 100644 --- a/tests/telemetry/test_telemetry_metrics_e2e.py +++ b/tests/telemetry/test_telemetry_metrics_e2e.py @@ -74,7 +74,6 @@ def parse_payload(data): def test_telemetry_metrics_enabled_on_gunicorn_child_process(test_agent_session): token = "tests.telemetry.test_telemetry_metrics_e2e.test_telemetry_metrics_enabled_on_gunicorn_child_process" - initial_event_count = len(test_agent_session.get_events("generate-metrics")) with gunicorn_server(telemetry_metrics_enabled="true", token=token) as context: _, gunicorn_client = context @@ -86,11 +85,12 @@ def test_telemetry_metrics_enabled_on_gunicorn_child_process(test_agent_session) response = gunicorn_client.get("/count_metric") assert response.status_code == 200 - metrics = test_agent_session.get_events("generate-metrics") - assert len(metrics) > initial_event_count - assert len(metrics) == 1 - assert metrics[0]["payload"]["series"][0]["metric"] == "test_metric" - assert metrics[0]["payload"]["series"][0]["points"][0][1] == 5 + # Ensure /count_metric was called 5 times (these counts could be sent in different payloads) + metrics = test_agent_session.get_metrics("test_metric") + count = 0 + for metric in metrics: + count += metric["points"][0][1] + assert count == 5 def test_span_creation_and_finished_metrics_datadog(test_agent_session, ddtrace_run_python_code_in_subprocess): @@ -104,14 +104,13 @@ def test_span_creation_and_finished_metrics_datadog(test_agent_session, ddtrace_ env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" _, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env) assert status == 0, stderr - metrics_events = test_agent_session.get_events("generate-metrics") - metrics_sc = get_metrics_from_events("spans_created", metrics_events) + metrics_sc = test_agent_session.get_metrics("spans_created") assert len(metrics_sc) == 1 assert metrics_sc[0]["metric"] == "spans_created" assert metrics_sc[0]["tags"] == ["integration_name:datadog"] assert metrics_sc[0]["points"][0][1] == 10 - metrics_sf = get_metrics_from_events("spans_finished", metrics_events) + metrics_sf = test_agent_session.get_metrics("spans_finished") assert len(metrics_sf) == 1 assert metrics_sf[0]["metric"] == "spans_finished" assert metrics_sf[0]["tags"] == ["integration_name:datadog"] @@ -133,15 +132,13 @@ def test_span_creation_and_finished_metrics_otel(test_agent_session, ddtrace_run _, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env) assert status == 0, stderr - metrics_events = test_agent_session.get_events("generate-metrics") - - metrics_sc = get_metrics_from_events("spans_created", metrics_events) + metrics_sc = test_agent_session.get_metrics("spans_created") assert len(metrics_sc) == 1 assert metrics_sc[0]["metric"] == "spans_created" assert metrics_sc[0]["tags"] == ["integration_name:otel"] assert metrics_sc[0]["points"][0][1] == 9 - metrics_sf = get_metrics_from_events("spans_finished", metrics_events) + metrics_sf = test_agent_session.get_metrics("spans_finished") assert len(metrics_sf) == 1 assert metrics_sf[0]["metric"] == "spans_finished" assert metrics_sf[0]["tags"] == ["integration_name:otel"] @@ -163,15 +160,13 @@ def test_span_creation_and_finished_metrics_opentracing(test_agent_session, ddtr _, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env) assert status == 0, stderr - metrics_events = test_agent_session.get_events("generate-metrics") - - metrics_sc = get_metrics_from_events("spans_created", metrics_events) + metrics_sc = test_agent_session.get_metrics("spans_created") assert len(metrics_sc) == 1 assert metrics_sc[0]["metric"] == "spans_created" assert metrics_sc[0]["tags"] == ["integration_name:opentracing"] assert metrics_sc[0]["points"][0][1] == 2 - metrics_sf = get_metrics_from_events("spans_finished", metrics_events) + metrics_sf = test_agent_session.get_metrics("spans_finished") assert len(metrics_sf) == 1 assert metrics_sf[0]["metric"] == "spans_finished" assert metrics_sf[0]["tags"] == ["integration_name:opentracing"] @@ -202,8 +197,7 @@ def test_span_creation_no_finish(test_agent_session, ddtrace_run_python_code_in_ _, stderr, status, _ = ddtrace_run_python_code_in_subprocess(code, env=env) assert status == 0, stderr - metrics_events = test_agent_session.get_events("generate-metrics") - metrics = get_metrics_from_events("spans_created", metrics_events) + metrics = test_agent_session.get_metrics("spans_created") assert len(metrics) == 3 assert metrics[0]["metric"] == "spans_created" @@ -215,13 +209,3 @@ def test_span_creation_no_finish(test_agent_session, ddtrace_run_python_code_in_ assert metrics[2]["metric"] == "spans_created" assert metrics[2]["tags"] == ["integration_name:otel"] assert metrics[2]["points"][0][1] == 4 - - -def get_metrics_from_events(name, events): - metrics = [] - for event in events: - for series in event["payload"]["series"]: - if series["metric"] == name: - metrics.append(series) - metrics.sort(key=lambda x: (x["metric"], x["tags"]), reverse=False) - return metrics diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 99142617059..6416e8b1a58 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -479,8 +479,7 @@ def test_app_client_configuration_changed_event(telemetry_writer, test_agent_ses # force periodic call to flush the first app_started call telemetry_writer.periodic(force_flush=True) """asserts that queuing a configuration sends a valid telemetry request""" - with override_global_config(dict(_telemetry_dependency_collection=False)): - initial_event_count = len(test_agent_session.get_events("app-client-configuration-change")) + with override_global_config(dict()): telemetry_writer.add_configuration("appsec_enabled", True) telemetry_writer.add_configuration("DD_TRACE_PROPAGATION_STYLE_EXTRACT", "datadog") telemetry_writer.add_configuration("appsec_enabled", False, "env_var") @@ -488,12 +487,8 @@ def test_app_client_configuration_changed_event(telemetry_writer, test_agent_ses telemetry_writer.periodic(force_flush=True) events = test_agent_session.get_events("app-client-configuration-change") - assert len(events) >= initial_event_count + 1 - assert events[0]["request_type"] == "app-client-configuration-change" - received_configurations = events[0]["payload"]["configuration"] - # Sort the configuration list by name + received_configurations = [c for event in events for c in event["payload"]["configuration"]] received_configurations.sort(key=lambda c: c["name"]) - # assert the latest configuration value is send to the agent assert received_configurations == [ { From bbd297f2a67e9f614b0c8a63caf18a042cf63f13 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 1 Oct 2024 12:42:32 -0400 Subject: [PATCH 12/12] run all dependency tests in a subprocess --- tests/conftest.py | 13 ++++- tests/telemetry/test_writer.py | 89 +++++++++++++--------------------- 2 files changed, 45 insertions(+), 57 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4f85a4afc5d..82638f4f43f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -558,15 +558,24 @@ def get_events(self, event_type=None, filter_heartbeats=True): requests = self.get_requests(event_type, filter_heartbeats) return [req["body"] for req in requests] - def get_metrics(self, name): + def get_metrics(self, name=None): metrics = [] for event in self.get_events("generate-metrics"): for series in event["payload"]["series"]: - if series["metric"] == name: + if name is None or series["metric"] == name: metrics.append(series) metrics.sort(key=lambda x: (x["metric"], x["tags"]), reverse=False) return metrics + def get_dependencies(self, name=None): + deps = [] + for event in self.get_events("app-dependencies-loaded"): + for dep in event["payload"]["dependencies"]: + if name is None or dep["name"] == name: + deps.append(dep) + deps.sort(key=lambda x: x["name"], reverse=False) + return deps + @pytest.fixture def test_agent_session(telemetry_writer, request): diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 6416e8b1a58..058d01f1c46 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -10,7 +10,6 @@ import pytest import ddtrace.internal.telemetry -from ddtrace.internal.telemetry import modules from ddtrace.internal.telemetry.constants import TELEMETRY_APM_PRODUCT from ddtrace.internal.telemetry.data import get_application from ddtrace.internal.telemetry.data import get_host_info @@ -352,35 +351,32 @@ def test_app_started_event_configuration_override( assert result == expected -def test_update_dependencies_event(telemetry_writer, test_agent_session, mock_time): - import xmltodict +def test_update_dependencies_event(test_agent_session, ddtrace_run_python_code_in_subprocess): + env = os.environ.copy() + # app-started events are sent 10 seconds after ddtrace imported, this configuration overrides this + # behavior to force the app-started event to be queued immediately + env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" - new_deps = [xmltodict.__name__] - telemetry_writer._app_dependencies_loaded_event(new_deps) - # force a flush - telemetry_writer.periodic(force_flush=True) - events = test_agent_session.get_events("app-dependencies-loaded") - assert len(events) >= 1 - xmltodict_events = [e for e in events if e["payload"]["dependencies"][0]["name"] == "xmltodict"] - assert len(xmltodict_events) == 1 - assert "xmltodict" in telemetry_writer._imported_dependencies - assert telemetry_writer._imported_dependencies["xmltodict"] + # Import httppretty after ddtrace is imported, this ensures that the module is sent in a dependencies event + # Imports httpretty twice and ensures only one dependency entry is sent + _, stderr, status, _ = ddtrace_run_python_code_in_subprocess("import xmltodict", env=env) + assert status == 0, stderr + deps = test_agent_session.get_dependencies("xmltodict") + assert len(deps) == 1, deps -def test_update_dependencies_event_when_disabled(telemetry_writer, test_agent_session, mock_time): - with override_global_config(dict(_telemetry_dependency_collection=False)): - # Fetch modules to reset the state of seen modules - modules.get_newly_imported_modules() - - import xmltodict +def test_update_dependencies_event_when_disabled(test_agent_session, ddtrace_run_python_code_in_subprocess): + env = os.environ.copy() + # app-started events are sent 10 seconds after ddtrace imported, this configuration overrides this + # behavior to force the app-started event to be queued immediately + env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" + env["DD_TELEMETRY_DEPENDENCY_COLLECTION_ENABLED"] = "false" - new_deps = [xmltodict.__name__] - telemetry_writer._app_dependencies_loaded_event(new_deps) - # force a flush - telemetry_writer.periodic(force_flush=True) - events = test_agent_session.get_events() - for event in events: - assert event["request_type"] != "app-dependencies-loaded" + # Import httppretty after ddtrace is imported, this ensures that the module is sent in a dependencies event + # Imports httpretty twice and ensures only one dependency entry is sent + _, stderr, status, _ = ddtrace_run_python_code_in_subprocess("import xmltodict") + events = test_agent_session.get_events("app-dependencies-loaded") + assert len(events) == 0, events def test_update_dependencies_event_not_stdlib(test_agent_session, ddtrace_run_python_code_in_subprocess): @@ -390,36 +386,19 @@ def test_update_dependencies_event_not_stdlib(test_agent_session, ddtrace_run_py env["_DD_INSTRUMENTATION_TELEMETRY_TESTS_FORCE_APP_STARTED"] = "true" # Import httppretty after ddtrace is imported, this ensures that the module is sent in a dependencies event - _, stderr, status, _ = ddtrace_run_python_code_in_subprocess("import ddtrace; import httpretty", env=env) + # Imports httpretty twice and ensures only one dependency entry is sent + _, stderr, status, _ = ddtrace_run_python_code_in_subprocess( + """ +import sys +import httpretty +del sys.modules["httpretty"] +import httpretty +""", + env=env, + ) assert status == 0, stderr - events = test_agent_session.get_events("app-dependencies-loaded") - assert len(events) >= 1 - deps = [dep["name"] for event in events for dep in event["payload"]["dependencies"]] - assert "httpretty" in deps - - -def test_update_dependencies_event_not_duplicated(telemetry_writer, test_agent_session, mock_time): - # Fetch modules to reset the state of seen modules - modules.get_newly_imported_modules() - - import xmltodict - - new_deps = [xmltodict.__name__] - telemetry_writer._app_dependencies_loaded_event(new_deps) - # force a flush - telemetry_writer.periodic(force_flush=True) - events = test_agent_session.get_events("app-dependencies-loaded") - assert events[0]["payload"]["dependencies"][0]["name"] == "xmltodict" - - telemetry_writer._app_dependencies_loaded_event(new_deps) - # force a flush - telemetry_writer.periodic(force_flush=True) - events = test_agent_session.get_events("app-dependencies-loaded") - - assert events[0]["seq_id"] == 1 - # only one event must be sent with a non empty payload - # flaky - # assert sum(e["payload"] != {} for e in events) == 1 + deps = test_agent_session.get_dependencies("httpretty") + assert len(deps) == 1, deps def test_app_closing_event(telemetry_writer, test_agent_session, mock_time):