-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(proto): Remove error log when source_event_id is not present #21257
Conversation
lib/vector-core/src/event/proto.rs
Outdated
@@ -677,8 +677,6 @@ impl From<Metadata> for EventMetadata { | |||
|
|||
if let Ok(uuid) = Uuid::from_slice(&value.source_event_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should source_event_id
have been an option like the other fields here so that we can detect when it is present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea makes sense, can convert to optional
EDIT: Following up from offline discussion, will keep the proto definition as is and change the value in EventMetadata
to optional
Datadog ReportBranch report: ✅ 0 Failed, 7 Passed, 0 Skipped, 25.49s Total Time |
…al in EventMetadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this was a tricky one!
Regression Detector ResultsRun ID: 4d606864-f607-4779-9e8a-a2d0f429b5f5 Metrics dashboard Baseline: f94c28c Performance changes are noted in the perf column of each table:
No significant changes in experiment optimization goalsConfidence level: 90.00% There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.
|
perf | experiment | goal | Δ mean % | Δ mean % CI | links |
---|---|---|---|---|---|
✅ | file_to_blackhole | egress throughput | +16.30 | [+8.77, +23.82] |
Fine details of change detection per experiment
perf | experiment | goal | Δ mean % | Δ mean % CI | links |
---|---|---|---|---|---|
✅ | file_to_blackhole | egress throughput | +16.30 | [+8.77, +23.82] | |
➖ | otlp_http_to_blackhole | ingress throughput | +4.20 | [+4.08, +4.33] | |
➖ | socket_to_socket_blackhole | ingress throughput | +3.83 | [+3.77, +3.89] | |
➖ | syslog_regex_logs2metric_ddmetrics | ingress throughput | +3.63 | [+3.47, +3.80] | |
➖ | http_to_http_acks | ingress throughput | +2.73 | [+1.46, +3.99] | |
➖ | splunk_hec_route_s3 | ingress throughput | +2.24 | [+1.93, +2.55] | |
➖ | syslog_loki | ingress throughput | +2.07 | [+1.98, +2.16] | |
➖ | syslog_log2metric_splunk_hec_metrics | ingress throughput | +1.37 | [+1.27, +1.48] | |
➖ | datadog_agent_remap_datadog_logs_acks | ingress throughput | +0.84 | [+0.66, +1.02] | |
➖ | otlp_grpc_to_blackhole | ingress throughput | +0.63 | [+0.52, +0.74] | |
➖ | fluent_elasticsearch | ingress throughput | +0.36 | [-0.13, +0.85] | |
➖ | http_elasticsearch | ingress throughput | +0.18 | [+0.01, +0.35] | |
➖ | http_text_to_http_json | ingress throughput | +0.18 | [+0.06, +0.30] | |
➖ | http_to_http_noack | ingress throughput | +0.17 | [+0.08, +0.25] | |
➖ | http_to_s3 | ingress throughput | +0.13 | [-0.14, +0.41] | |
➖ | syslog_log2metric_tag_cardinality_limit_blackhole | ingress throughput | +0.06 | [-0.04, +0.15] | |
➖ | splunk_hec_indexer_ack_blackhole | ingress throughput | +0.04 | [-0.04, +0.12] | |
➖ | http_to_http_json | ingress throughput | +0.03 | [-0.00, +0.07] | |
➖ | splunk_hec_to_splunk_hec_logs_noack | ingress throughput | +0.02 | [-0.07, +0.11] | |
➖ | splunk_hec_to_splunk_hec_logs_acks | ingress throughput | -0.00 | [-0.11, +0.10] | |
➖ | datadog_agent_remap_blackhole_acks | ingress throughput | -0.17 | [-0.27, -0.06] | |
➖ | datadog_agent_remap_blackhole | ingress throughput | -0.39 | [-0.51, -0.27] | |
➖ | syslog_humio_logs | ingress throughput | -0.41 | [-0.54, -0.29] | |
➖ | syslog_splunk_hec_logs | ingress throughput | -0.88 | [-1.00, -0.77] | |
➖ | datadog_agent_remap_datadog_logs | ingress throughput | -1.46 | [-1.68, -1.24] | |
➖ | syslog_log2metric_humio_metrics | ingress throughput | -2.00 | [-2.12, -1.88] |
Explanation
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Regression Detector ResultsRun ID: e0014fed-c091-4ab0-8b01-241c53006c62 Metrics dashboard Baseline: 81fa4e8 Performance changes are noted in the perf column of each table:
No significant changes in experiment optimization goalsConfidence level: 90.00% There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.
|
perf | experiment | goal | Δ mean % | Δ mean % CI | links |
---|---|---|---|---|---|
➖ | file_to_blackhole | egress throughput | +1.72 | [-4.85, +8.30] |
Fine details of change detection per experiment
perf | experiment | goal | Δ mean % | Δ mean % CI | links |
---|---|---|---|---|---|
➖ | socket_to_socket_blackhole | ingress throughput | +4.53 | [+4.46, +4.59] | |
➖ | otlp_http_to_blackhole | ingress throughput | +4.11 | [+3.96, +4.26] | |
➖ | http_to_http_acks | ingress throughput | +2.74 | [+1.48, +4.00] | |
➖ | fluent_elasticsearch | ingress throughput | +2.51 | [+2.01, +3.00] | |
➖ | syslog_log2metric_splunk_hec_metrics | ingress throughput | +2.33 | [+2.24, +2.43] | |
➖ | file_to_blackhole | egress throughput | +1.72 | [-4.85, +8.30] | |
➖ | syslog_humio_logs | ingress throughput | +1.36 | [+1.24, +1.48] | |
➖ | syslog_loki | ingress throughput | +1.25 | [+1.16, +1.33] | |
➖ | splunk_hec_route_s3 | ingress throughput | +1.14 | [+0.83, +1.45] | |
➖ | otlp_grpc_to_blackhole | ingress throughput | +0.57 | [+0.46, +0.67] | |
➖ | syslog_log2metric_humio_metrics | ingress throughput | +0.31 | [+0.18, +0.44] | |
➖ | datadog_agent_remap_blackhole_acks | ingress throughput | +0.27 | [+0.16, +0.38] | |
➖ | http_to_http_noack | ingress throughput | +0.12 | [+0.05, +0.19] | |
➖ | http_to_http_json | ingress throughput | +0.03 | [-0.02, +0.09] | |
➖ | splunk_hec_to_splunk_hec_logs_noack | ingress throughput | -0.00 | [-0.10, +0.09] | |
➖ | splunk_hec_indexer_ack_blackhole | ingress throughput | -0.00 | [-0.09, +0.08] | |
➖ | splunk_hec_to_splunk_hec_logs_acks | ingress throughput | -0.01 | [-0.12, +0.11] | |
➖ | http_to_s3 | ingress throughput | -0.07 | [-0.34, +0.20] | |
➖ | datadog_agent_remap_datadog_logs_acks | ingress throughput | -0.25 | [-0.38, -0.11] | |
➖ | http_text_to_http_json | ingress throughput | -0.33 | [-0.47, -0.20] | |
➖ | syslog_splunk_hec_logs | ingress throughput | -0.35 | [-0.44, -0.27] | |
➖ | http_elasticsearch | ingress throughput | -0.36 | [-0.51, -0.21] | |
➖ | syslog_log2metric_tag_cardinality_limit_blackhole | ingress throughput | -0.48 | [-0.56, -0.40] | |
➖ | datadog_agent_remap_datadog_logs | ingress throughput | -1.03 | [-1.22, -0.84] | |
➖ | datadog_agent_remap_blackhole | ingress throughput | -1.34 | [-1.44, -1.23] | |
➖ | syslog_regex_logs2metric_ddmetrics | ingress throughput | -2.01 | [-2.18, -1.83] |
Explanation
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
#21074 added a new field
source_event_id
which uniquely identifies an event as it passes through different components.Initially made the assumption that
source_event_id
will always be present in proto, and if it is not present we should log an error. However, did not account for the cases such as when the source is a previous version of vector #21252, resulting in a noisy error logs. Removing this log as well as makingsource_event_id
Optional inEventMetadata