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

Refactor to reduce signature errors risks #482

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

VianneyRuhlmann
Copy link
Contributor

What does this PR do?

Refactor the fallback for unix specific functions in entity_id to reduce risks of signature inconsistency between the unix and the fallback implementation.

Motivation

Different signatures between fallback and unix versions fixed by #474

Additional Notes

Only refactor, no functional changes

@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.64%. Comparing base (f77a2dc) to head (262ffbb).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #482      +/-   ##
==========================================
- Coverage   69.65%   69.64%   -0.01%     
==========================================
  Files         198      199       +1     
  Lines       26602    26634      +32     
==========================================
+ Hits        18529    18549      +20     
- Misses       8073     8085      +12     
Components Coverage Δ
crashtracker 17.40% <ø> (ø)
datadog-alloc 98.76% <ø> (ø)
data-pipeline 51.30% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 85.97% <62.50%> (-0.47%) ⬇️
ddcommon-ffi 74.15% <ø> (ø)
ddtelemetry 56.11% <ø> (ø)
ipc 84.61% <ø> (ø)
profiling 79.14% <ø> (ø)
profiling-ffi 59.53% <ø> (ø)
serverless 0.00% <ø> (ø)
sidecar 36.28% <ø> (ø)
sidecar-ffi 0.00% <ø> (ø)
spawn-worker 54.98% <ø> (ø)
trace-mini-agent 69.36% <ø> (ø)
trace-normalization 97.79% <ø> (ø)
trace-obfuscation 95.74% <ø> (ø)
trace-protobuf 24.52% <ø> (ø)
trace-utils 90.80% <ø> (ø)

Copy link
Contributor

@bantonsson bantonsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup.

/// Set the path to cgroup file to mock it during tests
/// # Safety
/// Must not be called in multi-threaded contexts
pub unsafe fn set_cgroup_file(file: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[allow(unused_variables)] 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used _file clippy doesn't complain and it seems a bit more specific (even though in such a small function it doesn't make any difference)

@VianneyRuhlmann VianneyRuhlmann force-pushed the vianney/ddcommon/prevent-signature-errors branch 2 times, most recently from c7780c6 to e8d3329 Compare June 11, 2024 12:35
@VianneyRuhlmann VianneyRuhlmann marked this pull request as ready for review June 11, 2024 12:45
@VianneyRuhlmann VianneyRuhlmann requested a review from a team as a code owner June 11, 2024 12:45
@VianneyRuhlmann VianneyRuhlmann force-pushed the vianney/ddcommon/prevent-signature-errors branch from e8d3329 to 262ffbb Compare June 11, 2024 14:30
@VianneyRuhlmann VianneyRuhlmann merged commit 718e036 into main Jun 11, 2024
27 checks passed
@VianneyRuhlmann VianneyRuhlmann deleted the vianney/ddcommon/prevent-signature-errors branch June 11, 2024 15:53
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.

3 participants