docs(contrib): add apm-integrations skill and integration authoring guides - #5052
rarguelloF wants to merge 17 commits into
Conversation
Config Audit |
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 98af338 | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-09-25 13:50:27 Comparing candidate commit 98af338 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 334 metrics, 1 unstable metrics, 1 flaky benchmarks without significant changes.
|
…integration-authoring-docs
…acy naming-schema
…integration README
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ec05b9533
ℹ️ 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".
…integration-authoring-docs
kakkoyun
left a comment
There was a problem hiding this comment.
Great initiative! Thanks.
Could you add some data on effectiveness of these files? Some evaluation experiments maybe. We could use that setup for other parts as well.
wconti27
left a comment
There was a problem hiding this comment.
Could we make this pr an apm-integrations skill instead? that matches the implementations in JS, python, Java
…integration-authoring-docs # Conflicts: # AGENTS.md # contrib/AGENTS.md
|
@kakkoyun I've worked on a separate branch on some evals and an eval framework in #5217 and the results show improvement (I can share in private). Not sure if this eval framework will be eventually be merged or we will use an alternative one, but I would like to land these doc improvements in the meantime if possible 🙏 @wconti27 added a skill pointing to the documentation I added, please let me know if this works! |
@rarguelloF FYI, I also have an eval framework around integration code quality, that we can do an A / B test of performance w/ & w/o the skill |
| dd-trace-go integration (contrib) development guide. Use when creating, reviewing, or | ||
| debugging a contrib integration or its Orchestrion auto-instrumentation. |
There was a problem hiding this comment.
just curious, is there anything that agents generally struggle with around the integrations in go? EG: inn dd-trace-js, they sometimes struggle with Orchestrion-JS, so we have additional docs on that.
There was a problem hiding this comment.
you can see some examples of code reviews i did using this guide as a reference for new integrations not using these guidelines:
| 5. Set tags, service name and operation name. Cast the component tag, | ||
| `string(instrumentation.PackageX)`, or the span is attributed to `manual`. Leave `naming` | ||
| unset and hardcode operation names. Never put `tracer.WithStartSpanConfig(cachedBase)` first | ||
| in an option list, it corrupts the shared base. |
There was a problem hiding this comment.
Step 5 repeats three rules that INTEGRATIONS.md §5 also states: the string(...) cast for the component tag, the empty naming map, and the position of WithStartSpanConfig in the option list. When a maintainer corrects one of these rules in the guide, the copy in the skill can keep the old wording. contrib/AGENTS.md asks the maintainer to update the skill when a rule changes, but no check enforces this. Is the duplication deliberate? If yes, please add one sentence to INTEGRATIONS.md §5 that states the skill repeats these rules. The sentence tells a corrector that two texts exist.
There was a problem hiding this comment.
You're right, that was accidental duplication rather than a deliberate copy. Pulled all the repeated rules out of the skill, not just the three in step 5, the same issue was in steps 3, 6, 7 and 8 too. The skill is now a pure router: workflow steps with links to the section in INTEGRATIONS.md/ORCHESTRION.md that has the actual rule, nothing restated. Fixed in 0f8bed8.
| 3. Add the traced package's import path, the same value used for `TracedPackage` in step 2 (not the | ||
| contrib module path), to `contribIntegrations` in | ||
| [ddtrace/tracer/option.go](../ddtrace/tracer/option.go). This is how the tracer reports the | ||
| integration as imported, for example in startup logs and integration telemetry. |
There was a problem hiding this comment.
The previous contrib/README.md required a pull request in the DataDog/documentation repository. The pull request added each new integration to the list of compatible integrations. This guide replaces the README as the registration checklist, and no step covers that pull request. No script or CI job in this repository performs it either. A new author who follows section 7 registers the integration inside the tracer, but the public compatibility page stays stale. Please restore the step, or state where the documentation list is maintained now.
| 3. Add the traced package's import path, the same value used for `TracedPackage` in step 2 (not the | |
| contrib module path), to `contribIntegrations` in | |
| [ddtrace/tracer/option.go](../ddtrace/tracer/option.go). This is how the tracer reports the | |
| integration as imported, for example in startup logs and integration telemetry. | |
| 3. Add the traced package's import path, the same value used for `TracedPackage` in step 2 (not the | |
| contrib module path), to `contribIntegrations` in | |
| [ddtrace/tracer/option.go](../ddtrace/tracer/option.go). This is how the tracer reports the | |
| integration as imported, for example in startup logs and integration telemetry. | |
| 4. Open a pull request in [Datadog/documentation](https://github.com/DataDog/documentation) to add | |
| the integration to the list of | |
| [compatible integrations](https://github.com/DataDog/documentation/blob/master/content/en/tracing/trace_collection/compatibility/go.md). |
There was a problem hiding this comment.
Deliberately left this one out for now. The old step told authors to open a PR against Datadog/documentation as part of authoring the integration, but that races the actual release: the compatibility page shouldn't list an integration before a tagged version ships it. I'd rather fix this properly as part of the release process, automating the doc update once a version is tagged, than restore a manual step that's wrong about timing. Tracking that as a follow-up rather than doing it here.
LLM ValidationLLM Validation Gate — dd-trace-go-agent✅ PASS
AnalysisChanged instruction file(s): No safety or blocking-case regressions across 3 case(s). Overall pairwise win-rate 56% [49%–62%], quality +2.4 — see the verdict above for whether that clears the noise band. Results
Cases
Per-dimension scores, token usage, latency, and estimated cost are in the CI job logs. |
What does this PR do?
Adds an
apm-integrationsskill, plus the two guides it points at, so there is one place to learn how to build a dd-trace-go integration.The skill lives in
.agents/skills/with a.claude/skills/symlink, matching the layout dd-trace-js, dd-trace-py and dd-trace-java already use for their ownapm-integrationsskills. It carries the workflow and the rules that are easiest to get wrong, and links out for the detail:contrib/INTEGRATIONS.md, the authoring guide.contrib/ORCHESTRION.md, auto-instrumentation. Replacesorchestrion/README.mdandorchestrion/AGENTS.md.contrib/README.mdis now a short user-facing reference, andcontrib/AGENTS.mdpoints at the skill while staying readable on its own for tools that do not support skills.Motivation
Guidance on building an integration was scattered and incomplete.
contrib/README.mdcovered naming and file layout but said nothing about Orchestrion, about which interception patterns can actually be auto-instrumented, or about tag and service naming. Contributors and coding agents defaulted to patterns that are hard or impossible to auto-instrument, or copied the deprecated per-component naming machinery.Keeping the guides in
contrib/instead of inside the skill is deliberate. They are for people too.The skill also cuts how much an agent has to read.
contrib/AGENTS.mdpreviously required both guides in full, roughly 8,000 tokens, for any contrib change. The skill is roughly 1,000 and links to the section that applies.Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Unsure? Have a question? Request a review!