fix(config): Make config watcher recursive. #20996
Conversation
There was a problem hiding this comment.
Good catch, thanks @iFurySt !
Could we add:
- A changelog fragment describing the change? See https://github.com/vectordotdev/vector/blob/master/changelog.d/README.md
- A test? You can see one at the bottom of
watcher.rsto use as an example
|
Of course, let me handle it |
Signed-off-by: ifuryst <ifuryst@gmail.com>
eba9045 to
06f4ff4
Compare
|
@jszwedko All requested changes were done. The unit tests passed on my machine. Feel free to review it again when you have time. |
|
I think this just needs a |
Signed-off-by: ifuryst <ifuryst@gmail.com>
Head branch was pushed to by a user without write access
|
@jszwedko Sorry for that, I've fixed it. It looks like the |
Ah, yes, the markdown lint errors have to be fixed manually. I see you did so. Thank you! |
Regression Detector ResultsRun ID: 8b939301-4cfe-4491-a2f9-77d7ea099449 Metrics dashboard Baseline: 93e423f 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 | -6.59 | [-13.51, +0.34] |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | links |
|---|---|---|---|---|---|
| ➖ | syslog_humio_logs | ingress throughput | +3.16 | [+3.03, +3.30] | |
| ➖ | syslog_log2metric_tag_cardinality_limit_blackhole | ingress throughput | +1.02 | [+0.93, +1.10] | |
| ➖ | datadog_agent_remap_datadog_logs_acks | ingress throughput | +0.62 | [+0.45, +0.80] | |
| ➖ | syslog_loki | ingress throughput | +0.46 | [+0.36, +0.57] | |
| ➖ | http_to_s3 | ingress throughput | +0.35 | [+0.09, +0.62] | |
| ➖ | http_text_to_http_json | ingress throughput | +0.33 | [+0.22, +0.44] | |
| ➖ | http_to_http_acks | ingress throughput | +0.25 | [-1.07, +1.57] | |
| ➖ | socket_to_socket_blackhole | ingress throughput | +0.22 | [+0.15, +0.28] | |
| ➖ | http_to_http_noack | ingress throughput | +0.14 | [+0.07, +0.22] | |
| ➖ | otlp_grpc_to_blackhole | ingress throughput | +0.12 | [+0.00, +0.24] | |
| ➖ | http_to_http_json | ingress throughput | +0.02 | [-0.02, +0.05] | |
| ➖ | splunk_hec_to_splunk_hec_logs_noack | ingress throughput | +0.01 | [-0.08, +0.11] | |
| ➖ | splunk_hec_to_splunk_hec_logs_acks | ingress throughput | +0.01 | [-0.09, +0.10] | |
| ➖ | splunk_hec_indexer_ack_blackhole | ingress throughput | -0.02 | [-0.10, +0.06] | |
| ➖ | otlp_http_to_blackhole | ingress throughput | -0.18 | [-0.36, -0.01] | |
| ➖ | syslog_splunk_hec_logs | ingress throughput | -0.47 | [-0.54, -0.39] | |
| ➖ | datadog_agent_remap_blackhole_acks | ingress throughput | -0.74 | [-0.85, -0.63] | |
| ➖ | http_elasticsearch | ingress throughput | -0.86 | [-1.00, -0.72] | |
| ➖ | fluent_elasticsearch | ingress throughput | -0.86 | [-1.35, -0.38] | |
| ➖ | syslog_regex_logs2metric_ddmetrics | ingress throughput | -0.99 | [-1.13, -0.86] | |
| ➖ | syslog_log2metric_splunk_hec_metrics | ingress throughput | -1.20 | [-1.31, -1.09] | |
| ➖ | splunk_hec_route_s3 | ingress throughput | -2.04 | [-2.34, -1.74] | |
| ➖ | datadog_agent_remap_blackhole | ingress throughput | -2.28 | [-2.40, -2.15] | |
| ➖ | syslog_log2metric_humio_metrics | ingress throughput | -2.79 | [-2.94, -2.63] | |
| ➖ | datadog_agent_remap_datadog_logs | ingress throughput | -3.72 | [-3.92, -3.53] | |
| ➖ | file_to_blackhole | egress throughput | -6.59 | [-13.51, +0.34] |
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".
* fix: Make config watcher recursive. Signed-off-by: ifuryst <ifuryst@gmail.com> * fix check-fmt fail. Signed-off-by: ifuryst <ifuryst@gmail.com> --------- Signed-off-by: ifuryst <ifuryst@gmail.com>
* fix: Make config watcher recursive. Signed-off-by: ifuryst <ifuryst@gmail.com> * fix check-fmt fail. Signed-off-by: ifuryst <ifuryst@gmail.com> --------- Signed-off-by: ifuryst <ifuryst@gmail.com>
* fix: Make config watcher recursive. Signed-off-by: ifuryst <ifuryst@gmail.com> * fix check-fmt fail. Signed-off-by: ifuryst <ifuryst@gmail.com> --------- Signed-off-by: ifuryst <ifuryst@gmail.com>
Close #20547, this problem is that the watcher doesn't watch dir recursively.
This easily changes the watcher to watch dir recursively. While this may not be the best solution, considering that
config::watcher::spawn_threadis currently only used for watching config. IMHO, it seems intuitive to watch paths recursively.Please let me know if you have any concerns about potential implications for users to upgrade the version.