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

[dogstatsd] Fix unicode handling of event text #661

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

sgnn7
Copy link
Contributor

@sgnn7 sgnn7 commented Jun 15, 2021

What does this PR do?

Fixes handling of unicode data when sending events in dogstatsd

Description of the Change

We now properly handle both the type and length calculation of such strings

Closes #660

Alternate Designs

Possible Drawbacks

Tiny reduction in throughput

Verification Process

  1. Add print to string composed in event()
  2. Python2: python2 -m unittest -vvv tests.unit.dogstatsd.test_statsd.TestDogStatsd.test_unicode_event
  3. Python3: python3 -m unittest -vvv tests.unit.dogstatsd.test_statsd.TestDogStatsd.test_unicode_event

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.

@sgnn7 sgnn7 added changelog/Fixed Fixed features results into a bug fix version bump kind/bug Bug related issue severity/normal Normal severity issue labels Jun 15, 2021
@sgnn7 sgnn7 added this to the Next milestone Jun 15, 2021
@sgnn7 sgnn7 self-assigned this Jun 15, 2021
@sgnn7 sgnn7 requested review from a team as code owners June 15, 2021 19:29
@sgnn7 sgnn7 force-pushed the sgnn7/fix-dsd-unicode-processing branch 2 times, most recently from 419b924 to b2bffb6 Compare June 15, 2021 19:47
Old code was not properly handling unicode in messages which included
incorrect length calculation in Python3 and error messages in Python2.
We now properly evaluate those values before sending the data.
@sgnn7 sgnn7 force-pushed the sgnn7/fix-dsd-unicode-processing branch from b2bffb6 to 2dc9948 Compare June 15, 2021 19:49
@therve
Copy link
Contributor

therve commented Jun 16, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@therve therve merged commit dfb9f3a into master Jun 16, 2021
@therve therve deleted the sgnn7/fix-dsd-unicode-processing branch June 16, 2021 07:18
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 kind/bug Bug related issue severity/normal Normal severity issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[statsd] DogStatsd does not calculate correct text/header lengths for unicode
2 participants