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.

Sign up for GitHub

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

Jump to bottom

chore(dev): Have clippy catch debug statements #10125

Merged
merged 3 commits into from
Nov 23, 2021
Merged

chore(dev): Have clippy catch debug statements #10125

merged 3 commits into from
Nov 23, 2021

Conversation

jszwedko
Copy link
Member

@jszwedko jszwedko commented Nov 19, 2021 •
edited
Loading

This PR adds lints for catching stray debug statements: clippy::print_stdout, clippy::print_stderr, and clippy::dbg_macro.

My initial attempt to add this as a lint rule to all crates similar to the other lints we enable that way failed (see rust-lang/rust-clippy#8023). @fuchsnj pointed out that I could pass these globally via flags in .config/cargo though so the subsequent commit does this.

Disabling these lint on a case-by-case basis is pretty verbose because they require wrapping the print macros in a block to apply the attribute, but I think it is worth it to catch stray debug statements before they are released (like #10117).

I had hoped to disable these lints globally for tests since that encompasses most of our usage, but couldn't find a way to do that. I brought this question up in the clippy zulip.

I'm curious to get any thoughts on improving this approach.

Signed-off-by: Jesse Szwedko [email protected]

@jszwedko
chore(dev): Have clippy catch debug statements
5130f38 
This isn't currently working (it didn't catch a println I added).

Signed-off-by: Jesse Szwedko <[email protected]>
@netlify
Copy link

netlify bot commented Nov 19, 2021 •
edited
Loading

✔️ Deploy Preview for vector-project canceled.

🔨 Explore the source changes: 9ec07b2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vector-project/deploys/619c1df2a32de500077d20bf

@github-actions github-actions bot added domain: core Anything related to core crates i.e. vector-core, core-common, etc domain: vrl Anything related to the Vector Remap Language labels Nov 19, 2021
@github-actions
Copy link

Soak Test Results

Baseline: 4a92c2b
Comparison: 5130f38
Total Vector CPUs: 4

What follows is a statistical summary of the soak captures between the SHAs given above. Units are bytes/second/CPU, except for 'skewness' and 'kurtosis'. Higher numbers in 'comparison' is generally better. Higher skewness or kurtosis numbers indicate a lack of consistency in behavior, making predictions of fitness in the field challenging.

datadog_agent_remap_blackhole

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 10.55Mi 10.59Mi 10.59Mi 10.59Mi -0.74 0.23
comparison 10.53Mi 10.56Mi 10.56Mi 10.56Mi 0.05 -0.72

datadog_agent_remap_datadog_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 18.08Mi 18.14Mi 18.16Mi 18.16Mi -0.53 0.59
comparison 19.55Mi 19.60Mi 19.61Mi 19.61Mi 0.24 -1.39

fluent_elasticsearch

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 55.02Mi 55.34Mi 55.39Mi 55.40Mi 0.45 -1.30
comparison 53.94Mi 54.24Mi 54.30Mi 54.30Mi -0.33 -1.26

fluent_remap_aws_firehose

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 39.23Mi 39.52Mi 39.55Mi 39.55Mi -0.04 -1.48
comparison 39.83Mi 39.95Mi 39.98Mi 39.98Mi -0.06 -0.36

splunk_hec_route_s3

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 5.53Mi 5.73Mi 5.81Mi 5.81Mi 0.53 -0.50
comparison 5.37Mi 5.60Mi 5.63Mi 5.64Mi -0.16 -0.73

splunk_transforms_splunk3

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 2.46Mi 2.65Mi 2.68Mi 2.71Mi 0.04 -0.89
comparison 2.51Mi 2.62Mi 2.65Mi 2.65Mi 0.12 -0.43

syslog_humio_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 7.14Mi 7.23Mi 7.23Mi 7.23Mi -0.33 -1.62
comparison 7.09Mi 7.18Mi 7.20Mi 7.20Mi 0.99 -0.32

syslog_log2metric_humio_metrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 4.98Mi 5.05Mi 5.05Mi 5.05Mi -0.99 -0.60
comparison 5.07Mi 5.10Mi 5.11Mi 5.11Mi -0.89 -0.06

syslog_log2metric_splunk_hec_metrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 5.19Mi 5.21Mi 5.21Mi 5.21Mi -0.89 -0.11
comparison 5.12Mi 5.13Mi 5.14Mi 5.15Mi 1.20 0.75

syslog_loki

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 3.93Mi 4.20Mi 4.23Mi 4.24Mi 0.02 -1.42
comparison 3.80Mi 4.29Mi 4.33Mi 4.33Mi -0.60 -0.17

syslog_regex_logs2metric_ddmetrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 3.84Mi 3.85Mi 3.86Mi 3.86Mi 0.40 -0.12
comparison 3.44Mi 3.46Mi 3.46Mi 3.46Mi 0.20 -0.48

syslog_splunk_hec_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 7.12Mi 7.14Mi 7.14Mi 7.14Mi -0.21 -1.43
comparison 7.04Mi 7.08Mi 7.09Mi 7.09Mi -0.04 -0.64

@github-actions github-actions bot added domain: codecs Anything related to Vector's codecs (encoding/decoding) domain: sinks Anything related to the Vector's sinks domain: sources Anything related to the Vector's sources labels Nov 22, 2021
@jszwedko jszwedko requested a review from blt November 22, 2021 22:54
@jszwedko jszwedko marked this pull request as ready for review November 22, 2021 22:54
blt
blt approved these changes Nov 22, 2021
View reviewed changes
lib/vector-core/core-common/src/event_test_util.rs
@@ -18,6 +18,7 @@ pub fn clear_recorded_events() {
pub fn debug_print_events() {
EVENTS_RECORDED.with(|events| {
for event in events.borrow().iter() {
#![allow(clippy::print_stdout)] // that is the purpose of this function
Copy link
Contributor

Choose a reason for hiding this comment

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

:)

@github-actions
Copy link

Soak Test Results

Baseline: 127f05b
Comparison: 9ec07b2
Total Vector CPUs: 4

What follows is a statistical summary of the soak captures between the SHAs given above. Units are bytes/second/CPU, except for 'skewness' and 'kurtosis'. Higher numbers in 'comparison' is generally better. Higher skewness or kurtosis numbers indicate a lack of consistency in behavior, making predictions of fitness in the field challenging.

datadog_agent_remap_blackhole

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 10.62Mi 10.65Mi 10.66Mi 10.66Mi -0.28 -0.53
comparison 10.71Mi 10.73Mi 10.74Mi 10.74Mi 0.02 -0.71

datadog_agent_remap_datadog_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 19.77Mi 19.85Mi 19.86Mi 19.87Mi 0.64 -0.63
comparison 19.23Mi 19.35Mi 19.36Mi 19.37Mi -0.30 -1.49

fluent_elasticsearch

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 52.17Mi 52.44Mi 52.47Mi 52.48Mi -0.38 -0.51
comparison 53.41Mi 53.69Mi 53.71Mi 53.75Mi 0.07 -1.29

fluent_remap_aws_firehose

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 39.07Mi 39.24Mi 39.29Mi 39.29Mi 0.23 -0.59
comparison 39.47Mi 39.65Mi 39.68Mi 39.69Mi -0.49 -0.78

splunk_hec_route_s3

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 5.37Mi 5.57Mi 5.60Mi 5.60Mi -0.43 -0.87
comparison 5.48Mi 5.64Mi 5.68Mi 5.69Mi 0.53 -0.28

splunk_transforms_splunk3

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 2.44Mi 2.67Mi 2.72Mi 2.72Mi 0.11 -1.01
comparison 2.46Mi 2.61Mi 2.63Mi 2.67Mi 0.01 -0.29

syslog_humio_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 7.06Mi 7.08Mi 7.08Mi 7.08Mi -0.33 -0.69
comparison 7.01Mi 7.06Mi 7.07Mi 7.07Mi -0.19 -1.33

syslog_log2metric_humio_metrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 5.01Mi 5.04Mi 5.04Mi 5.04Mi -0.14 -1.26
comparison 5.07Mi 5.10Mi 5.10Mi 5.10Mi 0.48 -0.64

syslog_log2metric_splunk_hec_metrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 5.11Mi 5.12Mi 5.13Mi 5.13Mi 0.36 -0.85
comparison 5.24Mi 5.25Mi 5.26Mi 5.26Mi -0.03 0.16

syslog_loki

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 3.99Mi 4.30Mi 4.32Mi 4.33Mi -0.16 -1.37
comparison 3.86Mi 4.07Mi 4.10Mi 4.10Mi -0.11 -1.58

syslog_regex_logs2metric_ddmetrics

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 3.86Mi 3.87Mi 3.87Mi 3.88Mi 0.34 -0.72
comparison 3.55Mi 3.56Mi 3.57Mi 3.57Mi 0.45 -0.38

syslog_splunk_hec_logs

EXPERIMENT VALUE_min VALUE_p90 VALUE_p99 VALUE_max VALUE_skewness VALUE_kurtosis
baseline 7.00Mi 7.02Mi 7.02Mi 7.02Mi 0.11 -1.22
comparison 6.90Mi 7.05Mi 7.05Mi 7.05Mi -0.34 -1.43

@jszwedko jszwedko merged commit 7f8e41c into master Nov 23, 2021
@jszwedko jszwedko deleted the clippy-debug-denies branch November 23, 2021 18:13
bruceg added a commit that referenced this pull request Dec 14, 2021
@bruceg
chore(dev): Avoid unnecessary cargo check/clippy rebuilds
6e2dfca 
The changes introduced in 7f8e41c (chore(dev): Have clippy catch debug
statements (#10125)) caused cargo to use different `rustflags` between
runs of `cargo clippy` and all other builds. This ended up causing
running `cargo clippy` to clear cached data for the other builds and
vice versa, resulting in some check cycles taking longer than necessary.
Since these flags are just defines, they can be turned on for all
targets, eliminating the problem.
@bruceg bruceg mentioned this pull request Dec 14, 2021
Merged
bruceg added a commit that referenced this pull request Dec 14, 2021
@bruceg
chore(dev): Avoid unnecessary cargo check/clippy rebuilds (#10450)
4897bf3 
The changes introduced in 7f8e41c (chore(dev): Have clippy catch debug
statements (#10125)) caused cargo to use different `rustflags` between
runs of `cargo clippy` and all other builds. This ended up causing
running `cargo clippy` to clear cached data for the other builds and
vice versa, resulting in some check cycles taking longer than necessary.
Since these flags are just defines, they can be turned on for all
targets, eliminating the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blt blt blt approved these changes

Assignees
No one assigned
Labels
domain: codecs Anything related to Vector's codecs (encoding/decoding) domain: core Anything related to core crates i.e. vector-core, core-common, etc domain: sinks Anything related to the Vector's sinks domain: sources Anything related to the Vector's sources domain: vrl Anything related to the Vector Remap Language
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jszwedko @blt