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

[APMSP-1229] Implement new origin detection spec #509

Merged

Conversation

VianneyRuhlmann
Copy link
Contributor

@VianneyRuhlmann VianneyRuhlmann commented Jun 27, 2024

What does this PR do?

Implement the new container origin spec by:

  • Changing cid prefix to ci
  • Supporting the External env variable

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

How to test the change?

Describe here in detail how the change can be validated.

@VianneyRuhlmann VianneyRuhlmann changed the title [Container Tagging] Implement new origin detection spec [APMSP-1229] Implement new origin detection spec Jun 27, 2024
@VianneyRuhlmann VianneyRuhlmann force-pushed the vianney/ddcommon/implement-new-container-origin branch 2 times, most recently from 0138570 to 49698e0 Compare June 27, 2024 09:50
@pr-commenter
Copy link

pr-commenter bot commented Jun 27, 2024

Benchmarks

Comparison

Parameters

Baseline Candidate
config baseline candidate
git_branch main vianney/ddcommon/implement-new-container-origin
git_commit_date 1719591383 1719827601
git_commit_sha 44cedac 4401386
iterations 715.0 712.0
See matching parameters
Baseline Candidate
ci_job_date 1719827841 1719827841
ci_job_id 557745705 557745705
ci_pipeline_id 37963512 37963512
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 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

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
712.0 candidate Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz 1719827841 557745705 37963512 4401386 1719827601 vianney/ddcommon/implement-new-container-origin
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.819µs 70.888µs ± 0.042µs 70.881µs ± 0.033µs 70.923µs 70.956µs 70.975µs 70.977µs 0.13% 0.281 -0.952 0.06% 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 [70.880µs; 70.897µs] or [-0.012%; +0.012%] 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
715.0 baseline Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz 1719827841 557745705 37963512 44cedac 1719591383 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 69.844µs 69.940µs ± 0.046µs 69.931µs ± 0.032µs 69.977µs 70.013µs 70.051µs 70.053µs 0.17% 0.223 -0.508 0.07% 0.005µ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 [69.931µs; 69.949µs] or [-0.013%; +0.013%] 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.
---------------------------------------------------------------------------

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

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

Project coverage is 70.38%. Comparing base (44cedac) to head (4401386).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #509   +/-   ##
=======================================
  Coverage   70.37%   70.38%           
=======================================
  Files         204      204           
  Lines       27824    27835   +11     
=======================================
+ Hits        19581    19591   +10     
- Misses       8243     8244    +1     
Components Coverage Δ
crashtracker 16.70% <ø> (ø)
datadog-alloc 98.76% <ø> (ø)
data-pipeline 51.15% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 86.12% <91.66%> (+0.02%) ⬆️
ddcommon-ffi 74.15% <ø> (ø)
ddtelemetry 59.37% <ø> (ø)
ipc 84.66% <ø> (+<0.01%) ⬆️
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 91.21% <ø> (ø)

@VianneyRuhlmann VianneyRuhlmann force-pushed the vianney/ddcommon/implement-new-container-origin branch from 49698e0 to c206588 Compare June 27, 2024 15:40
@VianneyRuhlmann VianneyRuhlmann marked this pull request as ready for review June 27, 2024 15:41
@VianneyRuhlmann VianneyRuhlmann requested a review from a team as a code owner June 27, 2024 15:41
@VianneyRuhlmann VianneyRuhlmann merged commit f66cd95 into main Jul 1, 2024
32 checks passed
@VianneyRuhlmann VianneyRuhlmann deleted the vianney/ddcommon/implement-new-container-origin branch July 1, 2024 12:24
duncanpharvey pushed a commit that referenced this pull request Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants