Skip to content

Conversation

@relud
Copy link
Contributor

@relud relud commented Nov 6, 2025

because print writes the ping and newline separately, which increases the chances of something else, like another ping, getting printed between them.

This was trialed in experimenter and fixed the issue with double-pings showing up in the logs.

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • make test runs without emitting any warnings
    • make lint runs without emitting any errors
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to language binding APIs are noted explicitly

because print writes the ping and newline separately, which increases the chances of something else, like another ping, getting printed between them
@relud relud requested a review from a team as a code owner November 6, 2025 03:51
@relud relud requested review from badboy and removed request for a team November 6, 2025 03:51
@relud
Copy link
Contributor Author

relud commented Nov 6, 2025

cc @akkomar

Copy link
Collaborator

@akkomar akkomar left a comment

Choose a reason for hiding this comment

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

TIL!, IIUC it's happening here.

I think this is acceptable workaround.

Ideally we would use mozlog. For some reason I don't remember with the first consumer that used this (Relay) we opted to default to print and allow application to override this. Let's talk about this when we meet later today.

@relud
Copy link
Contributor Author

relud commented Nov 12, 2025

@akkomar anything i can do to help move this forward? I'd like to update experimenter and cirrus to use this.

@akkomar akkomar enabled auto-merge (squash) November 17, 2025 15:41
@akkomar akkomar merged commit 31e9435 into mozilla:main Nov 17, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants