Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UDS SOCK_STREAM support to the DogStatsD client #869

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ddrthall
Copy link

@ddrthall ddrthall commented Nov 15, 2024

What does this PR do?

Includes full support for the unix://, unixstream://, and unixgram:// socket_path prefixes utilized by DD_DOGSTATSD_URL in preparation to support that feature.

Autodetects SOCK_DGRAM vs SOCK_STREAM for users currently providing a raw socket path.

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Set up a datadog agent with both a datagram and a stream uds socket path.
Ensure that a client initialized with socket_path={socket_path} and socket_path=unix://{socket_path} can successfully transmit metrics/events/services to both types of sockets when socket_path is an absolute path and when socket_path is a relative path.
Similarly ensure that socket_path=unixstream://{socket_path} can send to the stream socket and socket_path=unixgram://{socket_path} can send to the datagram socket. Confirm for both prefixes that sending to the wrong socket_path produces an appropriate error.
Ensure for all configurations that invalid socket_paths are error handled in a sane manner.

This process should be repeated for the telemetry flow, where telemetry_socket_path is set rather than socket_path.

Additional Notes

Documentation pushing users to utilize unix://{socket_path} over {socket_path} will be introduced on the inclusion of DD_DOGSTATSD_URL support via https://datadoghq.atlassian.net/browse/AMLII-2173

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.

@ddrthall ddrthall added the changelog/Added Added features results into a minor version bump label Nov 15, 2024
@ddrthall ddrthall force-pushed the ryan.hall/add_uds_stream_support branch from 41f1854 to e62632e Compare November 15, 2024 20:15
@ddrthall ddrthall added ci/integrations Run integration tests and removed ci/integrations Run integration tests labels Nov 15, 2024
@ddrthall ddrthall force-pushed the ryan.hall/add_uds_stream_support branch from e62632e to f496719 Compare November 15, 2024 21:12
@ddrthall ddrthall force-pushed the ryan.hall/add_uds_stream_support branch 2 times, most recently from ae4d6be to f8be9f2 Compare November 18, 2024 14:41
@ddrthall ddrthall marked this pull request as ready for review November 18, 2024 14:48
@ddrthall ddrthall requested review from a team as code owners November 18, 2024 14:48
Comment on lines 751 to 755
try:
# This will fail in python2.7
sk_name = socket_kind.name
except AttributeError:
sk_name = socket_kind
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about doing this instead?

Suggested change
try:
# This will fail in python2.7
sk_name = socket_kind.name
except AttributeError:
sk_name = socket_kind
sk_name = { socket.SOCK_STREAM: "stream", socket.SOCK_DGRAM: "datagram" }[socket_kind]

Copy link
Author

Choose a reason for hiding this comment

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

Sold

@ddrthall ddrthall force-pushed the ryan.hall/add_uds_stream_support branch 4 times, most recently from 77d6ced to 767c223 Compare November 18, 2024 19:10
Includes full support for the unix://, unixstream://, and unixgram:// socket_path prefixes
utilized by DD_DOGSTATSD_URL in preparation to support that feature.

Autodetects SOCK_DGRAM vs SOCK_STREAM for users currently providing a raw socket path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Added Added features results into a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants