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

Bump rustls version to 0.22, hyper-rustls to 0.25, tokio-rustls to 0.25 in datadog-trace-utils and ddcommon #503

Conversation

duncanpharvey
Copy link
Contributor

@duncanpharvey duncanpharvey commented Jun 25, 2024

What does this PR do?

  • Bumps hyper-rustls to 0.25 for datadog-trace-utils
  • Bumps hyper-rustls to 0.25 for ddcommon
  • Bumps rustls to 0.22 for ddcommon
  • Bumps tokio-rustls to 0.25 for ddcommon

Motivation

Resolve BadCertificate errors when running the Serverless Mini Agent in a Node Azure Function on Windows.

2024-06-25T17:19:05Z   [Information]   [2024-06-25T17:19:05Z WARN  rustls::conn] Sending fatal alert BadCertificate
2024-06-25T17:19:05Z   [Information]   [2024-06-25T17:19:05Z ERROR datadog_trace_mini_agent::stats_flusher] Error sending stats: Failed to send trace stats: error trying to connect: invalid peer certificate contents: invalid peer certificate: UnknownIssuer

Additional Notes

How to test the change?

  • Build mini agent locally
  • Deploy Azure Function with mini agent binary in root path
  • Set DD_MINI_AGENT_PATH
    • Azure: /home/site/wwwroot/datadog-serverless-trace-mini-agent

@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.40%. Comparing base (c00ab02) to head (31423b7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #503      +/-   ##
==========================================
- Coverage   70.40%   70.40%   -0.01%     
==========================================
  Files         204      204              
  Lines       27882    27881       -1     
==========================================
- Hits        19630    19629       -1     
  Misses       8252     8252              
Components Coverage Δ
crashtracker 16.70% <ø> (ø)
datadog-alloc 98.76% <ø> (ø)
data-pipeline 51.15% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 86.48% <100.00%> (-0.01%) ⬇️
ddcommon-ffi 74.15% <ø> (ø)
ddtelemetry 59.37% <ø> (ø)
ipc 84.66% <ø> (ø)
profiling 78.63% <ø> (ø)
profiling-ffi 58.19% <ø> (ø)
serverless 0.00% <ø> (ø)
sidecar 36.19% <ø> (ø)
sidecar-ffi 0.00% <ø> (ø)
spawn-worker 54.98% <ø> (ø)
trace-mini-agent 69.70% <ø> (ø)
trace-normalization 97.79% <ø> (ø)
trace-obfuscation 95.75% <ø> (ø)
trace-protobuf 77.16% <ø> (ø)
trace-utils 90.84% <0.00%> (ø)

@duncanpharvey duncanpharvey changed the title Duncan harvey/datadog trace utils bump hyper rustls version Bump rustls version to 0.22, hyper-rsutls to 0.25, tokio-rustls to 0.25 in datadog-trace-utils and ddcommon Jun 26, 2024
@duncanpharvey duncanpharvey changed the title Bump rustls version to 0.22, hyper-rsutls to 0.25, tokio-rustls to 0.25 in datadog-trace-utils and ddcommon Bump rustls version to 0.22, hyper-rustls to 0.25, tokio-rustls to 0.25 in datadog-trace-utils and ddcommon Jun 26, 2024
@pr-commenter
Copy link

pr-commenter bot commented Jul 2, 2024

Benchmarks

Comparison

Parameters

Baseline Candidate
config baseline candidate
git_branch main duncan-harvey/datadog-trace-utils-bump-hyper-rustls-version
git_commit_date 1719935341 1719955994
git_commit_sha c00ab02 31423b7
iterations 711.0 678.0
See matching parameters
Baseline Candidate
ci_job_date 1719956233 1719956233
ci_job_id 560443870 560443870
ci_pipeline_id 38204893 38204893
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz

Summary

Found 0 performance improvements and 1 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics.

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:sql/obfuscate_sql_string worse
[+3.375µs; +3.394µs] or [+4.794%; +4.822%]
73.778µs 70.393µs

Candidate

Candidate benchmark details

Group 1

iterations config cpu_model ci_job_date ci_job_id ci_pipeline_id git_commit_sha git_commit_date git_branch
678.0 candidate Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz 1719956233 560443870 38204893 31423b7 1719955994 duncan-harvey/datadog-trace-utils-bump-hyper-rustls-version
scenario metric min mean ± sd median ± mad p75 p95 p99 max peak_to_median_ratio skewness kurtosis cv sem runs sample_size
sql/obfuscate_sql_string execution_time 73.669µs 73.778µs ± 0.041µs 73.781µs ± 0.028µs 73.807µs 73.835µs 73.857µs 73.863µs 0.11% -0.365 -0.392 0.05% 0.004µs 1 100
scenario metric 95% CI mean Shapiro-Wilk pvalue Ljung-Box pvalue (lag=1) Dip test pvalue
sql/obfuscate_sql_string execution_time [73.770µs; 73.786µs] or [-0.011%; +0.011%] None None None

Warnings

Scenario: sql/obfuscate_sql_string, Metric: execution_time

Measurements are autocorrelated.

Autocorrelation is present for lags 1..10.

The measurements are not independent, thus confidence intervals
may be less precise.
---------------------------------------------------------------------------
Scenario: sql/obfuscate_sql_string, Metric: execution_time

Sample size is 100, which is lower than 105.

The minimal sample size in case of normal distribution to achieve significance
level of 0.05 for difference of means with effect size Cohen's d = 0.5 must be at
least 105.

The conclusions from confidence intervals may be invalid.
---------------------------------------------------------------------------

Baseline

Baseline benchmark details

Group 1

iterations config cpu_model ci_job_date ci_job_id ci_pipeline_id git_commit_sha git_commit_date git_branch
711.0 baseline Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz 1719956233 560443870 38204893 c00ab02 1719935341 main
scenario metric min mean ± sd median ± mad p75 p95 p99 max peak_to_median_ratio skewness kurtosis cv sem runs sample_size
sql/obfuscate_sql_string execution_time 70.303µs 70.393µs ± 0.028µs 70.398µs ± 0.017µs 70.412µs 70.435µs 70.448µs 70.449µs 0.07% -0.538 0.217 0.04% 0.003µs 1 100
scenario metric 95% CI mean Shapiro-Wilk pvalue Ljung-Box pvalue (lag=1) Dip test pvalue
sql/obfuscate_sql_string execution_time [70.388µs; 70.399µs] or [-0.008%; +0.008%] None None None

Warnings

Scenario: sql/obfuscate_sql_string, Metric: execution_time

Measurements are autocorrelated.

Autocorrelation is present for lags 1..10.

The measurements are not independent, thus confidence intervals
may be less precise.
---------------------------------------------------------------------------
Scenario: sql/obfuscate_sql_string, Metric: execution_time

Sample size is 100, which is lower than 105.

The minimal sample size in case of normal distribution to achieve significance
level of 0.05 for difference of means with effect size Cohen's d = 0.5 must be at
least 105.

The conclusions from confidence intervals may be invalid.
---------------------------------------------------------------------------

@bwoebi
Copy link
Contributor

bwoebi commented Jul 9, 2024

I suppose we can close this now that #459 is merged. (Bumping to rustls to 0.23, hyper-rustls to 0.27 and tokio-rustls to 0.27).

But maybe this PR did more than just that - to resolve the certificate errors?

@duncanpharvey
Copy link
Contributor Author

@bwoebi Thanks for upgrading rustls in #459! Yes we can close this PR. Some additional changes to fix the certificate related errors in Azure Functions running on Windows were necessary in addition to the rustls upgrade. You can find those changes in the following PR.

#523

@duncanpharvey duncanpharvey deleted the duncan-harvey/datadog-trace-utils-bump-hyper-rustls-version branch July 25, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants