skill(apm-integrations): Message Queues guidance - #12293
jordan-wong wants to merge 6 commits into
Conversation
…sume operations When a client API returns a batch of items from one call (e.g. a message broker's poll returning N records), spanning the batch call itself prevents attaching per-item follow-on work to the item that triggered it. Document the wrap-the-iterable pattern already used by kafka-clients (TracingIterable/ TracingIterator/TracingList/TracingListIterator) as the standard for any future batch-consume instrumentation, not just messaging libraries.
🟢 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. |
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50ff311c27
ℹ️ 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".
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The guidance can remove required client-operation spans from search and bulk integrations. It also gives incorrect timeout behavior for abandoned root iterations in the default legacy context manager.
🤖 Datadog Autotest · Commit 50ff311 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
jordan-wong
changed the title
…er covers a callback overload too If a single advice's method matcher covers both a synchronous overload and a sibling overload that accepts a completion callback, unconditionally finishing the span in the exit advice truncates the callback-based call's span duration and drops any error the callback would report — the span finishes when the submitting call returns, not when the operation actually completes. Document the fix: branch on whether the callback argument is present, and finish the span from the wrapped callback instead of the exit advice for that call.
This comment has been minimized.
This comment has been minimized.
…idance Addresses 4 review findings on the "one span per item" rule: - Scope it to independent-consume domains (message/job queues) only. Search and bulk client operations return many elements from ONE outbound call and must stay a single span (see Elasticsearch7RestClientInstrumentation) -- applying the per-item pattern there would misattribute application work and produce thousands of spans for one request. - Add context extraction to the example: each item's span must be started from that item's own propagated context (e.g. its headers), not the caller's active context -- items in a batch can come from different producers. The prior example omitted this entirely. - Correct the abandoned-iteration claim: under the default legacy context manager, activateNext's root iteration scopes ARE force-finished by a background cleaner after trace.scope.iteration.keep.alive (default 30s) -- the prior text said no such timeout exists. - Note that a List-implementing wrapper (mirroring TracingList) does not preserve equals/hashCode -- callers relying on List equality need a different wrapper shape or explicit overrides.
jordan-wong
added
inst: kafka
…s doc-only change)
…age (robolectric-4.17/openai-java 4.59-4.69), now resolved on master
jordan-wong
changed the title
This branch has not been deployed
2 participants
Changes
Adds two rules to
.agents/skills/apm-integrations/references/advice-class.md:Iterable/Iterator/Listso each item gets its own span, not the batch call itself.send(Message)/send(Message, CompletionListener)), it must finish the span from the callback when present, not unconditionally at exit — otherwise the async variant's span is truncated and its errors are lost.Source
From a blind-regen research cycle on JMS (#11170) and kafka-clients (#12361): the batch pattern is already correct in
kafka-clients-0.11but undocumented; the callback bug was observed live in the JMS regen.Toolkit research context
apm-instrumentation-toolkit Java eval-research: blind-regenerates existing instrumentation to find toolkit-guidance gaps, encodes fixes here. Docs-only.