Skip to content

chore(datadog): add support for writing delimited payloads to RequestBuilder<E, O>#660

Merged
tobz merged 2 commits intomainfrom
tobz/delimited-payloads-dd-request-builder
May 9, 2025
Merged

chore(datadog): add support for writing delimited payloads to RequestBuilder<E, O>#660
tobz merged 2 commits intomainfrom
tobz/delimited-payloads-dd-request-builder

Conversation

@tobz
Copy link
Member

@tobz tobz commented May 5, 2025

Summary

This PR adds support for delimited payloads to RequestBuilder<E, O>.

In the case of the Datadog Metrics destination, individual inputs are self-delimiting by virtue of being elements in a repeated message field in the Protocol Buffers payload. This means that while the request itself is a single message with N inputs bundled into it, and not N messages, we still don't have to do anything special to wrap those inputs or separate them: just write them individually, concatenated, and the payload is valid.

However, for other destinations, like Datadog Events/Service Checks, the payloads are JSON arrays. This means that the payload has a prefix and suffix ([ and ]) and each input needs to be separated by a comma. Naturally, you get this when encoding an array of values all at once, but we want to build our requests incrementally, input by input.

This PR adds support for this model by allowing EndpointEncoder to specify a prefix, suffix, and input separator (referred to collectively as "delimiters") that must be used when building a payload. RequestBuilder<E, O> uses these to generate a valid payload, whether we've written a single input or 10,000 inputs.

While these changes are simple, most of this PR revolves around ensuring that our size limiting logic and request splitting logic works correctly in delimited mode as well as non-delimited mode. We've done a decent amount of refactoring to try and share more of the encode/flush logic between the regular encode/flush functions, and their usage in the request splitting codepath.

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

How did you test this PR?

Unit tests and correctness test.

References

AGTMETRICS-184

@tobz tobz added the type/chore Updates to dependencies or general "administrative" tasks necessary to maintain the codebase/repo. label May 5, 2025
@pr-commenter
Copy link

pr-commenter bot commented May 5, 2025

Regression Detector (DogStatsD)

Regression Detector Results

Run ID: 94c8407a-b4a0-4cbb-83cc-637b82ef1ae5

Baseline: 7.65.0-rc.9
Comparison: 7.65.0-rc.9

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
quality_gates_idle_rss memory utilization +0.07 [-0.06, +0.21] 1
dsd_uds_100mb_3k_contexts ingress throughput +0.00 [-0.10, +0.11] 1
dsd_uds_1mb_3k_contexts_dualship ingress throughput +0.00 [-0.00, +0.00] 1
dsd_uds_1mb_50k_contexts_memlimit ingress throughput +0.00 [-0.00, +0.00] 1
dsd_uds_40mb_12k_contexts_40_senders ingress throughput +0.00 [-0.00, +0.00] 1
dsd_uds_1mb_50k_contexts ingress throughput -0.00 [-0.00, +0.00] 1
dsd_uds_512kb_3k_contexts ingress throughput -0.00 [-0.01, +0.01] 1
dsd_uds_1mb_3k_contexts ingress throughput -0.00 [-0.00, +0.00] 1
dsd_uds_100mb_250k_contexts ingress throughput -0.00 [-0.13, +0.12] 1
dsd_uds_10mb_3k_contexts ingress throughput -0.01 [-0.04, +0.02] 1
dsd_uds_500mb_3k_contexts ingress throughput -0.23 [-0.32, -0.14] 1
dsd_uds_100mb_3k_contexts_distributions_only memory utilization -0.66 [-0.87, -0.45] 1

Bounds Checks: ❌ Failed

perf experiment bounds_check_name replicates_passed links
quality_gates_idle_rss memory_usage 0/10

Explanation

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

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

  • ✅ = significantly better comparison variant performance
  • ❌ = significantly worse comparison variant performance
  • ➖ = no significant change in performance

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
Copy link

pr-commenter bot commented May 5, 2025

Regression Detector (Checks Agent Go)

Regression Detector Results

Run ID: 12edfb03-da10-4bb6-ab6a-25106333c609

Baseline: f61d1f4e054b884cb1894254ab2714b84b4684cb
Comparison: f61d1f4e054b884cb1894254ab2714b84b4684cb
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
quality_gates_idle_rss memory utilization -0.59 [-0.65, -0.53] 1

Explanation

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

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

  • ✅ = significantly better comparison variant performance
  • ❌ = significantly worse comparison variant performance
  • ➖ = no significant change in performance

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
Copy link

pr-commenter bot commented May 5, 2025

Regression Detector (Checks Agent)

Regression Detector Results

Run ID: cd270469-db0b-4db6-b780-ec59d38232a8

Baseline: 7ee652b
Comparison: e609a13
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
quality_gates_idle_rss memory utilization +0.28 [+0.27, +0.30] 1

Bounds Checks: ✅ Passed

perf experiment bounds_check_name replicates_passed links
quality_gates_idle_rss memory_usage 10/10

Explanation

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

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

  • ✅ = significantly better comparison variant performance
  • ❌ = significantly worse comparison variant performance
  • ➖ = no significant change in performance

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".

@tobz tobz changed the title chore(datadog): add support for writing delimited payloads to RequestBuilder chore(datadog): add support for writing delimited payloads to RequestBuilder<E, O> May 5, 2025
@pr-commenter
Copy link

pr-commenter bot commented May 5, 2025

Regression Detector (Saluki)

Regression Detector Results

Run ID: 85577f83-55d3-475a-aa4a-2fb627d7c658

Baseline: 7ee652b
Comparison: e609a13
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
quality_gates_idle_rss memory utilization +1.03 [+0.94, +1.11] 1
dsd_uds_100mb_3k_contexts_distributions_only memory utilization +0.43 [+0.27, +0.59] 1
dsd_uds_50mb_10k_contexts_no_inlining_no_allocs ingress throughput +0.02 [-0.08, +0.12] 1
dsd_uds_100mb_3k_contexts ingress throughput +0.02 [-0.05, +0.08] 1
dsd_uds_100mb_250k_contexts ingress throughput +0.01 [-0.04, +0.07] 1
dsd_uds_50mb_10k_contexts_no_inlining ingress throughput +0.01 [-0.09, +0.10] 1
dsd_uds_40mb_12k_contexts_40_senders ingress throughput +0.00 [-0.03, +0.04] 1
dsd_uds_1mb_3k_contexts ingress throughput +0.00 [-0.00, +0.00] 1
dsd_uds_1mb_3k_contexts_dualship ingress throughput -0.00 [-0.00, +0.00] 1
dsd_uds_512kb_3k_contexts ingress throughput -0.00 [-0.01, +0.01] 1
dsd_uds_1mb_50k_contexts ingress throughput -0.00 [-0.02, +0.02] 1
dsd_uds_10mb_3k_contexts ingress throughput -0.03 [-0.06, +0.01] 1
dsd_uds_500mb_3k_contexts ingress throughput -1.16 [-1.27, -1.04] 1
dsd_uds_1mb_50k_contexts_memlimit ingress throughput -3.01 [-4.73, -1.28] 1

Bounds Checks: ✅ Passed

perf experiment bounds_check_name replicates_passed links
quality_gates_idle_rss memory_usage 10/10

Explanation

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

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

  • ✅ = significantly better comparison variant performance
  • ❌ = significantly worse comparison variant performance
  • ➖ = no significant change in performance

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
Copy link

pr-commenter bot commented May 5, 2025

Regression Detector Links

ADP 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_3k_contexts_dualship [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_40mb_12k_contexts_40_senders [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]
quality_gates_idle_rss [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]

Checks Agent Experiment Result Links

experiment link(s)
quality_gates_idle_rss [Profiling] [SMP Dashboard]

@tobz tobz marked this pull request as ready for review May 6, 2025 18:10
@tobz tobz requested a review from a team as a code owner May 6, 2025 18:10
@tobz tobz force-pushed the tobz/refactor-dd-metrics-to-generic-request-builder branch from e100ac1 to 0dd1134 Compare May 8, 2025 20:23
@tobz tobz force-pushed the tobz/delimited-payloads-dd-request-builder branch from 4b8ecd2 to 65f8fc1 Compare May 8, 2025 20:23
@tobz tobz force-pushed the tobz/refactor-dd-metrics-to-generic-request-builder branch from 0dd1134 to 66bdf4b Compare May 9, 2025 03:24
@tobz tobz force-pushed the tobz/delimited-payloads-dd-request-builder branch from 65f8fc1 to e609a13 Compare May 9, 2025 03:24
Copy link
Member Author

tobz commented May 9, 2025

Merge activity

  • May 9, 8:32 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • May 9, 8:37 AM EDT: Graphite rebased this pull request as part of a merge.
  • May 9, 8:40 AM EDT: @tobz merged this pull request with Graphite.

@tobz tobz changed the base branch from tobz/refactor-dd-metrics-to-generic-request-builder to graphite-base/660 May 9, 2025 12:33
@tobz tobz changed the base branch from graphite-base/660 to main May 9, 2025 12:36
@tobz tobz force-pushed the tobz/delimited-payloads-dd-request-builder branch from e609a13 to 397c069 Compare May 9, 2025 12:36
@tobz tobz merged commit ad9cec3 into main May 9, 2025
18 of 34 checks passed
github-actions bot pushed a commit that referenced this pull request May 9, 2025
…tBuilder<E, O>` (#660)

## Summary

This PR adds support for delimited payloads to `RequestBuilder<E, O>`.

In the case of the Datadog Metrics destination, individual inputs are self-delimiting by virtue of being elements in a repeated message field in the Protocol Buffers payload. This means that while the request itself is a single message with N inputs bundled into it, and not N messages, we still don't have to do anything special to wrap those inputs or separate them: just write them individually, concatenated, and the payload is valid.

However, for other destinations, like Datadog Events/Service Checks, the payloads are JSON arrays. This means that the payload has a prefix and suffix (`[` and `]`) and each input needs to be separated by a comma. Naturally, you get this when encoding an array of values all at once, but we want to build our requests incrementally, input by input.

This PR adds support for this model by allowing `EndpointEncoder` to specify a prefix, suffix, and input separator (referred to collectively as "delimiters") that must be used when building a payload. `RequestBuilder<E, O>` uses these to generate a valid payload, whether we've written a single input or 10,000 inputs.

While these changes are simple, most of this PR revolves around ensuring that our size limiting logic and request splitting logic works correctly in delimited mode as well as non-delimited mode. We've done a decent amount of refactoring to try and share more of the encode/flush logic between the regular encode/flush functions, and their usage in the request splitting codepath.

## Change Type

- [ ] Bug fix
- [ ] New feature
- [x] Non-functional (chore, refactoring, docs)
- [ ] Performance

## How did you test this PR?

Unit tests and correctness test.

## References

AGTMETRICS-184 ad9cec3
@tobz tobz deleted the tobz/delimited-payloads-dd-request-builder branch September 24, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/components Sources, transforms, and destinations. area/io General I/O and networking. type/chore Updates to dependencies or general "administrative" tasks necessary to maintain the codebase/repo.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants