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 DD_DOGSTATSD_URL support for unix and udp urls #870

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ddrthall
Copy link

@ddrthall ddrthall commented Nov 18, 2024

Adds support for the unix:// and udp:// variants of DD_DOGSTATSD_URL.

Description of the Change

During dogstatsd creation, utilize the DD_DOGSTATSD_URL environment variable if the provided host and port are their default values and no socket_path has been provided. unix:// and udp:// paths are supported, but \\.\pipe\ paths (windows named pipes) are not yet implemented.

Alternate Designs

Possible Drawbacks

Verification Process

Form a Dogstatsd object with different values of DD_DOGSTATSD_URL. The following cases are important to verify:

  • A non-default host, port or socket_path provided to the Dogstatsd constructor should take precedence over DD_DOGSTATS_URL.
  • unix:// prefixed DOGSTATD_URLS should result in a socket_path and a None host/port pair
  • udp:// prefixed DOGSTATSD_URLS should result in a valid host/port pair, with appropriate type safety around casting the port component from a string to an integer.
  • \\.\pipe\ prefixed DOGSTATD_URLS should provide an informative debug message and fallback to other connection identifications.
  • An unrecognized prefix for DOGSTATD_URL should fallback to other connection identifications

During fallback, DD_AGENT_HOST and DD_DOGSTATSD_PORT should be utilized if present. Otherwise, udp is selected with DEFAULT_HOST:DEFAULT_PORT.

Additional Notes

This PR is reliant on the functionality in !869, with a comparison available here

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.

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.
@ddrthall ddrthall added the changelog/Added Added features results into a minor version bump label Nov 18, 2024
@github-actions github-actions bot added the documentation Documentation related changes label Nov 18, 2024
@ddrthall ddrthall force-pushed the ryan.hall/add_dogstatsd_url_support branch from 0ed5e91 to 7a4ec25 Compare November 18, 2024 21:13
Adds support for the unix:// and udp:// variants of DD_DOGSTATSD_URL.
Will only be applied if the host and port are their default values
and no socket_path has been provided.
@ddrthall ddrthall force-pushed the ryan.hall/add_dogstatsd_url_support branch from 7a4ec25 to 37f0173 Compare November 18, 2024 21:16
@ddrthall ddrthall added the do-not-merge/HOLD Do not merge this PR label Nov 18, 2024
@ddrthall ddrthall marked this pull request as ready for review November 18, 2024 21:19
@ddrthall ddrthall requested review from a team as code owners November 18, 2024 21:19
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 do-not-merge/HOLD Do not merge this PR documentation Documentation related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant