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

Improve quantization #3041

Merged
merged 2 commits into from
Aug 15, 2023
Merged

Improve quantization #3041

merged 2 commits into from
Aug 15, 2023

Conversation

lloeki
Copy link
Member

@lloeki lloeki commented Aug 10, 2023

What does this PR do?

Small bits of improvement to quantization and obfuscation.

Motivation

Stumbled upon a few areas that were lacking.

Additional Notes

Regex has been checked for linearity with the new Ruby 3.3 Regexp.linear_time?.

How to test the change?

env SKIP_SIMPLECOV=1 bundle exec rspec spec/datadog/tracing/contrib/utils/quantization/http_spec.rb

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Aug 10, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #3041 (c9451b7) into master (75cb379) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3041      +/-   ##
==========================================
- Coverage   98.12%   98.11%   -0.01%     
==========================================
  Files        1322     1322              
  Lines       74648    74648              
  Branches     3403     3403              
==========================================
- Hits        73247    73244       -3     
- Misses       1401     1404       +3     
Files Changed Coverage Δ
...datadog/tracing/contrib/utils/quantization/http.rb 83.33% <ø> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ivoanjo
Copy link
Member

ivoanjo commented Aug 10, 2023

Regex has been checked for linearity with the new Ruby 3.3 Regexp.linear_time?.

Maybe consider adding a test for this? We already have 3.3 in CI, and we could skip the test on all other Rubies -- we'd still benefit from no accidental changes :)

@lloeki
Copy link
Member Author

lloeki commented Aug 10, 2023

Maybe consider adding a test for this? We already have 3.3 in CI

I already considered it, and am actually doing it in a separate PR to keep this one small and easy to merge.

@lloeki lloeki marked this pull request as ready for review August 10, 2023 11:24
@lloeki lloeki requested a review from a team August 10, 2023 11:24
@TonyCTHsu TonyCTHsu merged commit 50a2035 into master Aug 15, 2023
@TonyCTHsu TonyCTHsu deleted the improve-quantization branch August 15, 2023 14:29
@TonyCTHsu TonyCTHsu added this to the 1.14.0 milestone Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants