Skip to content

Conversation

@rayz
Copy link
Contributor

@rayz rayz commented Jul 15, 2025

What does this PR do?

Fixes bug where self._transport was never set, thus causing self._flush_telemetry to fail when adding the client_transport tag.

Description of the Change

Sets self._transport to udp as a default.

Alternate Designs

Possible Drawbacks

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@rayz rayz requested review from a team as code owners July 15, 2025 20:46
@rayz rayz changed the title Set transport even if socket fails Fix missing transport attribute in when flushing telemetry Jul 15, 2025
@rayz rayz changed the title Fix missing transport attribute in when flushing telemetry Fix missing transport attribute when flushing telemetry Jul 15, 2025
@rayz rayz requested a review from a team as a code owner July 15, 2025 20:49
@github-actions github-actions bot added the documentation Documentation related changes label Jul 15, 2025
@rayz rayz added changelog/Fixed Fixed features results into a bug fix version bump and removed documentation Documentation related changes labels Jul 15, 2025
iadjivon
iadjivon previously approved these changes Jul 15, 2025
Copy link

@iadjivon iadjivon left a comment

Choose a reason for hiding this comment

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

Approved!

else:
self._transport = "uds"
else:
self._transport = "udp"
Copy link
Contributor

Choose a reason for hiding this comment

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

This repeats the code already written not even 10 lines before. Would it make sense to avoid copy-pasting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the code just set transport to udp for consistency as we already default to using the UDP optimal payload length when socket is None. Let me know if that works with you.

@github-actions github-actions bot added the documentation Documentation related changes label Jul 16, 2025
@carlosroman carlosroman self-requested a review July 23, 2025 15:06
@rayz rayz requested a review from iadjivon July 23, 2025 15:08
Copy link

@iadjivon iadjivon left a comment

Choose a reason for hiding this comment

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

🚢

@rayz rayz merged commit 755d728 into master Jul 25, 2025
11 checks passed
@rayz rayz deleted the rayz/fix-missing-transport-attribute branch July 25, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/Fixed Fixed features results into a bug fix version bump documentation Documentation related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants