Rewrite readiness-probe comment to be bug-agnostic#64
Open
pawelchcki wants to merge 1 commit intopc/test-hang-fixfrom
Open
Rewrite readiness-probe comment to be bug-agnostic#64pawelchcki wants to merge 1 commit intopc/test-hang-fixfrom
pawelchcki wants to merge 1 commit intopc/test-hang-fixfrom
Conversation
This was referenced Apr 21, 2026
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
pawelchcki
force-pushed
the
graphite-base/64
branch
from
a38dc0a to
99e8f12
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
f8b5e63 to
79ee32d
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
79ee32d to
830ce2b
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
2 times, most recently
from
6a34305 to
9f8d21d
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
2 times, most recently
from
0791740 to
546027b
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
9f8d21d to
2d7e81a
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
546027b to
cf1d98d
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
2d7e81a to
a3cf24d
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
cf1d98d to
74e3eec
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
a3cf24d to
6db78f7
Compare
pawelchcki
requested review from
cataphract and
Copilot
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
Updates the integration test harness documentation to explain why readiness is probed via TCP (instead of HTTP) in a way that remains accurate even as mod_datadog internals evolve.
Changes:
- Rewrites the readiness-probe comment to be bug-agnostic and focused on handler/trace side effects.
- Clarifies that HTTP probes may generate traces in the per-test agent session, while raw TCP connect does not.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Probe readiness with a bare TCP connect rather than an HTTP GET: | ||
| # HTTP probes run through mod_datadog's handlers and can emit a | ||
| # trace into the per-test agent session, contaminating assertions | ||
| # tests make about per-request trace counts. A raw TCP connect |
There was a problem hiding this comment.
Minor grammar: consider adding “that” to clarify the sentence (“…can emit a trace into the per-test agent session, contaminating assertions that tests make about per-request trace counts.”).
Suggested change
| # tests make about per-request trace counts. A raw TCP connect | |
| # that tests make about per-request trace counts. A raw TCP connect |
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
74e3eec to
c03a185
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
6db78f7 to
b20bf17
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: 33d9e1c | Docs | Datadog PR Page | Give us feedback! |
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
b20bf17 to
bd7f943
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
c03a185 to
100093b
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
bd7f943 to
ad645a1
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
100093b to
886f1c6
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
ad645a1 to
5ea1662
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
886f1c6 to
27ea853
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
5ea1662 to
b636ade
Compare
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
27ea853 to
e4163a4
Compare
The previous comment explained the TCP-over-HTTP choice by citing a specific bug in the DatadogTracing Off scope-merge path. That bug is now fixed, so the comment reads as if a live bug is still in the tree. Restate the rationale in terms that will still be true after further mod_datadog changes: any HTTP probe runs through the module's handlers and can emit a trace into the per-test agent session; a bare TCP connect can't.
pawelchcki
force-pushed
the
pc/probe-comment-cleanup
branch
from
e4163a4 to
33d9e1c
Compare
pawelchcki
force-pushed
the
pc/test-hang-fix
branch
from
b636ade to
c5d42da
Compare
xlamorlette-datadog
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The readiness-probe comment cited a specific bug in the
DatadogTracing Offscope-merge path to justify using TCP over HTTP. That bug is fixed earlier in this stack, so the comment now reads as if a live bug is still in the tree.Restate the rationale bug-agnostically: any HTTP probe runs through the module's handlers and can emit a trace into the per-test agent session; a bare TCP connect can't. This stays true regardless of future mod_datadog changes.