Skip to content

test: disabling telemetry for all tests - #12573

Merged
davidsbatista merged 7 commits into
mainfrom
tests/disable-telemetry-for-tests
Sep 3, 2026
Merged

test: disabling telemetry for all tests#12573
davidsbatista merged 7 commits into
mainfrom
tests/disable-telemetry-for-tests

Conversation

@davidsbatista

@davidsbatista davidsbatista commented Sep 2, 2026 •
edited by julian-risch
Loading

Copy link
Copy Markdown
Contributor

Proposed Changes:

  • haystack/testing/telemetry.py: added block_telemetry_network_calls (and tag_pipeline_telemetry_as_test), imported as autouse fixtures in test/conftest.py and e2e/conftest.py, so they apply to every test in both suites
  • Forces telemetry to a real Telemetry() instance (so the real decorator logic still runs)
  • Replaces posthog.capture with a mock (so nothing is ever sent over the network)

Why

  • All pipeline tests now exercise the real telemetry code path on every Pipeline.run().
  • No test makes a real network call to PostHog anymore (verified empirically — see below).
  • Verified with a temporary global socket block: the full test/ suite (6550 tests) ran with zero connection attempts recorded (main makes 24 attempts to eu.posthog.com from test/core/super_component + test/tools alone).

Why the test_pipeline_run_reports_tagged_event_name test?

  • Telemetry failures are silently swallowed (send_telemetry catches and logs, never raises).
  • So the other tests exercise telemetry but assert nothing about it, they'd pass even if telemetry logic were broken.
  • This test that actually asserts on the telemetry output, it's the one regression check for both the tagging fixture and the real Telemetry.send_event logic.

In practice

  • A test calls pipe.run() → this calls pipeline_running(self), which fixtures have swapped for a wrapper.
  • That wrapper calls telemetry.send_event(), which the fixtures forced to be a real Telemetry object.
  • but send_event calls posthog.capture(...) was replaced with a mock, so the chain ends there instead of hitting the network.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@vercel

vercel Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
haystack-docs Ignored Ignored Preview Sep 3, 2026 7:05am UTC

Request Review

@davidsbatista davidsbatista changed the title disabling telemetry for all tests test: disabling telemetry for all tests Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Coverage report

This PR does not seem to contain any modification to coverable code.

@davidsbatista davidsbatista added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Sep 2, 2026
@davidsbatista
davidsbatista marked this pull request as ready for review September 2, 2026 09:38
@davidsbatista
davidsbatista requested a review from a team as a code owner September 2, 2026 09:38
@davidsbatista
davidsbatista requested review from julian-risch and removed request for a team September 2, 2026 09:38
julian-risch
julian-risch requested changes Sep 2, 2026
View reviewed changes

@julian-risch julian-risch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not send events for tests. We're paying by number of events. There are no insights for us in telemetry events coming from tests, right?

@davidsbatista

davidsbatista commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Let's not send events for tests. We're paying by number of events. There are no insights for us in telemetry events coming from tests, right?

nope

julian-risch
julian-risch requested changes Sep 2, 2026
View reviewed changes

@julian-risch julian-risch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move to test/conftest.py so that we cover also other places in tests where a pipeline is running, incl. test/core/super_component/ (SuperComponent runs an inner pipeline), test/tools/test_pipeline_tool.py, test/components/agents/test_agent.py, and most of test/components/retrievers/

A contributor who followed the documented opt-out gets a fresh telemetry user_id written to their home directory just by running the test suite. We should avoid that.

@julian-risch @claude
test: share telemetry fixtures via haystack.testing.telemetry and kee…
6192d40 
…p config out of $HOME

Move the two autouse fixtures that were duplicated in test/conftest.py and
e2e/conftest.py into haystack/testing/telemetry.py and import them from both.

- Build the Telemetry instance once per session with CONFIG_PATH redirected
  to a tmp_path_factory directory, so running the suite no longer writes a
  fresh telemetry user_id into ~/.haystack/config.yaml - previously it did so
  even for contributors who opted out via HAYSTACK_TELEMETRY_ENABLED=false.
- block_telemetry_network_calls now returns the posthog.capture mock;
  test_telemetry_tagging.py asserts on that mock instead of building its own,
  so the test fails if the network block is removed (it passed before).
- Use inspect.unwrap() instead of .__wrapped__ so the module passes mypy,
  and rename the inner wrapper so it no longer shadows test_pipeline_running.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the type:documentation Improvements on the docs label Sep 3, 2026
julian-risch
julian-risch approved these changes Sep 3, 2026
View reviewed changes

@julian-risch julian-risch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now. I took the liberty to add a commit that builds the Telemetry instance once per session with CONFIG_PATH redirected to a tmp_path_factory directory. That way running the tests no longer writes a fresh telemetry user_id into ~/.haystack/config.yaml.
I also moved the two autouse fixtures because I realized they were now duplicated in test/conftest.py and e2e/conftest.py.

@davidsbatista

davidsbatista commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

nice 👍🏽

@davidsbatista
davidsbatista merged commit 7a82935 into main Sep 3, 2026
25 checks passed
@davidsbatista
davidsbatista deleted the tests/disable-telemetry-for-tests branch September 3, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@julian-risch julian-risch julian-risch approved these changes

Assignees

No one assigned

Labels

ignore-for-release-notes PRs with this flag won't be included in the release notes. topic:tests type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@davidsbatista @julian-risch