Skip to content

Conversation

hsanjuan
Copy link
Contributor

Disable the plugin in cli tests and sharness by default. Enable only in telemetry tests.

There are cases when tests get stuck or get killed and leave daemons hanging around. We don't want to be getting telemetry from those.

@hsanjuan hsanjuan self-assigned this Aug 22, 2025
@hsanjuan hsanjuan requested a review from a team as a code owner August 22, 2025 08:10
@hsanjuan hsanjuan added the skip/changelog This change does NOT require a changelog entry label Aug 22, 2025
Disable the plugin in cli tests and sharness by default. Enable only in
telemetry tests.

There are cases when tests get stuck or get killed and leave daemons hanging around. We don't want to be getting telemetry from those.
@hsanjuan hsanjuan force-pushed the tests-disable-telemetry branch from fc96981 to c8063ec Compare August 22, 2025 08:11
@hsanjuan hsanjuan linked an issue Aug 22, 2025 that may be closed by this pull request
51 tasks
hsanjuan and others added 3 commits August 22, 2025 13:10
The sharness problem is that when the telemetry plugin is configured
initially with 'ipfs config --bool', it creates a structure without
the 'Config: null' field, but when the config is copied and replaced,
it expects the structure to be preserved.

Adding omitempty ensures the Config field is omitted from JSON when
nil, making the config structure consistent between initial creation
and replacement operations.
@lidel
Copy link
Member

lidel commented Aug 22, 2025

@hsanjuan The sharness problem is that when the telemetry plugin is configured initially with ipfs config --bool, it creates a structure without the Config: null field, but when the config is copied and replaced, it expects the structure to be preserved.

iiuc this (changing JSON on roundtrip) was always broken, we just never used it in tests.
Pushed a fix (8b0df49) that should help with this.

@hsanjuan
Copy link
Contributor Author

but when the config is copied and replaced, it expects the structure to be preserved.

Thank you for saving me the time to figure that out

@hsanjuan hsanjuan enabled auto-merge (squash) August 23, 2025 20:32
@lidel lidel disabled auto-merge August 24, 2025 12:28
@lidel lidel changed the title Tests: disable telemetry in tests by default fix: disable telemetry in test profile Aug 24, 2025
@lidel lidel merged commit 15f723a into master Aug 24, 2025
16 checks passed
@lidel lidel deleted the tests-disable-telemetry branch August 24, 2025 12:30
lidel added a commit that referenced this pull request Aug 27, 2025
* Tests: disable telemetry in tests by default

Disable the plugin in cli tests and sharness by default. Enable only in
telemetry tests.

There are cases when tests get stuck or get killed and leave daemons hanging around. We don't want to be getting telemetry from those.

* sharness: attempt to fix

* sharness: add missing --bool flag

* fix(ci): add omitempty to Plugin.Config field

The sharness problem is that when the telemetry plugin is configured
initially with 'ipfs config --bool', it creates a structure without
the 'Config: null' field, but when the config is copied and replaced,
it expects the structure to be preserved.

Adding omitempty ensures the Config field is omitted from JSON when
nil, making the config structure consistent between initial creation
and replacement operations.

---------

Co-authored-by: Marcin Rataj <lidel@lidel.org>
(cherry picked from commit 15f723a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does NOT require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release 0.37
2 participants