Uh oh!
There was an error while loading. Please reload this page.
Migrate Spring Boot RabbitMQ smoke test to JUnit - #12128
Conversation
There was a problem hiding this comment.
The migrated test changes RabbitMQ's known teardown-error allowlist from the actual client text occured to occurred. When either app is stopped, that disconnect log is treated as an unexpected error and the smoke test fails in teardown; restore the original spelling.
🤖 Datadog Autotest · Commit 1201890 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:1201890609
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
| return trace( | ||
| SORT_BY_ANCESTRY, | ||
| sp("spring-rabbit-0", "servlet.request", "GET /roundtrip/{message}").root(), |
There was a problem hiding this comment.
Match received chunks instead of one cross-JVM trace
When a round trip crosses the sender and receiver JVMs, each tracer flushes its own trace chunk to the backend; TestAgentBackend.fetchTraces() only decodes /test/session/traces and does not coalesce chunks sharing a trace ID. Consequently, no received DecodedTrace contains all 12 spans required here, so this assertion times out even when propagation is correct. Match the per-JVM chunks separately and compare their trace/parent IDs, or explicitly merge chunks by trace ID before applying this matcher.
Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
1201890 to
979b3f4Compare
This comment has been minimized.
This comment has been minimized.
979b3f4 to
353ea51Compare3b1d4ed to
bb468daComparebb468da to
2cb3514Compare2cb3514 to
3053afcCompare3053afc to
4d018c9Compare| void roundTripsProduceFullAmqpTraceStructure() throws IOException { | ||
| // Drive 3 round-trips through the sender; each travels | ||
| // sender -> otherqueue -> receiver -> queue -> sender. | ||
| String[] MESSAGES = {"foo", "bar", "baz"}; |
There was a problem hiding this comment.
small nit for MESSAGES to be lowercase since it's a local variable, but not a big deal
| return trace(sp(service, "amqp.command", command).root()); | ||
| } | ||
| private static SpanMatcher sp(String service, String operation, String resource) { |
There was a problem hiding this comment.
nit: maybe spm would be a bit better name?
There was a problem hiding this comment.
Or simply a method local span method (eventually using a qualified SpanMatcher.span())
| return trace(sp(service, "amqp.command", command).root()); | ||
| } | ||
| private static SpanMatcher sp(String service, String operation, String resource) { |
There was a problem hiding this comment.
Or simply a method local span method (eventually using a qualified SpanMatcher.span())
| class SpringBootRabbitSmokeTest { | ||
| private static final int TIMEOUT_SECONDS = 60; | ||
| private static final int RABBIT_AMQP_PORT = 5672; | ||
| private static final OkHttpClient CLIENT = new OkHttpClient(); |
There was a problem hiding this comment.
question: Is there a reason to make this static rather than an instance variable ?
4d018c9 to
2f155a2Compare
What Does This Do
This PR migrates the Spring Boot RabbitMQ smoke tests to the new JUnit smoke test framework.
Motivation
The new tests checks the whole trace structures and span values, in addition to trace payload validity and invariants.
It also check that three round-trips are present (no only one) and the trace is fully-connected.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: APMLP-1247