feat(otelc): add support for otelc compile-time instrumentation - #5117
feat(otelc): add support for otelc compile-time instrumentation#5117rarguelloF wants to merge 27 commits into
Conversation
Config Audit |
|
BenchmarksBenchmark execution time: 2026-08-11 13:57:20 Comparing candidate commit d072f53 in PR branch Found 0 performance improvements and 4 performance regressions! Performance is the same for 322 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
kakkoyun
left a comment
kakkoyun
left a comment
There was a problem hiding this comment.
Looking good. Even though there are a lot of line changes, semantically not much has changed.
Only one thing to comment. And we should also understand why we have this performance regression.
| // plain `go test` and under `otelc go test` (where the flag is already true). | ||
| // That the flag actually gets flipped is proven by | ||
| // internal/orchestrion/_integration/otelc, which only passes in an otelc build. | ||
| func TestEnabled(t *testing.T) { |
There was a problem hiding this comment.
It would make more sense to use otelc here and printout the value of the the enabled in the output binary. Otherwise this test proves nothing.
There was a problem hiding this comment.
Good catch. Yes this test is not bringing much, so I'm removing it (the test you mention already exists in internal/orchestrion/_integration/otelc/proof_test.go)
kakkoyun
left a comment
kakkoyun
left a comment
There was a problem hiding this comment.
Looking good. Even though there are a lot of line changes, semantically not much has changed.
Only one thing to comment. And we should also understand why we have this performance regression.
…t binary on Windows
6fd87f6 to
73560ba
Compare
![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: 73560ba83d
ℹ️ 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".
| @@ -0,0 +1,250 @@ | |||
| name: OTelc | |||
There was a problem hiding this comment.
This adds a new GitHub Actions workflow without updating CONTRIBUTING.md; the repository instructions explicitly classify a new GitHub or GitLab workflow as a significant feature that must be documented there. Add the OTelc lane, its purpose, and how to invoke/configure it.
AGENTS.md reference: AGENTS.md:L21-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not doing the docs in this PR. Worth noting the CI Workflows section in CONTRIBUTING.md does not document the orchestrion lane either, so documenting otelc on its own would be inconsistent. Both should be covered together in a follow-up.
| // Package otelc reports whether the current binary was instrumented at compile | ||
| // time by otelc, the OpenTelemetry Go compile-time instrumentation tool | ||
| // (https://github.com/open-telemetry/opentelemetry-go-compile-instrumentation). |
There was a problem hiding this comment.
The commit introduces internal/otelc but leaves internal/README.md unchanged, so the package is absent from the repository's internal-package catalog. Document its name and its role as the runtime build-mode signal for OTelc instrumentation.
AGENTS.md reference: internal/AGENTS.md:L3-L6
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not doing the docs in this PR. Will be covered in the same follow-up as the CONTRIBUTING.md entry.
hannahkm
left a comment
hannahkm
left a comment
There was a problem hiding this comment.
One quick nit. Also, should we update the profiler to also start with the tracer? Or do you prefer to save that for a future PR?
| // writes literally into this package. See the equivalent test in ddtrace/tracer | ||
| // for the full rationale. | ||
| // | ||
| // Keep in sync with instrumentation/appsec/dyngo/gls.otelc.yaml. All three targets |
There was a problem hiding this comment.
Why do we need to have two copies of the same code file? Can we consolidate this into one place?
3 participants
What does this PR do?
Makes it possible to build an application with otelc and get Datadog tracing, the way orchestrion does today. No integration is migrated yet, this is only the groundwork.
In an otelc build the tracer now starts and stops on its own, and the goroutine-local storage bridge is woven in so a span still finds its parent when no
context.Contextwas passed down. The same applies to AppSec operations.Applications turn it on by blank-importing
github.com/DataDog/dd-trace-go/otelc/all/v2from anotel.instrumentation.go, the same shape asorchestrion/all. Integrations get added to that package as they are migrated.The GLS test suites under
internal/orchestrion/_integration/glsandgls-leaknow run under either tool and pass identically, with nothing skipped and no assertion relaxed.Two known gaps, both written down in the rule files:
func init(), before any other package init runs. Here it starts at the top ofmain, so a span emitted from a package init is lost. Doing it the same way needs otelc'sadd_file, which currently also applies to test binaries and emits an invalid package clause there. Two upstream issues to file.net/httpis migrated.Motivation
Support otelc.
Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.otelc/allmodule.