From 49288e7ed4f7a002c25a2de59f5a0b701a1f213e Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Wed, 20 Nov 2024 12:26:49 +0100 Subject: [PATCH] feat: rename `dd:ignore` to `orchestrion:ignore` (#405) The `//orchestrion:ignore` directive replaces `//dd:ignore` to make it clear that it opts the node tree out of any orchestrion processing, and not just of datadog integrations. The `dd:ignore` directive continues to work, but a warning will be emitted when it is encountered, prompting users to update their code. --- README.md | 30 ++++++++++++------ _integration-tests/tests/chi.v5/chi.go | 2 +- .../tests/gcp_pubsub/gcp_pubsub.go | 2 +- docs/content/docs/_index.md | 2 +- docs/content/docs/custom-trace.md | 31 ++++++++++++------- docs/content/docs/features.md | 2 +- docs/layouts/partials/navbar.html | 1 - internal/injector/injector.go | 2 +- internal/injector/labels.go | 17 ++++++++-- .../testdata/injector/http-ignored/config.yml | 2 +- 10 files changed, 61 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index fcbd4074..78b009f9 100644 --- a/README.md +++ b/README.md @@ -127,12 +127,12 @@ Library | Since | Notes `github.com/go-redis/redis/v8` | `v0.7.0` | [Aspect][go-redis-v8] `github.com/gofiber/fiber/v2` | `v0.7.0` | [Aspect][fiber-v2] `github.com/gomodule/redigo/redis` | `v0.7.0` | [Aspect][redigo] -`github.com/gorilla/mux` | `v0.7.0` | [Aspect][gorilla]. Cannot be opted out of via `//dd:ignore` +`github.com/gorilla/mux` | `v0.7.0` | [Aspect][gorilla] ([library-side][lib-side]) `github.com/jinzhu/gorm` | `v0.7.0` | [Aspect][jinzhu-gorm] `github.com/labstack/echo/v4` | `v0.7.0` | [Aspect][echo] `google.golang.org/grpc` | `v0.7.0` | [Aspect][grpc] `gorm.io/gorm` | `v0.7.0` | [Aspect][gorm] -`net/http` | `v0.7.0` | [Client][net-http.client] / [Server][net-http.server] +`net/http` | `v0.7.0` | [Client][net-http.client] ([library-side][lib-side]) / [Server][net-http.server] `go.mongodb.org/mongo-driver/mongo` | `v0.7.3` | [Aspect][mongo] `github.com/aws-sdk-go/aws` | `v0.7.4` | [Aspect][aws-sdk-go] `github.com/hashicorp/vault` | `v0.7.4` | [Aspect][hashicorp-vault] @@ -144,7 +144,7 @@ Library | Since | Notes `github.com/aws/aws-sdk-go-v2` | `v0.8.0` | [Aspect][aws-sdk-go-v2] `github.com/redis/go-redis/v9` | `v0.8.0` | [Aspect][go-redis-v9] `github.com/gocql/gocql` | `v0.8.0` | [Aspect][gocql] -`cloud.google.com/go/pubsub` | `v0.9.0` | [Aspect][pubsub] +`cloud.google.com/go/pubsub` | `v0.9.0` | [Aspect][pubsub] ([library-side][lib-side]) `github.com/99designs/gqlgen` | `v0.9.1` | [Aspect][gqlgen] `github.com/redis/go-redis` | `v0.9.1` | [Aspect][go-redis] `github.com/graph-gophers/graphql-go` | `v0.9.1` | [Aspect][graph-gophers] @@ -153,12 +153,14 @@ Library | Since | Notes `github.com/jackc/pgx` | `v0.9.4` | [Aspect][pgx] `github.com/elastic/go-elasticsearch` | `v0.9.4` | [Aspect][elasticsearch] `github.com/twitchtv/twirp` | `v0.9.4` | [Aspect][twirp] -`github.com/segmentio/kafka-go` | `v0.9.4` | [Aspect][segmentio-kafka-go] -`github.com/confluentinc/confluent-kafka-go/kafka` | `v0.9.4` | [Aspect][confluent-kafka-go-v1] -`github.com/confluentinc/confluent-kafka-go/kafka/v2` | `v0.9.4` | [Aspect][confluent-kafka-go-v2] -`github.com/julienschmidt/httprouter` | `v0.9.4` | [Aspect][httprouter] +`github.com/segmentio/kafka-go` | `v0.9.4` | [Aspect][segmentio-kafka-go] ([library-side][lib-side]) +`github.com/confluentinc/confluent-kafka-go/kafka` | `v0.9.4` | [Aspect][confluent-kafka-go-v1] ([library-side][lib-side]) +`github.com/confluentinc/confluent-kafka-go/kafka/v2` | `v0.9.4` | [Aspect][confluent-kafka-go-v2] ([library-side][lib-side]) +`github.com/julienschmidt/httprouter` | `v0.9.4` | [Aspect][httprouter] ([library-side][lib-side]) `github.com/sirupsen/logrus` | `v0.9.4` | [Aspect][logrus] +[lib-side]: #library-side + [db-sql]: https://datadoghq.dev/orchestrion/docs/built-in/stdlib/database-sql/ [gin]: https://datadoghq.dev/orchestrion/docs/built-in/http/gin/ [chi-v5]: https://datadoghq.dev/orchestrion/docs/built-in/http/chi/#use-v5-tracer-middleware @@ -202,8 +204,18 @@ Library | Since | Notes [test-optimization]: https://docs.datadoghq.com/tests/ -Calls to these libraries are instrumented with library-specific code adding tracing to them, including support for -distributed traces. +### Library Side + +Most integrations are added by orchestrion at the call site, making it possible to use the [`//orchestrion:ignore` +directive][orchestrion-ignore] to locally opt out of instrumentation for a specific instance of a component. + +[orchestrion-ignore]: https://datadoghq.dev/orchestrion/docs/custom-trace/#prevent-instrumentation-of-a-section-of-code + +Some integrations are however injected directly into the library (library-side instrumentation, also called callee-side +instrumentation), and are hence always active and cannot be locally opted out. If you have a use-case where you need to +locally opt out of a library-side instrumentation, please let us know about it by filing a [GitHub issue][new-gh-issue]. + +[new-gh-issue]: https://github.com/DataDog/orchestrion/issues/new/choose ## Troubleshooting diff --git a/_integration-tests/tests/chi.v5/chi.go b/_integration-tests/tests/chi.v5/chi.go index 5b756e87..3b931a0e 100644 --- a/_integration-tests/tests/chi.v5/chi.go +++ b/_integration-tests/tests/chi.v5/chi.go @@ -28,7 +28,7 @@ type TestCase struct { func (tc *TestCase) Setup(t *testing.T) { router := chi.NewRouter() - //dd:ignore + //orchestrion:ignore tc.Server = &http.Server{ Addr: "127.0.0.1:" + utils.GetFreePort(t), Handler: router, diff --git a/_integration-tests/tests/gcp_pubsub/gcp_pubsub.go b/_integration-tests/tests/gcp_pubsub/gcp_pubsub.go index fcebbb79..7ed7ffb2 100644 --- a/_integration-tests/tests/gcp_pubsub/gcp_pubsub.go +++ b/_integration-tests/tests/gcp_pubsub/gcp_pubsub.go @@ -55,7 +55,7 @@ func (tc *TestCase) Setup(t *testing.T) { projectID := tc.container.Settings.ProjectID - //dd:ignore + //orchestrion:ignore conn, err := grpc.NewClient(tc.container.URI, grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) diff --git a/docs/content/docs/_index.md b/docs/content/docs/_index.md index 5671d144..9fb9ba0b 100644 --- a/docs/content/docs/_index.md +++ b/docs/content/docs/_index.md @@ -28,7 +28,7 @@ they are compiled or linked. - **Flexible** – Developers can easily influence the observability data produced by their applications by adding special directives, such as - `//dd:ignore`, or `//dd:span custom-tag:value`. + `//orchestrion:ignore`, or `//dd:span custom-tag:value`. - **Configurable** – Orchestrion's code manipulations can be configured with simple YAML documents, allowing developers to provide specific diff --git a/docs/content/docs/custom-trace.md b/docs/content/docs/custom-trace.md index 01ad197f..de86c1d6 100644 --- a/docs/content/docs/custom-trace.md +++ b/docs/content/docs/custom-trace.md @@ -16,11 +16,19 @@ removes the possibility of someone forgetting to instrument a particular call. There are however cases where you may want specific sections of your application to not be instrumented, either because they result in excessively verbose -traces, or because those trace spans would be duplicated (for example, when -using a custom-made `net/http` middleware stack). +traces, or because those trace spans would be duplicated. -The `//dd:ignore` directive can be added anywhere in your application's code, -and will disable all `orchestrion` instrumentation in the annotated syntax tree. +The `//orchestrion:ignore` directive can be added anywhere in your application's +code, and will disable all `orchestrion` instrumentation in the annotated syntax +tree. + +{{}} +Library-side (also known as callee-side) instrumentation cannot be opted out of +using `//orchestrion:ignore`. Refer to the [README document][readme] to learn +about which integrations are library-side. + +[readme]: https://github.com/DataDog/orchestrion#supported-libraries +{{}} For example: @@ -29,7 +37,7 @@ package demo import "net/http" -//dd:ignore I don't want any of this to be instrumented, ever. +//orchestrion:ignore I don't want any of this to be instrumented, ever. func noInstrumentationThere() { // Orchestrion will never add or modify any code in this function // ... etc ... @@ -39,11 +47,8 @@ func definitelyInstrumented() { // Orchestrion may add or modify code in this function // ... etc ... - //dd:ignore This particular server will NOT be instrumented - server := &http.Server { - Addr: "127.0.0.1:8080", - Handler: internalServerHandler, - } + //orchestrion:ignore This particular database connection will NOT be instrumented + db, err := db.Open("driver-name", "database=example") // Orchestrion may add or modify code further down in this function // ... etc ... @@ -57,8 +62,12 @@ instrumentation). In such cases, it is currently not possible to opt-out of instrumentation. This is the case for: +- `cloud.google.com/go/pubsub` +- `github.com/confluentinc/confluent-kafka-go/kafka` +- `github.com/gorilla/mux` +- `github.com/julienschmidt/httprouter` +- `github.com/segmentio/kafka-go` - `net/http` client instrumentation -- `github.com/gorilla/mux` middleware instrumentation {{}} ## Creating custom trace spans diff --git a/docs/content/docs/features.md b/docs/content/docs/features.md index 44c774a1..b5791db4 100644 --- a/docs/content/docs/features.md +++ b/docs/content/docs/features.md @@ -15,7 +15,7 @@ the designated environment variables, such as `DD_ENV`, `DD_SERVICE`, `DD_VERSION`, etc... You can get more information on what environment variables are available in the [documentation][env-var-doc]. -If the `main` function is annotated with the `//dd:ignore` directive, the tracer +If the `main` function is annotated with the `//orchestrion:ignore` directive, the tracer will not be started automatically, and you are responsible for calling {{}} with your preferred configuration options. diff --git a/docs/layouts/partials/navbar.html b/docs/layouts/partials/navbar.html index b2a3eb90..04adb27e 100644 --- a/docs/layouts/partials/navbar.html +++ b/docs/layouts/partials/navbar.html @@ -25,7 +25,6 @@ {{- if (.Site.Params.navbar.displayTitle | default true) }} {{- .Site.Title -}} {{- end }} - Alpha {{- $currentPage := . -}} diff --git a/internal/injector/injector.go b/internal/injector/injector.go index 171049b1..b55879cf 100644 --- a/internal/injector/injector.go +++ b/internal/injector/injector.go @@ -178,7 +178,7 @@ func (i *Injector) applyAspects(decorator *decorator.Decorator, file *dst.File, ) pre := func(csor *dstutil.Cursor) bool { - if err != nil || csor.Node() == nil || ddIgnored(csor.Node()) { + if err != nil || csor.Node() == nil || isIgnored(csor.Node()) { return false } root := chain == nil diff --git a/internal/injector/labels.go b/internal/injector/labels.go index 20658d6b..67315598 100644 --- a/internal/injector/labels.go +++ b/internal/injector/labels.go @@ -7,18 +7,29 @@ package injector import ( "strings" + "sync" + "github.com/DataDog/orchestrion/internal/log" "github.com/dave/dst" ) const ( - ddIgnore = "//dd:ignore" + ddIgnore = "//dd:ignore" + orchestrionIgnore = "//orchestrion:ignore" ) -// ddIgnored returns true if the node is prefixed by a `//dd:ignore` directive. -func ddIgnored(node dst.Node) bool { +var warnOnce sync.Once + +// isIgnored returns true if the node is prefixed by an `//orchestrion:ignore` (or the legacy `//dd:ignore`) directive. +func isIgnored(node dst.Node) bool { for _, cmt := range node.Decorations().Start.All() { + if cmt == orchestrionIgnore || strings.HasPrefix(cmt, orchestrionIgnore+" ") { + return true + } if cmt == ddIgnore || strings.HasPrefix(cmt, ddIgnore+" ") { + warnOnce.Do(func() { + log.Warnf("The //dd:ignore directive is deprecated and may be removed in a future release of orchestrion. Please use //orchestrion:ignore instead.") + }) return true } } diff --git a/internal/injector/testdata/injector/http-ignored/config.yml b/internal/injector/testdata/injector/http-ignored/config.yml index f07f2a0b..086b7d15 100644 --- a/internal/injector/testdata/injector/http-ignored/config.yml +++ b/internal/injector/testdata/injector/http-ignored/config.yml @@ -54,7 +54,7 @@ code: |- log.Printf("Server shut down: %v", s.ListenAndServe()) } - //dd:ignore + //orchestrion:ignore func handle(w http.ResponseWriter, r *http.Request) { data, err := io.ReadAll(r.Body) if err != nil {