[contrib] add orchestrion integration configuration #3074

RomainMuller · 2025-01-09T14:42:04Z

What does this PR do?

Moves orchestrion integrations configuration from orchestrion itself to dd-trace-go.

Motivation

This improves maintainability of automatic injection configuration, and allows orchestrion customers more fine-grained control of what gets injected automatically.

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
There is a benchmark for any new code, or changes to existing code.
If this interacts with the agent in a new way, a system test has been added.
Add an appropriate team label so this PR gets put in the right place for the release notes.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.
For internal contributors, a matching PR should be created to the v2-dev branch and reviewed by @DataDog/apm-go.

Unsure? Have a question? Request a review!

.github/workflows/orchestrion.yml

internal/orchestrion/_integration/go.mod

.github/workflows/orchestrion.yml

datadog-datadog-prod-us1 · 2025-01-09T14:49:42Z

Datadog Report

Branch report: romain.marcadier/APPSEC-55160/orchestrion
Commit report: 9fcb60b
Test service: dd-trace-go

✅ 0 Failed, 5201 Passed, 72 Skipped, 2m 47.81s Total Time

datadog-datadog-prod-us1 · 2025-01-09T15:49:15Z

.github/workflows/orchestrion.yml

+
+      # ddapm-test-agent is used to observe side effects from the tracer during integration tests.
+      - name: Set up Python
+        uses: actions/setup-python@v5


🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

pr-commenter · 2025-01-09T16:30:07Z

Benchmarks

Benchmark execution time: 2025-01-20 15:18:12

Comparing candidate commit 6e09796 in PR branch romain.marcadier/APPSEC-55160/orchestrion with baseline commit 2e3193f in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 1 unstable metrics.

internal/orchestrion/_integration/internal/agent/logger.go

internal/orchestrion/_integration/net_http/base.go

… repo)

…t internals

rarguelloF

LGTM overall! left some comments / questions 🙇

contrib/os/os.go

rarguelloF · 2025-01-17T15:00:51Z

contrib/sirupsen/logrus/example_test.go

@@ -29,6 +29,6 @@ func ExampleHook() {
 	cLog := logrus.WithContext(sctx).WithTime(time.Date(2000, 1, 1, 1, 1, 1, 0, time.UTC))
 	// Log as desired using the context-aware logger
 	cLog.Info("Completed some work!")
-	// Output:
+	// You should see:


I'm curious the motivation of the change, was the testable example failing? I'm wondering if we actually run these in CI 🤔

They run as part of go test normal behaviour. It didn't work because the hook is not adding the fields for trace ID and span ID.

I checked the hook's code and it wasn't changed. I'm not sure why it triggered here, but it did.

internal/orchestrion/generator/main.go

rarguelloF · 2025-01-17T15:10:45Z

contrib/net/http/client/client.go

+// short-hand request functions [http.Get], [http.Head], [http.Post], and
+// [http.PostForm]. Using these functions allows for better control over the
+// trace context propagation.
+package client


could this package be made internal? it doesn't seem to make a lot of sense outside of Orchestrion right?

it's intended to allow for users to be able to manually do what orchestrion automatically does, and provides context-aware versions of short-hand functions in the net/http package which otherwise allow no context.

This package actually has very little use to orchestrion itself (as it enables GLS-based context propagation, making the context.Context passing possibly un-necessary).

what I mean is this package doesn't provide any tracing specific funcionality, these are just convenience helpers over http.NewRequestWithContext + http.DefaultClient.Do that IMO shouldn't be part of the exposed api of dd-trace-go.

contrib/net/http/option.go

internal/orchestrion/_.orchestrion.yml

rarguelloF · 2025-01-17T15:19:37Z

internal/orchestrion/_integration/README.md

@@ -0,0 +1,65 @@
+## Orchestrion Integration Tests


why does the folder name start with _

The go toolchain (alledgedly) ignores folders starting with _ so it makes it clear that this is a module delineation that isn't meant for users to directly use.

…EC-55160/orchestrion

datadog-datadog-prod-us1 bot reviewed Jan 9, 2025

View reviewed changes

.github/workflows/orchestrion.yml Outdated Show resolved Hide resolved

.github/workflows/orchestrion.yml Outdated Show resolved Hide resolved

internal/orchestrion/_integration/go.mod Outdated Show resolved Hide resolved

internal/orchestrion/_integration/go.mod Outdated Show resolved Hide resolved

datadog-datadog-prod-us1 bot reviewed Jan 9, 2025

View reviewed changes

.github/workflows/orchestrion.yml Outdated Show resolved Hide resolved

.github/workflows/orchestrion.yml Outdated Show resolved Hide resolved

[contrib] add orchestrion integration configuration

4631a52

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch from 60deffb to ed1218e Compare January 9, 2025 14:43

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch 6 times, most recently from f06d0d3 to 8591beb Compare January 9, 2025 15:28

WIP: Set up orchestrion integration testing

6d487dd

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch from 8591beb to 6d487dd Compare January 9, 2025 15:48

datadog-datadog-prod-us1 bot reviewed Jan 9, 2025

View reviewed changes

RomainMuller added 6 commits January 9, 2025 17:56

Add CI viz integration test

4ac23cc

Add GLS integration tests

6a90e73

Add harness + gqlgen integration test

ee47a13

Add AWS.v1 tests

d60d93e

Add AWS.v2 tests

1295f26

Add the rest of integration tests

41e9732

github-advanced-security bot found potential problems Jan 10, 2025

View reviewed changes

internal/orchestrion/_integration/internal/agent/logger.go Dismissed Show dismissed Hide dismissed

internal/orchestrion/_integration/net_http/base.go Dismissed Show dismissed Hide dismissed

RomainMuller added 5 commits January 10, 2025 13:41

Fixup path to requirements-dev.txt

882c4c7

Add workflow_call trigger to orchestrion workflow

31fac3e

Add option to collect coverage

839cfba

Make workflow work when called via workflow_call (event is in foreign…

435e6ea

… repo)

Quotes around the not

03f1102

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch 3 times, most recently from 9386875 to d0e5e17 Compare January 13, 2025 09:56

darccio added 2 commits January 17, 2025 12:18

fix(contrib/sirupsen/logrus): remove output assertion

08cb209

fix(ddtrace/tracer): update TestSpanFromContext to new SpanFromContex…

965bd82

…t internals

darccio approved these changes Jan 17, 2025

View reviewed changes

rarguelloF reviewed Jan 17, 2025

View reviewed changes

RomainMuller added 5 commits January 20, 2025 11:32

Add os.OpenFile function to contrib/os

db0b477

Add explanation of what the internal/orchestrion/generator does

8128730

s/cfg/internalconfig/

4a44e8a

rename _.orchestrion.yml to gls.orchestrion.yml

c430ae9

Use non-endlessly-respawing orchestrion release

afde3ea

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch from 888d119 to afde3ea Compare January 20, 2025 13:43

RomainMuller added 10 commits January 20, 2025 14:48

Explicitly go mod download

ada7401

Try to troubleshoot integration tests failure

c5fc0a1

Force CGO_ENABLED

cca226f

Print out complete go env

abbb144

Remove explicit CGO_ENABLED setting

b2c8d4c

use go mod tidy instead of go mod download

9507c38

Also run linter in integration package directory

beb36c6

Merge remote-tracking branch 'origin/main' into romain.marcadier/APPS…

508b9e5

…EC-55160/orchestrion

Okay got it now :)

2a8b0d4

Add matrix join point

6e09796

RomainMuller force-pushed the romain.marcadier/APPSEC-55160/orchestrion branch from d770999 to 6e09796 Compare January 20, 2025 14:51

RomainMuller enabled auto-merge (squash) January 20, 2025 16:24

rarguelloF requested review from darccio and rarguelloF January 20, 2025 16:26

rarguelloF approved these changes Jan 20, 2025

View reviewed changes

Merge branch 'main' into romain.marcadier/APPSEC-55160/orchestrion

0dd7466

darccio approved these changes Jan 20, 2025

View reviewed changes

RomainMuller merged commit c9ff7bb into main Jan 20, 2025
193 of 194 checks passed

RomainMuller deleted the romain.marcadier/APPSEC-55160/orchestrion branch January 20, 2025 16:33

darccio added a commit that referenced this pull request Feb 7, 2025

fix(contrib/net/http): apply #3074 refactor

ba1fe43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[contrib] add orchestrion integration configuration #3074

[contrib] add orchestrion integration configuration #3074

RomainMuller commented Jan 9, 2025

datadog-datadog-prod-us1 bot commented Jan 9, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot Jan 9, 2025

pr-commenter bot commented Jan 9, 2025 •

edited

Loading

rarguelloF left a comment

rarguelloF Jan 17, 2025

darccio Jan 17, 2025

rarguelloF Jan 17, 2025

RomainMuller Jan 20, 2025

rarguelloF Jan 20, 2025

rarguelloF Jan 17, 2025

RomainMuller Jan 20, 2025

[contrib] add orchestrion integration configuration #3074

[contrib] add orchestrion integration configuration #3074

Conversation

RomainMuller commented Jan 9, 2025

What does this PR do?

Motivation

Reviewer's Checklist

datadog-datadog-prod-us1 bot commented Jan 9, 2025 • edited Loading

Datadog Report

datadog-datadog-prod-us1 bot Jan 9, 2025

Choose a reason for hiding this comment

🟠 Code Vulnerability

pr-commenter bot commented Jan 9, 2025 • edited Loading

Benchmarks

rarguelloF left a comment

Choose a reason for hiding this comment

rarguelloF Jan 17, 2025

Choose a reason for hiding this comment

darccio Jan 17, 2025

Choose a reason for hiding this comment

rarguelloF Jan 17, 2025

Choose a reason for hiding this comment

RomainMuller Jan 20, 2025

Choose a reason for hiding this comment

rarguelloF Jan 20, 2025

Choose a reason for hiding this comment

rarguelloF Jan 17, 2025

Choose a reason for hiding this comment

RomainMuller Jan 20, 2025

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot commented Jan 9, 2025 •

edited

Loading

pr-commenter bot commented Jan 9, 2025 •

edited

Loading