DataDog / saluki

An experimental toolkit for building telemetry data planes in Rust.
Apache License 2.0
12 stars 2 forks source link

[APR-205] Start to flatten out framing and codecs #195

Closed lukesteensen closed 3 weeks ago

lukesteensen commented 1 month ago

The goal here is just to start reducing some complexity so that subsequent changes are easier to make and simpler to review. Where previously we had a Deserializer wrapping a framer wrapping a codec, we now just have a framer and a codec that are used side-by-side. By avoiding the nesting, we're making it easier to introduce a semi-parsed result struct from the codec without needing to thread reference lifetimes through multiple layers of machinery. Actually implementing that change to the codec's parsing function will be the next step, and from there we will be able to rework the rest of the logic (enrichment, differentiating between message types, forwarding, etc) to be a more straightforward function from that semi-parsed representation to a full event, with fewer intermediate steps and less mutation, as well as our ultimate goal, to easily pass in a string/context interner specific to that socket handler.

There's likely to be a few bits that were dropped that need to be added back (as well as some things that ought to be simplified further given the new state), so don't hesitate to point anything out in review 😅

pr-commenter[bot] commented 1 month ago

Regression Detector (DogStatsD)

Regression Detector Results

Run ID: c8b5a889-66f8-407b-9537-edc6da91f058

Baseline: 7.52.0 Comparison: 7.52.1

Performance changes are noted in the perf column of each table:

No significant changes in experiment optimization goals

Confidence level: 90.00% Effect size tolerance: |Δ mean %| ≥ 5.00%

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

| perf | experiment | goal | Δ mean % | Δ mean % CI | links | |------|----------------------------------------------|--------------------|----------|----------------|-------| | ➖ | dsd_uds_100mb_3k_contexts_distributions_only | memory utilization | +2.65 | [+2.45, +2.86] | | | ➖ | dsd_uds_512kb_3k_contexts | ingress throughput | +0.03 | [-0.05, +0.11] | | | ➖ | dsd_uds_1mb_3k_contexts | ingress throughput | +0.02 | [-0.04, +0.08] | | | ➖ | dsd_uds_1mb_50k_contexts | ingress throughput | +0.02 | [+0.00, +0.03] | | | ➖ | dsd_uds_500mb_3k_contexts | ingress throughput | +0.00 | [+0.00, +0.01] | | | ➖ | dsd_uds_100mb_3k_contexts | ingress throughput | -0.00 | [-0.01, +0.01] | | | ➖ | dsd_uds_10mb_3k_contexts | ingress throughput | -0.00 | [-0.03, +0.03] | | | ➖ | dsd_uds_100mb_250k_contexts | ingress throughput | -0.01 | [-0.06, +0.05] | | | ➖ | dsd_uds_1mb_50k_contexts_memlimit | ingress throughput | -0.06 | [-0.11, -0.02] | |

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: 1. Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look. 2. 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. 3. Its configuration does not mark it "erratic".
pr-commenter[bot] commented 1 month ago

Regression Detector (Saluki)

Regression Detector Results

Run ID: 1b461e20-eeed-4ccc-9aef-3d0a86d3123c

Baseline: d2adfc84dc22babfdc1d6dede30f68e2b292c599 Comparison: 0093928757188f566931f35d2faae2ceb1df1a25

Performance changes are noted in the perf column of each table:

No significant changes in experiment optimization goals

Confidence level: 90.00% Effect size tolerance: |Δ mean %| ≥ 5.00%

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

| perf | experiment | goal | Δ mean % | Δ mean % CI | links | |------|-------------------------------------------------|--------------------|----------|----------------|-------| | ➖ | dsd_uds_500mb_3k_contexts | ingress throughput | +0.97 | [+0.87, +1.07] | | | ➖ | dsd_uds_100mb_250k_contexts | ingress throughput | +0.05 | [-0.23, +0.34] | | | ➖ | dsd_uds_1mb_50k_contexts | ingress throughput | +0.02 | [-0.03, +0.06] | | | ➖ | dsd_uds_10mb_3k_contexts | ingress throughput | +0.00 | [-0.06, +0.07] | | | ➖ | dsd_uds_50mb_10k_contexts_no_inlining | ingress throughput | +0.00 | [-0.02, +0.02] | | | ➖ | dsd_uds_50mb_10k_contexts_no_inlining_no_allocs | ingress throughput | -0.00 | [-0.05, +0.05] | | | ➖ | dsd_uds_100mb_3k_contexts | ingress throughput | -0.00 | [-0.03, +0.02] | | | ➖ | dsd_uds_1mb_3k_contexts | ingress throughput | -0.01 | [-0.14, +0.12] | | | ➖ | dsd_uds_512kb_3k_contexts | ingress throughput | -0.03 | [-0.22, +0.16] | | | ➖ | dsd_uds_100mb_3k_contexts_distributions_only | memory utilization | -0.90 | [-1.03, -0.77] | | | ➖ | dsd_uds_1mb_50k_contexts_memlimit | ingress throughput | -1.89 | [-5.15, +1.38] | |

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: 1. Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look. 2. 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. 3. Its configuration does not mark it "erratic".
pr-commenter[bot] commented 1 month ago

Regression Detector Links

Experiment Result Links

experiment link(s)
dsd_uds_100mb_250k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_100mb_3k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_100mb_3k_contexts_distributions_only [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_10mb_3k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_1mb_3k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_1mb_50k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_1mb_50k_contexts_memlimit [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_500mb_3k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_512kb_3k_contexts [Profiling (ADP)] [Profiling (DSD)] [SMP Dashboard]
dsd_uds_50mb_10k_contexts_no_inlining (ADP only) [Profiling (ADP)] [SMP Dashboard]
dsd_uds_50mb_10k_contexts_no_inlining_no_allocs (ADP only) [Profiling (ADP)] [SMP Dashboard]
lukesteensen commented 3 weeks ago

/merge

dd-devflow[bot] commented 3 weeks ago

:steam_locomotive: MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals. Note: if you pushed new commits since the last approval, you may need additional approval. You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

dd-devflow[bot] commented 3 weeks ago

:warning: MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

lukesteensen commented 3 weeks ago

/merge

dd-devflow[bot] commented 3 weeks ago

:steam_locomotive: MergeQueue: pull request added to the queue

The median merge time in main is 0s.

Use /merge -c to cancel this operation!