update span name based on latest semconv (#367)

Author: minimAluminiumalism

Diff for: .github/workflows/plugins.yml

@@ -37,4 +37,4 @@ jobs:
     - name: Build
       run: make build
     - name: Test
-      run: go test -timeout 50m -v github.com/alibaba/opentelemetry-go-auto-instrumentation/test -run ".*Plugins"
+      run: go test -timeout 90m -v github.com/alibaba/opentelemetry-go-auto-instrumentation/test -run ".*Plugins"

Diff for: docs/experimental-feature.md

@@ -11,3 +11,9 @@
 | Environment Variable                                       | Type    | Default | Description                                                 |
 |------------------------------------------------------------|---------|---------|-------------------------------------------------------------|
 | `OTEL_INSTRUMENTATION_NACOS_EXPERIMENTAL_METRICS_ENABLE`   | Boolean | `false` | Enable the capture of experimental nacos metrics attributes.|
+
+## Settings for the DB instrumentation
+
+| Environment Variable                                       | Type    | Default | Description                                                 |
+|------------------------------------------------------------|---------|---------|-------------------------------------------------------------|
+| `OTEL_INSTRUMENTATION_DB_EXPERIMENTAL_ENABLE`              | Boolean | `false` | Enable the capture of experimental database span attributes.|

Diff for: docs/how-to-write-tests-for-plugins.md

@@ -50,8 +50,8 @@ represents for the `get` redis operation. You should use the `verifier` to verif
 import "github.com/alibaba/opentelemetry-go-auto-instrumentation/test/verifier"
 
 verifier.WaitAndAssertTraces(func (stubs []tracetest.SpanStubs) {
-	verifier.VerifyDbAttributes(stubs[0][0], "set", "", "redis", "", "localhost", "set a b ex 5 ", "set")
-	verifier.VerifyDbAttributes(stubs[1][0], "get", "", "redis", "", "localhost", "get a ", "get")
+	verifier.VerifyDbAttributes(stubs[0][0], "set", "", "redis", "", "localhost", "set a b ex 5 ", "set", "")
+	verifier.VerifyDbAttributes(stubs[1][0], "get", "", "redis", "", "localhost", "get a ", "get", "")
 })
 ```
 

Diff for: example/demo/pkg/http.go

@@ -101,7 +101,7 @@ func InitDB() {
 	}
 
 	// test insert
-	if _, err := db.ExecContext(context.Background(), `INSERT INTO usersx (id, name, age) VALUE ( ?, ?, ?)`, "0", "foo", 10); err != nil {
+	if _, err := db.ExecContext(context.Background(), `INSERT INTO usersx (id, name, age) VALUES ( ?, ?, ?)`, "0", "foo", 10); err != nil {
 		fmt.Printf("exec insert error: %v", err)
 	}
 }

Diff for: example/extension/sqlinject/sql_inject.go

@@ -28,7 +28,7 @@ func main() {
 		fmt.Printf("exec create error: %v", err)
 	}
 
-	if _, err := db.ExecContext(context.Background(), `INSERT INTO usersx (id, name, age) VALUE ( ?, ?, ?)`, "0", "foo", 10); err != nil {
+	if _, err := db.ExecContext(context.Background(), `INSERT INTO usersx (id, name, age) VALUES ( ?, ?, ?)`, "0", "foo", 10); err != nil {
 		fmt.Printf("exec insert error: %v", err)
 	}
 

Diff for: go.mod

@@ -119,6 +119,7 @@ require (
 	github.com/gorilla/css v1.0.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/henrylee2cn/ameda v1.4.10 // indirect
 	github.com/henrylee2cn/goutil v0.0.0-20210127050712-89660552f6f8 // indirect
 	github.com/huandu/xstrings v1.3.3 // indirect
@@ -194,6 +195,7 @@ require (
 	github.com/xdg-go/scram v1.1.2 // indirect
 	github.com/xdg-go/stringprep v1.0.4 // indirect
 	github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
+	github.com/xwb1989/sqlparser v0.0.0-20180606152119-120387863bf2
 	github.com/yargevad/filepathx v1.0.0 // indirect
 	github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect
 	github.com/yusufpapurcu/wmi v1.2.4 // indirect

Diff for: go.sum

@@ -342,6 +342,8 @@ github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
+github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
+github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/henrylee2cn/ameda v1.4.8/go.mod h1:liZulR8DgHxdK+MEwvZIylGnmcjzQ6N6f2PlWe7nEO4=
 github.com/henrylee2cn/ameda v1.4.10 h1:JdvI2Ekq7tapdPsuhrc4CaFiqw6QXFvZIULWJgQyCAk=
 github.com/henrylee2cn/ameda v1.4.10/go.mod h1:liZulR8DgHxdK+MEwvZIylGnmcjzQ6N6f2PlWe7nEO4=
@@ -645,6 +647,8 @@ github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17
 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
 github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU=
 github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8=
+github.com/xwb1989/sqlparser v0.0.0-20180606152119-120387863bf2 h1:zzrxE1FKn5ryBNl9eKOeqQ58Y/Qpo3Q9QNxKHX5uzzQ=
+github.com/xwb1989/sqlparser v0.0.0-20180606152119-120387863bf2/go.mod h1:hzfGeIUDq/j97IG+FhNqkowIyEcD88LrW6fyU3K3WqY=
 github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0 h1:6fRhSjgLCkTD3JnJxvaJ4Sj+TYblw757bqYgZaOq5ZY=
 github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0/go.mod h1:/LWChgwKmvncFJFHJ7Gvn9wZArjbV5/FppcK2fKk/tI=
 github.com/yargevad/filepathx v1.0.0 h1:SYcT+N3tYGi+NvazubCNlvgIPbzAk7i7y2dwg3I5FYc=

Diff for: pkg/inst-api-semconv/instrumenter/db/db_client_extractor.go

@@ -17,27 +17,37 @@ package db
 import (
 	"context"
 	"fmt"
+	"os"
+	"strconv"
+
 	"github.com/alibaba/opentelemetry-go-auto-instrumentation/pkg/inst-api/instrumenter"
 	"github.com/alibaba/opentelemetry-go-auto-instrumentation/pkg/inst-api/utils"
 	"go.opentelemetry.io/otel/attribute"
 	semconv "go.opentelemetry.io/otel/semconv/v1.30.0"
-	"strconv"
 )
 
 // TODO: remove server.address and put it into NetworkAttributesExtractor
 
-type DbExperimentalAttributesEnabler interface {
-	Enable() bool
-}
+const EnvDBExperimentalEnabled = "OTEL_INSTRUMENTATION_DB_EXPERIMENTAL_ENABLE"
 
-type defaultDbExperimentalAttributesEnabler struct {
+type dbExperimentalEnabler struct {
+	enabled bool
 }
 
-func (d defaultDbExperimentalAttributesEnabler) Enable() bool {
-	return false
+func (d dbExperimentalEnabler) Enable() bool {
+	return d.enabled
 }
 
-var experimentalAttributesEnabler = instrumenter.NewDefaultInstrumentEnabler()
+var experimentalAttributesEnabler instrumenter.InstrumentEnabler = &dbExperimentalEnabler{
+	// SQL params parsing is not enabled by default.
+	enabled: func() bool {
+		val, err := strconv.ParseBool(os.Getenv(EnvDBExperimentalEnabled))
+		if err != nil {
+			return false
+		}
+		return val
+	}(),
+}
 
 type DbClientCommonAttrsExtractor[REQUEST any, RESPONSE any, GETTER DbClientCommonAttrsGetter[REQUEST]] struct {
 	Getter           GETTER
@@ -86,6 +96,9 @@ func (d *DbClientAttrsExtractor[REQUEST, RESPONSE, GETTER]) OnEnd(attrs []attrib
 	}, attribute.KeyValue{
 		Key:   semconv.ServerAddressKey,
 		Value: attribute.StringValue(d.Base.Getter.GetServerAddress(request)),
+	}, attribute.KeyValue{
+		Key:   semconv.DBCollectionNameKey,
+		Value: attribute.StringValue(d.Base.Getter.GetCollection(request)),
 	})
 	batchSize := d.Base.Getter.GetBatchSize(request)
 	if batchSize > 0 {
@@ -95,13 +108,12 @@ func (d *DbClientAttrsExtractor[REQUEST, RESPONSE, GETTER]) OnEnd(attrs []attrib
 	if dbNameSpace != "" {
 		attrs = append(attrs, attribute.KeyValue{Key: semconv.DBNamespaceKey, Value: attribute.StringValue(dbNameSpace)})
 	}
-	// TODO: add db.collection.name after doing sanitizing
 	if d.Base.AttributesFilter != nil {
 		attrs = d.Base.AttributesFilter(attrs)
 	}
 	if experimentalAttributesEnabler.Enable() {
 		params := d.Base.Getter.GetParameters(request)
-		if params != nil && len(params) > 0 {
+		if len(params) > 0 {
 			for i, param := range params {
 				attrs = append(attrs, attribute.String("db.query.parameter."+strconv.Itoa(i), fmt.Sprintf("%v", param)))
 			}

Diff for: pkg/inst-api-semconv/instrumenter/db/db_client_extractor_test.go

@@ -16,16 +16,18 @@ package db
 
 import (
 	"context"
+	"log"
+	"testing"
+
 	"github.com/alibaba/opentelemetry-go-auto-instrumentation/pkg/inst-api/utils"
 	"go.opentelemetry.io/otel/attribute"
 	semconv "go.opentelemetry.io/otel/semconv/v1.30.0"
-	"log"
-	"testing"
 )
 
 type testRequest struct {
 	Name      string
 	Operation string
+	Target    string
 }
 
 type testResponse struct {
@@ -66,6 +68,10 @@ func (m mongoAttrsGetter) GetParameters(request testRequest) []any {
 	return nil
 }
 
+func (m mongoAttrsGetter) GetCollection(request testRequest) string {
+	return request.Target
+}
+
 func TestGetSpanKey(t *testing.T) {
 	dbExtractor := &DbClientAttrsExtractor[testRequest, any, mongoAttrsGetter]{}
 	if dbExtractor.GetSpanKey() != utils.DB_CLIENT_KEY {

Diff for: pkg/inst-api-semconv/instrumenter/db/db_client_getter.go

@@ -25,6 +25,7 @@ type DbClientAttrsGetter[REQUEST any] interface {
 	DbClientCommonAttrsGetter[REQUEST]
 	GetStatement(REQUEST) string
 	GetOperation(REQUEST) string
+	GetCollection(REQUEST) string
 	GetParameters(REQUEST) []any
 }
 

Diff for: pkg/inst-api-semconv/instrumenter/db/db_span_name_extractor.go

@@ -18,10 +18,23 @@ type DBSpanNameExtractor[REQUEST any] struct {
 	Getter DbClientAttrsGetter[REQUEST]
 }
 
+// ref: https://opentelemetry.io/docs/specs/semconv/database/database-spans/#name
 func (d *DBSpanNameExtractor[REQUEST]) Extract(request REQUEST) string {
 	operation := d.Getter.GetOperation(request)
-	if operation == "" {
-		return "DB Query"
+	target := d.Getter.GetCollection(request)
+	system := d.Getter.GetSystem(request)
+
+	if operation != "" && target != "" {
+		return operation + " " + target
 	}
-	return operation
-}
+
+	if operation != "" {
+		return operation
+	}
+	// If neither {db.operation.name} nor {target} are available, span name SHOULD be {db.system}.
+	if system != "" {
+		return system
+	}
+	
+	return "DB"
+}

Diff for: pkg/inst-api-semconv/instrumenter/db/db_span_name_extractor_test.go

@@ -16,17 +16,23 @@ package db
 
 import "testing"
 
-func TestDbNameExtractor(t *testing.T) {
+func TestDbSpanNameExtractor(t *testing.T) {
 	dbSpanNameExtractor := DBSpanNameExtractor[testRequest]{
 		Getter: mongoAttrsGetter{},
 	}
-	if dbSpanNameExtractor.Extract(testRequest{}) != "DB Query" {
-		t.Fatalf("Should have returned DB Query")
+	if dbSpanNameExtractor.Extract(testRequest{}) != "DB" {
+		t.Fatalf("Should have returned DB")
 	}
-	if dbSpanNameExtractor.Extract(testRequest{Name: "test", Operation: "test"}) != "test" {
-		t.Fatalf("Should have returned test")
+	if dbSpanNameExtractor.Extract(testRequest{Name: "test", Operation: "op"}) != "op" {
+		t.Fatalf("Should have returned op")
 	}
 	if dbSpanNameExtractor.Extract(testRequest{Operation: "op_test"}) != "op_test" {
 		t.Fatalf("Should have returned op_test")
 	}
+	if dbSpanNameExtractor.Extract(testRequest{Operation: "op", Target: "table"}) != "op table" {
+		t.Fatalf("Should have returned `op table`")
+	}
+	if dbSpanNameExtractor.Extract(testRequest{Name: "test"}) != "test" {
+		t.Fatalf("Shoule have returned test")
+	}
 }